diff --git a/adb/adb.cpp b/adb/adb.cpp index 60966d6a8..19dd03db9 100644 --- a/adb/adb.cpp +++ b/adb/adb.cpp @@ -334,7 +334,7 @@ std::string get_connection_string() { #endif connection_properties.push_back(android::base::StringPrintf( - "features=%s", android::base::Join(supported_features(), ',').c_str())); + "features=%s", FeatureSetToString(supported_features()).c_str())); return android::base::StringPrintf( "%s::%s", adb_device_banner, @@ -396,9 +396,7 @@ void parse_banner(const std::string& banner, atransport* t) { } else if (key == "ro.product.device") { qual_overwrite(&t->device, value); } else if (key == "features") { - for (const auto& feature : android::base::Split(value, ",")) { - t->add_feature(feature); - } + t->SetFeatures(value); } } } @@ -1149,27 +1147,13 @@ int handle_host_request(const char* service, TransportType type, std::string error_msg; atransport* t = acquire_one_transport(kCsAny, type, serial, &error_msg); if (t != nullptr) { - SendOkay(reply_fd, android::base::Join(t->features(), '\n')); + SendOkay(reply_fd, FeatureSetToString(t->features())); } else { SendFail(reply_fd, error_msg); } return 0; } - if (!strncmp(service, "check-feature:", strlen("check-feature:"))) { - std::string error_msg; - atransport* t = acquire_one_transport(kCsAny, type, serial, &error_msg); - if (t && t->CanUseFeature(service + strlen("check-feature:"))) { - // We could potentially extend this to reply with the feature - // version if that becomes necessary. - SendOkay(reply_fd, "1"); - } else { - // Empty response means unsupported feature. - SendOkay(reply_fd, ""); - } - return 0; - } - // remove TCP transport if (!strncmp(service, "disconnect:", 11)) { const std::string address(service + 11); diff --git a/adb/commandline.cpp b/adb/commandline.cpp index f5c00bd20..cc933ea31 100644 --- a/adb/commandline.cpp +++ b/adb/commandline.cpp @@ -36,6 +36,7 @@ #include #include +#include #if !defined(_WIN32) #include @@ -108,7 +109,9 @@ static void help() { " 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 - run remote shell command\n" + " adb shell [-Tt] - run remote shell command\n" + " (-T disables PTY allocation)\n" + " (-t forces PTY allocation)\n" " adb emu - run emulator console command\n" " adb logcat [ ] - View device log\n" " adb forward --list - list all forward socket connections.\n" @@ -483,9 +486,10 @@ static void* stdin_read_thread(void* x) { return nullptr; } -static int interactive_shell(bool use_shell_protocol) { +static int interactive_shell(const std::string& service_string, + bool use_shell_protocol) { std::string error; - int fd = adb_connect("shell:", &error); + int fd = adb_connect(service_string, &error); if (fd < 0) { fprintf(stderr,"error: %s\n", error.c_str()); return 1; @@ -532,18 +536,16 @@ static std::string format_host_command(const char* command, TransportType type, return android::base::StringPrintf("%s:%s", prefix, command); } -// Checks whether the device indicated by |transport_type| and |serial| supports -// |feature|. Returns the response string, which will be empty if the device -// could not be found or the feature is not supported. -static std::string CheckFeature(const std::string& feature, - TransportType transport_type, +// Returns the FeatureSet for the indicated transport. +static FeatureSet GetFeatureSet(TransportType transport_type, const char* serial) { - std::string result, error, command("check-feature:" + feature); - if (!adb_query(format_host_command(command.c_str(), transport_type, serial), - &result, &error)) { - return ""; + std::string result, error; + + if (adb_query(format_host_command("features", transport_type, serial), + &result, &error)) { + return StringToFeatureSet(result); } - return result; + return FeatureSet(); } static int adb_download_buffer(const char *service, const char *fn, const void* data, unsigned sz, @@ -805,9 +807,8 @@ static int send_shell_command(TransportType transport_type, const char* serial, wait_for_device("wait-for-device", transport_type, serial); } - bool use_shell_protocol = !CheckFeature(kFeatureShell2, transport_type, - serial).empty(); - int exit_code = read_and_dump(fd, use_shell_protocol); + FeatureSet features = GetFeatureSet(transport_type, serial); + int exit_code = read_and_dump(fd, features.count(kFeatureShell2) > 0); if (adb_close(fd) < 0) { PLOG(ERROR) << "failure closing FD " << fd; @@ -1246,24 +1247,44 @@ int adb_commandline(int argc, const char **argv) { else if (!strcmp(argv[0], "shell") || !strcmp(argv[0], "hell")) { char h = (argv[0][0] == 'h'); + FeatureSet features = GetFeatureSet(transport_type, serial); + + bool use_shell_protocol = (features.count(kFeatureShell2) > 0); + if (!use_shell_protocol) { + D("shell protocol not supported, using raw data transfer"); + } else { + D("using shell protocol"); + } + + // Parse shell-specific command-line options. + // argv[0] is always "shell". + --argc; + ++argv; + std::string shell_type_arg; + while (argc) { + if (!strcmp(argv[0], "-T") || !strcmp(argv[0], "-t")) { + if (features.count(kFeatureShell2) == 0) { + fprintf(stderr, "error: target doesn't support PTY args -Tt\n"); + return 1; + } + shell_type_arg = argv[0]; + --argc; + ++argv; + } else { + break; + } + } + std::string service_string = android::base::StringPrintf( + "shell%s:", shell_type_arg.c_str()); + if (h) { printf("\x1b[41;33m"); fflush(stdout); } - bool use_shell_protocol; - if (CheckFeature(kFeatureShell2, transport_type, serial).empty()) { - D("shell protocol not supported, using raw data transfer"); - use_shell_protocol = false; - } else { - D("using shell protocol"); - use_shell_protocol = true; - } - - - if (argc < 2) { + if (!argc) { D("starting interactive shell"); - r = interactive_shell(use_shell_protocol); + r = interactive_shell(service_string, use_shell_protocol); if (h) { printf("\x1b[0m"); fflush(stdout); @@ -1271,19 +1292,14 @@ int adb_commandline(int argc, const char **argv) { return r; } - std::string cmd = "shell:"; - --argc; - ++argv; - while (argc-- > 0) { - // We don't escape here, just like ssh(1). http://b/20564385. - cmd += *argv++; - if (*argv) cmd += " "; - } + // We don't escape here, just like ssh(1). http://b/20564385. + service_string += android::base::Join( + std::vector(argv, argv + argc), ' '); while (true) { - D("non-interactive shell loop. cmd=%s", cmd.c_str()); + D("non-interactive shell loop. cmd=%s", service_string.c_str()); std::string error; - int fd = adb_connect(cmd, &error); + int fd = adb_connect(service_string, &error); int r; if (fd >= 0) { D("about to read_and_dump(fd=%d)", fd); @@ -1553,7 +1569,14 @@ int adb_commandline(int argc, const char **argv) { return 0; } else if (!strcmp(argv[0], "features")) { - return adb_query_command(format_host_command("features", transport_type, serial)); + // 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) { + printf("%s\n", name.c_str()); + } + } + return 0; } usage(); diff --git a/adb/services.cpp b/adb/services.cpp index d128efc3d..d0494ec02 100644 --- a/adb/services.cpp +++ b/adb/services.cpp @@ -194,7 +194,39 @@ void reverse_service(int fd, void* arg) adb_close(fd); } -#endif +// Shell service string can look like: +// shell[args]:[command] +// Currently the only supported args are -T (force raw) and -t (force PTY). +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; + } + + SubprocessProtocol protocol = + (transport->CanUseFeature(kFeatureShell2) ? SubprocessProtocol::kShell + : SubprocessProtocol::kNone); + + return StartSubprocess(command.c_str(), type, protocol); +} + +#endif // !ADB_HOST static int create_service_thread(void (*func)(int, void *), void *cookie) { @@ -265,14 +297,8 @@ int service_to_fd(const char* name, const atransport* transport) { ret = create_service_thread(framebuffer_service, 0); } else if (!strncmp(name, "jdwp:", 5)) { ret = create_jdwp_connection_fd(atoi(name+5)); - } else if(!strncmp(name, "shell:", 6)) { - const char* args = name + 6; - // Use raw for non-interactive, PTY for interactive. - SubprocessType type = (*args ? SubprocessType::kRaw : SubprocessType::kPty); - SubprocessProtocol protocol = - (transport->CanUseFeature(kFeatureShell2) ? SubprocessProtocol::kShell - : SubprocessProtocol::kNone); - ret = StartSubprocess(args, type, protocol); + } else if(!strncmp(name, "shell", 5)) { + ret = ShellService(name + 5, transport); } else if(!strncmp(name, "exec:", 5)) { ret = StartSubprocess(name + 5, SubprocessType::kRaw, SubprocessProtocol::kNone); diff --git a/adb/transport.cpp b/adb/transport.cpp index db9236edd..402546b0a 100644 --- a/adb/transport.cpp +++ b/adb/transport.cpp @@ -779,27 +779,37 @@ size_t atransport::get_max_payload() const { return max_payload; } -// Do not use any of [:;=,] in feature strings, they have special meaning -// in the connection banner. -// TODO(dpursell): add this in once we can pass features through to the client. -const char kFeatureShell2[] = "shell_2"; +namespace { -// The list of features supported by the current system. Will be sent to the -// other side of the connection in the banner. -static const FeatureSet gSupportedFeatures = { - kFeatureShell2, -}; +constexpr char kFeatureStringDelimiter = ','; + +} // namespace const FeatureSet& supported_features() { - return gSupportedFeatures; + // Local static allocation to avoid global non-POD variables. + static const FeatureSet* features = new FeatureSet{ + kFeatureShell2 + }; + + return *features; +} + +std::string FeatureSetToString(const FeatureSet& features) { + return android::base::Join(features, kFeatureStringDelimiter); +} + +FeatureSet StringToFeatureSet(const std::string& features_string) { + auto names = android::base::Split(features_string, + {kFeatureStringDelimiter}); + return FeatureSet(names.begin(), names.end()); } bool atransport::has_feature(const std::string& feature) const { return features_.count(feature) > 0; } -void atransport::add_feature(const std::string& feature) { - features_.insert(feature); +void atransport::SetFeatures(const std::string& features_string) { + features_ = StringToFeatureSet(features_string); } bool atransport::CanUseFeature(const std::string& feature) const { @@ -855,9 +865,6 @@ static void append_transport(const atransport* t, std::string* result, append_transport_info(result, "product:", t->product, false); append_transport_info(result, "model:", t->model, true); append_transport_info(result, "device:", t->device, false); - append_transport_info(result, "features:", - android::base::Join(t->features(), ',').c_str(), - false); } *result += '\n'; } diff --git a/adb/transport.h b/adb/transport.h index 999922a38..0ec8cebbf 100644 --- a/adb/transport.h +++ b/adb/transport.h @@ -29,7 +29,13 @@ typedef std::unordered_set FeatureSet; const FeatureSet& supported_features(); -const extern char kFeatureShell2[]; +// Encodes and decodes FeatureSet objects into human-readable strings. +std::string FeatureSetToString(const FeatureSet& features); +FeatureSet StringToFeatureSet(const std::string& features_string); + +// Do not use any of [:;=,] in feature strings, they have special meaning +// in the connection banner. +constexpr char kFeatureShell2[] = "shell_2"; class atransport { public: @@ -85,12 +91,14 @@ public: int get_protocol_version() const; size_t get_max_payload() const; - inline const FeatureSet features() const { + const FeatureSet& features() const { return features_; } bool has_feature(const std::string& feature) const; - void add_feature(const std::string& feature); + + // 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. diff --git a/adb/transport_test.cpp b/adb/transport_test.cpp index 10872ac3e..7d69c3ea7 100644 --- a/adb/transport_test.cpp +++ b/adb/transport_test.cpp @@ -144,23 +144,29 @@ TEST(transport, RunDisconnects) { ASSERT_EQ(0, count); } -TEST(transport, add_feature) { +TEST(transport, SetFeatures) { atransport t; ASSERT_EQ(0U, t.features().size()); - t.add_feature("foo"); + t.SetFeatures(FeatureSetToString(FeatureSet{"foo"})); ASSERT_EQ(1U, t.features().size()); ASSERT_TRUE(t.has_feature("foo")); - t.add_feature("bar"); + t.SetFeatures(FeatureSetToString(FeatureSet{"foo", "bar"})); ASSERT_EQ(2U, t.features().size()); ASSERT_TRUE(t.has_feature("foo")); ASSERT_TRUE(t.has_feature("bar")); - t.add_feature("foo"); + t.SetFeatures(FeatureSetToString(FeatureSet{"foo", "bar", "foo"})); ASSERT_EQ(2U, t.features().size()); ASSERT_TRUE(t.has_feature("foo")); ASSERT_TRUE(t.has_feature("bar")); + + t.SetFeatures(FeatureSetToString(FeatureSet{"bar", "baz"})); + ASSERT_EQ(2U, t.features().size()); + ASSERT_FALSE(t.has_feature("foo")); + ASSERT_TRUE(t.has_feature("bar")); + ASSERT_TRUE(t.has_feature("baz")); } TEST(transport, parse_banner_no_features) {