From 70ef7b40f96e44ebb86f4eb23ccfa1a9230cdb65 Mon Sep 17 00:00:00 2001 From: David Pursell Date: Wed, 30 Sep 2015 13:35:42 -0700 Subject: [PATCH] adb: put legacy shell: service back in. ddmlib does not use the ADB client, but instead connects directly to the adb server. This breaks some of the assumptions I previously made when enabling the shell protocol. To fix this, the adb server now defaults to no protocol for the standalone command, and the shell protocol must be explicitly requested by the client. For example: shell:echo foo -- no shell protocol shell,v2:echo foo -- shell protocol As long as I was touching the shell service arguments I also changed them to no longer duplicate the command-line arguments. This allows more flexibility to change the adb client CLI if necessary and makes the code more readable. Bug: http://b/24148636 Change-Id: I28d5ae578cf18cbe79347dc89cea1750ff4571a8 --- adb/adb.h | 2 +- adb/commandline.cpp | 86 ++++++++++++++++++++++++++++++++++----------- adb/services.cpp | 39 +++++++++++--------- adb/services.h | 24 +++++++++++++ adb/transport.cpp | 9 ++--- adb/transport.h | 9 +++-- 6 files changed, 122 insertions(+), 47 deletions(-) create mode 100644 adb/services.h diff --git a/adb/adb.h b/adb/adb.h index a20b3452a..c284c8c52 100644 --- a/adb/adb.h +++ b/adb/adb.h @@ -50,7 +50,7 @@ constexpr size_t MAX_PAYLOAD = MAX_PAYLOAD_V2; std::string adb_version(); // Increment this when we want to force users to start a new adb server. -#define ADB_SERVER_VERSION 33 +#define ADB_SERVER_VERSION 34 class atransport; struct usb_handle; diff --git a/adb/commandline.cpp b/adb/commandline.cpp index 8aad97dcb..d0ca67aea 100644 --- a/adb/commandline.cpp +++ b/adb/commandline.cpp @@ -49,6 +49,7 @@ #include "adb_io.h" #include "adb_utils.h" #include "file_sync_service.h" +#include "services.h" #include "shell_service.h" #include "transport.h" @@ -108,10 +109,11 @@ static void help() { " ('-a' means copy timestamp and mode)\n" " adb sync [ ] - copy host->device only if changed\n" " (-l means list but don't copy)\n" - " adb shell - run remote shell interactively\n" - " adb shell [-Tt] - run remote shell command\n" + " adb shell [-Ttx] - run remote shell interactively\n" + " adb shell [-Ttx] - run remote shell command\n" " (-T disables PTY allocation)\n" " (-t forces PTY allocation)\n" + " (-x disables remote exit codes and stdout/stderr separation)\n" " adb emu - run emulator console command\n" " adb logcat [ ] - View device log\n" " adb forward --list - list all forward socket connections.\n" @@ -785,12 +787,45 @@ static bool wait_for_device(const char* service, TransportType t, const char* se return adb_command(cmd); } +// Returns a shell service string with the indicated arguments and command. +static std::string ShellServiceString(bool use_shell_protocol, + const std::string& type_arg, + const std::string& command) { + std::vector args; + if (use_shell_protocol) { + args.push_back(kShellServiceArgShellProtocol); + } + if (!type_arg.empty()) { + args.push_back(type_arg); + } + + // Shell service string can look like: shell[,arg1,arg2,...]:[command]. + return android::base::StringPrintf("shell%s%s:%s", + args.empty() ? "" : ",", + android::base::Join(args, ',').c_str(), + command.c_str()); +} + +// Connects to the device "shell" service with |command| and prints the +// resulting output. static int send_shell_command(TransportType transport_type, const char* serial, - const std::string& command) { + const std::string& command, + bool disable_shell_protocol) { + // Only use shell protocol if it's supported and the caller doesn't want + // to explicitly disable it. + bool use_shell_protocol = false; + if (!disable_shell_protocol) { + FeatureSet features = GetFeatureSet(transport_type, serial); + use_shell_protocol = CanUseFeature(features, kFeatureShell2); + } + + std::string service_string = ShellServiceString(use_shell_protocol, "", + command); + int fd; while (true) { std::string error; - fd = adb_connect(command, &error); + fd = adb_connect(service_string, &error); if (fd >= 0) { break; } @@ -799,8 +834,7 @@ static int send_shell_command(TransportType transport_type, const char* serial, wait_for_device("wait-for-device", transport_type, serial); } - FeatureSet features = GetFeatureSet(transport_type, serial); - int exit_code = read_and_dump(fd, features.count(kFeatureShell2) > 0); + int exit_code = read_and_dump(fd, use_shell_protocol); if (adb_close(fd) < 0) { PLOG(ERROR) << "failure closing FD " << fd; @@ -813,7 +847,7 @@ static int logcat(TransportType transport, const char* serial, int argc, const c char* log_tags = getenv("ANDROID_LOG_TAGS"); std::string quoted = escape_arg(log_tags == nullptr ? "" : log_tags); - std::string cmd = "shell:export ANDROID_LOG_TAGS=\"" + quoted + "\"; exec logcat"; + std::string cmd = "export ANDROID_LOG_TAGS=\"" + quoted + "\"; exec logcat"; if (!strcmp(argv[0], "longcat")) { cmd += " -v long"; @@ -825,7 +859,8 @@ static int logcat(TransportType transport, const char* serial, int argc, const c cmd += " " + escape_arg(*argv++); } - return send_shell_command(transport, serial, cmd); + // No need for shell protocol with logcat, always disable for simplicity. + return send_shell_command(transport, serial, cmd, true); } static int backup(int argc, const char** argv) { @@ -1241,7 +1276,7 @@ int adb_commandline(int argc, const char **argv) { FeatureSet features = GetFeatureSet(transport_type, serial); - bool use_shell_protocol = (features.count(kFeatureShell2) > 0); + bool use_shell_protocol = CanUseFeature(features, kFeatureShell2); if (!use_shell_protocol) { D("shell protocol not supported, using raw data transfer"); } else { @@ -1255,19 +1290,22 @@ int adb_commandline(int argc, const char **argv) { std::string shell_type_arg; while (argc) { if (!strcmp(argv[0], "-T") || !strcmp(argv[0], "-t")) { - if (features.count(kFeatureShell2) == 0) { + if (!CanUseFeature(features, kFeatureShell2)) { fprintf(stderr, "error: target doesn't support PTY args -Tt\n"); return 1; } - shell_type_arg = argv[0]; + shell_type_arg = (argv[0][1] == 'T') ? kShellServiceArgRaw + : kShellServiceArgPty; + --argc; + ++argv; + } else if (!strcmp(argv[0], "-x")) { + use_shell_protocol = false; --argc; ++argv; } else { break; } } - std::string service_string = android::base::StringPrintf( - "shell%s:", shell_type_arg.c_str()); if (h) { printf("\x1b[41;33m"); @@ -1276,6 +1314,8 @@ int adb_commandline(int argc, const char **argv) { if (!argc) { D("starting interactive shell"); + std::string service_string = + ShellServiceString(use_shell_protocol, shell_type_arg, ""); r = interactive_shell(service_string, use_shell_protocol); if (h) { printf("\x1b[0m"); @@ -1285,8 +1325,10 @@ int adb_commandline(int argc, const char **argv) { } // We don't escape here, just like ssh(1). http://b/20564385. - service_string += android::base::Join( + std::string command = android::base::Join( std::vector(argv, argv + argc), ' '); + std::string service_string = + ShellServiceString(use_shell_protocol, shell_type_arg, command); while (true) { D("non-interactive shell loop. cmd=%s", service_string.c_str()); @@ -1389,7 +1431,9 @@ int adb_commandline(int argc, const char **argv) { } else if (!strcmp(argv[0], "bugreport")) { if (argc != 1) return usage(); - return send_shell_command(transport_type, serial, "shell:bugreport"); + // No need for shell protocol with bugreport, always disable for + // simplicity. + return send_shell_command(transport_type, serial, "bugreport", true); } else if (!strcmp(argv[0], "forward") || !strcmp(argv[0], "reverse")) { bool reverse = !strcmp(argv[0], "reverse"); @@ -1566,7 +1610,7 @@ int adb_commandline(int argc, const char **argv) { // Only list the features common to both the adb client and the device. FeatureSet features = GetFeatureSet(transport_type, serial); for (const std::string& name : features) { - if (supported_features().count(name) > 0) { + if (CanUseFeature(features, name)) { printf("%s\n", name.c_str()); } } @@ -1578,13 +1622,15 @@ int adb_commandline(int argc, const char **argv) { } static int pm_command(TransportType transport, const char* serial, int argc, const char** argv) { - std::string cmd = "shell:pm"; + std::string cmd = "pm"; while (argc-- > 0) { cmd += " " + escape_arg(*argv++); } - return send_shell_command(transport, serial, cmd); + // TODO(dpursell): add command-line arguments to install/uninstall to + // manually disable shell protocol if needed. + return send_shell_command(transport, serial, cmd, false); } static int uninstall_app(TransportType transport, const char* serial, int argc, const char** argv) { @@ -1605,8 +1651,8 @@ static int uninstall_app(TransportType transport, const char* serial, int argc, } static int delete_file(TransportType transport, const char* serial, const std::string& filename) { - std::string cmd = "shell:rm -f " + escape_arg(filename); - return send_shell_command(transport, serial, cmd); + std::string cmd = "rm -f " + escape_arg(filename); + return send_shell_command(transport, serial, cmd, false); } static int install_app(TransportType transport, const char* serial, int argc, const char** argv) { diff --git a/adb/services.cpp b/adb/services.cpp index 1153c3106..e832b1e60 100644 --- a/adb/services.cpp +++ b/adb/services.cpp @@ -46,6 +46,7 @@ #include "adb_utils.h" #include "file_sync_service.h" #include "remount_service.h" +#include "services.h" #include "shell_service.h" #include "transport.h" @@ -195,33 +196,37 @@ void reverse_service(int fd, void* arg) } // Shell service string can look like: -// shell[args]:[command] -// Currently the only supported args are -T (force raw) and -t (force PTY). +// shell[,arg1,arg2,...]:[command] static int ShellService(const std::string& args, const atransport* transport) { size_t delimiter_index = args.find(':'); if (delimiter_index == std::string::npos) { LOG(ERROR) << "No ':' found in shell service arguments: " << args; return -1; } + const std::string service_args = args.substr(0, delimiter_index); const std::string command = args.substr(delimiter_index + 1); - SubprocessType type; - if (service_args.empty()) { - // Default: use PTY for interactive, raw for non-interactive. - type = (command.empty() ? SubprocessType::kPty : SubprocessType::kRaw); - } else if (service_args == "-T") { - type = SubprocessType::kRaw; - } else if (service_args == "-t") { - type = SubprocessType::kPty; - } else { - LOG(ERROR) << "Unsupported shell service arguments: " << args; - return -1; - } + // Defaults: + // PTY for interactive, raw for non-interactive. + // No protocol. + SubprocessType type(command.empty() ? SubprocessType::kPty + : SubprocessType::kRaw); + SubprocessProtocol protocol = SubprocessProtocol::kNone; - SubprocessProtocol protocol = - (transport->CanUseFeature(kFeatureShell2) ? SubprocessProtocol::kShell - : SubprocessProtocol::kNone); + for (const std::string& arg : android::base::Split(service_args, ",")) { + if (arg == kShellServiceArgRaw) { + type = SubprocessType::kRaw; + } else if (arg == kShellServiceArgPty) { + type = SubprocessType::kPty; + } else if (arg == kShellServiceArgShellProtocol) { + protocol = SubprocessProtocol::kShell; + } + else if (!arg.empty()) { + LOG(ERROR) << "Unsupported shell service arguments: " << args; + return -1; + } + } return StartSubprocess(command.c_str(), type, protocol); } diff --git a/adb/services.h b/adb/services.h new file mode 100644 index 000000000..0428ca4ae --- /dev/null +++ b/adb/services.h @@ -0,0 +1,24 @@ +/* + * Copyright (C) 2015 The Android Open Source Project + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +#ifndef SERVICES_H_ +#define SERVICES_H_ + +constexpr char kShellServiceArgRaw[] = "raw"; +constexpr char kShellServiceArgPty[] = "pty"; +constexpr char kShellServiceArgShellProtocol[] = "v2"; + +#endif // SERVICES_H_ diff --git a/adb/transport.cpp b/adb/transport.cpp index 2a2fac714..501a39afa 100644 --- a/adb/transport.cpp +++ b/adb/transport.cpp @@ -808,6 +808,11 @@ FeatureSet StringToFeatureSet(const std::string& features_string) { return FeatureSet(names.begin(), names.end()); } +bool CanUseFeature(const FeatureSet& feature_set, const std::string& feature) { + return feature_set.count(feature) > 0 && + supported_features().count(feature) > 0; +} + bool atransport::has_feature(const std::string& feature) const { return features_.count(feature) > 0; } @@ -816,10 +821,6 @@ void atransport::SetFeatures(const std::string& features_string) { features_ = StringToFeatureSet(features_string); } -bool atransport::CanUseFeature(const std::string& feature) const { - return has_feature(feature) && supported_features().count(feature) > 0; -} - void atransport::AddDisconnect(adisconnect* disconnect) { disconnects_.push_back(disconnect); } diff --git a/adb/transport.h b/adb/transport.h index 0ec8cebbf..dfe887508 100644 --- a/adb/transport.h +++ b/adb/transport.h @@ -33,9 +33,12 @@ const FeatureSet& supported_features(); std::string FeatureSetToString(const FeatureSet& features); FeatureSet StringToFeatureSet(const std::string& features_string); +// Returns true if both local features and |feature_set| support |feature|. +bool CanUseFeature(const FeatureSet& feature_set, const std::string& feature); + // Do not use any of [:;=,] in feature strings, they have special meaning // in the connection banner. -constexpr char kFeatureShell2[] = "shell_2"; +constexpr char kFeatureShell2[] = "shell_v2"; class atransport { public: @@ -100,10 +103,6 @@ public: // Loads the transport's feature set from the given string. void SetFeatures(const std::string& features_string); - // Returns true if both we and the other end of the transport support the - // feature. - bool CanUseFeature(const std::string& feature) const; - void AddDisconnect(adisconnect* disconnect); void RemoveDisconnect(adisconnect* disconnect); void RunDisconnects();