From 0955c66b226db7a7f34613f834f7b0a145fd407d Mon Sep 17 00:00:00 2001 From: David Pursell Date: Mon, 31 Aug 2015 10:42:13 -0700 Subject: [PATCH 1/2] adb: implement shell protocol. Adds functionality for handling stdin/stdout/stderr streams and exit codes using the shell protocol. This CL just contains implementation for adbd which will not yet be enabled. Once we have the ability to query transport features from the adb client, another CL will add the implementation for the client side and update the feature list to turn this on. Note: this CL must be submitted together with a minadbd CL to update the service_to_fd() function signature. Bug: http://b/23030641 Change-Id: Ibed55e9c1946d8a35190696163ff63e8fb880238 --- adb/Android.mk | 2 + adb/adb.cpp | 2 +- adb/adb.h | 5 +- adb/services.cpp | 23 +-- adb/shell_service.cpp | 329 ++++++++++++++++++++++++++++++++++--- adb/shell_service.h | 8 +- adb/shell_service_test.cpp | 270 ++++++++++++++++++++++++++++++ adb/sockets.cpp | 5 +- adb/transport.cpp | 5 + adb/transport.h | 2 + 10 files changed, 615 insertions(+), 36 deletions(-) create mode 100644 adb/shell_service_test.cpp diff --git a/adb/Android.mk b/adb/Android.mk index 543f1eb2a..60673e618 100644 --- a/adb/Android.mk +++ b/adb/Android.mk @@ -129,8 +129,10 @@ LOCAL_CFLAGS := -DADB_HOST=0 $(LIBADB_CFLAGS) LOCAL_SRC_FILES := \ $(LIBADB_TEST_SRCS) \ $(LIBADB_TEST_linux_SRCS) \ + shell_service.cpp \ shell_service_protocol.cpp \ shell_service_protocol_test.cpp \ + shell_service_test.cpp \ LOCAL_SANITIZE := $(adb_target_sanitize) LOCAL_STATIC_LIBRARIES := libadbd diff --git a/adb/adb.cpp b/adb/adb.cpp index 0bd95a3f2..b260a785c 100644 --- a/adb/adb.cpp +++ b/adb/adb.cpp @@ -501,7 +501,7 @@ void handle_packet(apacket *p, atransport *t) if (t->online && p->msg.arg0 != 0 && p->msg.arg1 == 0) { char *name = (char*) p->data; name[p->msg.data_length > 0 ? p->msg.data_length - 1 : 0] = 0; - s = create_local_service_socket(name); + s = create_local_service_socket(name, t); if(s == 0) { send_close(0, p->msg.arg0, t); } else { diff --git a/adb/adb.h b/adb/adb.h index 8b3359ea8..4922040a5 100644 --- a/adb/adb.h +++ b/adb/adb.h @@ -221,7 +221,8 @@ void remove_socket(asocket *s); void close_all_sockets(atransport *t); asocket *create_local_socket(int fd); -asocket *create_local_service_socket(const char *destination); +asocket *create_local_service_socket(const char* destination, + const atransport* transport); asocket *create_remote_socket(unsigned id, atransport *t); void connect_to_remote(asocket *s, const char *destination); @@ -247,7 +248,7 @@ void init_usb_transport(atransport *t, usb_handle *usb, ConnectionState state); atransport* find_emulator_transport_by_adb_port(int adb_port); #endif -int service_to_fd(const char *name); +int service_to_fd(const char* name, const atransport* transport); #if ADB_HOST asocket *host_service_to_socket(const char* name, const char *serial); #endif diff --git a/adb/services.cpp b/adb/services.cpp index 561431c82..d128efc3d 100644 --- a/adb/services.cpp +++ b/adb/services.cpp @@ -225,7 +225,7 @@ static int create_service_thread(void (*func)(int, void *), void *cookie) return s[0]; } -int service_to_fd(const char* name) { +int service_to_fd(const char* name, const atransport* transport) { int ret = -1; if(!strncmp(name, "tcp:", 4)) { @@ -267,15 +267,15 @@ int service_to_fd(const char* name) { ret = create_jdwp_connection_fd(atoi(name+5)); } else if(!strncmp(name, "shell:", 6)) { const char* args = name + 6; - if (*args) { - // Non-interactive session uses a raw subprocess. - ret = StartSubprocess(args, SubprocessType::kRaw); - } else { - // Interactive session uses a PTY subprocess. - ret = StartSubprocess(args, SubprocessType::kPty); - } + // 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, "exec:", 5)) { - ret = StartSubprocess(name + 5, SubprocessType::kRaw); + ret = StartSubprocess(name + 5, SubprocessType::kRaw, + SubprocessProtocol::kNone); } else if(!strncmp(name, "sync:", 5)) { ret = create_service_thread(file_sync_service, NULL); } else if(!strncmp(name, "remount:", 8)) { @@ -291,9 +291,10 @@ int service_to_fd(const char* name) { } else if(!strncmp(name, "backup:", 7)) { ret = StartSubprocess(android::base::StringPrintf("/system/bin/bu backup %s", (name + 7)).c_str(), - SubprocessType::kRaw); + SubprocessType::kRaw, SubprocessProtocol::kNone); } else if(!strncmp(name, "restore:", 8)) { - ret = StartSubprocess("/system/bin/bu restore", SubprocessType::kRaw); + ret = StartSubprocess("/system/bin/bu restore", SubprocessType::kRaw, + SubprocessProtocol::kNone); } else if(!strncmp(name, "tcpip:", 6)) { int port; if (sscanf(name + 6, "%d", &port) != 1) { diff --git a/adb/shell_service.cpp b/adb/shell_service.cpp index 5f80a593e..0274ae33b 100644 --- a/adb/shell_service.cpp +++ b/adb/shell_service.cpp @@ -14,6 +14,67 @@ * limitations under the License. */ +// Functionality for launching and managing shell subprocesses. +// +// There are two types of subprocesses, PTY or raw. PTY is typically used for +// an interactive session, raw for non-interactive. There are also two methods +// of communication with the subprocess, passing raw data or using a simple +// protocol to wrap packets. The protocol allows separating stdout/stderr and +// passing the exit code back, but is not backwards compatible. +// ----------------+-------------------------------------- +// Type Protocol | Exit code? Separate stdout/stderr? +// ----------------+-------------------------------------- +// PTY No | No No +// Raw No | No No +// PTY Yes | Yes No +// Raw Yes | Yes Yes +// ----------------+-------------------------------------- +// +// Non-protocol subprocesses work by passing subprocess stdin/out/err through +// a single pipe which is registered with a local socket in adbd. The local +// socket uses the fdevent loop to pass raw data between this pipe and the +// transport, which then passes data back to the adb client. Cleanup is done by +// waiting in a separate thread for the subprocesses to exit and then signaling +// a separate fdevent to close out the local socket from the main loop. +// +// ------------------+-------------------------+------------------------------ +// Subprocess | adbd subprocess thread | adbd main fdevent loop +// ------------------+-------------------------+------------------------------ +// | | +// stdin/out/err <-----------------------------> LocalSocket +// | | | +// | | Block on exit | +// | | * | +// v | * | +// Exit ---> Unblock | +// | | | +// | v | +// | Notify shell exit FD ---> Close LocalSocket +// ------------------+-------------------------+------------------------------ +// +// The protocol requires the thread to intercept stdin/out/err in order to +// wrap/unwrap data with shell protocol packets. +// +// ------------------+-------------------------+------------------------------ +// Subprocess | adbd subprocess thread | adbd main fdevent loop +// ------------------+-------------------------+------------------------------ +// | | +// stdin/out <---> Protocol <---> LocalSocket +// stderr ---> Protocol ---> LocalSocket +// | | | +// v | | +// Exit ---> Exit code protocol ---> LocalSocket +// | | | +// | v | +// | Notify shell exit FD ---> Close LocalSocket +// ------------------+-------------------------+------------------------------ +// +// An alternate approach is to put the protocol wrapping/unwrapping in the main +// fdevent loop, which has the advantage of being able to re-use the existing +// select() code for handling data streams. However, implementation turned out +// to be more complex due to partial reads and non-blocking I/O so this model +// was chosen instead. + #define TRACE_TAG TRACE_SHELL #include "shell_service.h" @@ -22,8 +83,11 @@ #include #include +#include #include +#include + #include #include #include @@ -110,7 +174,8 @@ bool CreateSocketpair(ScopedFd* fd1, ScopedFd* fd2) { class Subprocess { public: - Subprocess(const std::string& command, SubprocessType type); + Subprocess(const std::string& command, SubprocessType type, + SubprocessProtocol protocol); ~Subprocess(); const std::string& command() const { return command_; } @@ -129,26 +194,42 @@ class Subprocess { int OpenPtyChildFd(const char* pts_name, ScopedFd* error_sfd); static void* ThreadHandler(void* userdata); + void PassDataStreams(); void WaitForExit(); + ScopedFd* SelectLoop(fd_set* master_read_set_ptr, + fd_set* master_write_set_ptr); + + // Input/output stream handlers. Success returns nullptr, failure returns + // a pointer to the failed FD. + ScopedFd* PassInput(); + ScopedFd* PassOutput(ScopedFd* sfd, ShellProtocol::Id id); + const std::string command_; SubprocessType type_; - + SubprocessProtocol protocol_; pid_t pid_ = -1; ScopedFd local_socket_sfd_; + // Shell protocol variables. + ScopedFd stdinout_sfd_, stderr_sfd_, protocol_sfd_; + std::unique_ptr input_, output_; + size_t input_bytes_left_ = 0; + DISALLOW_COPY_AND_ASSIGN(Subprocess); }; -Subprocess::Subprocess(const std::string& command, SubprocessType type) - : command_(command), type_(type) { +Subprocess::Subprocess(const std::string& command, SubprocessType type, + SubprocessProtocol protocol) + : command_(command), type_(type), protocol_(protocol) { } Subprocess::~Subprocess() { } bool Subprocess::ForkAndExec() { - ScopedFd parent_sfd, child_sfd, parent_error_sfd, child_error_sfd; + ScopedFd child_stdinout_sfd, child_stderr_sfd; + ScopedFd parent_error_sfd, child_error_sfd; char pts_name[PATH_MAX]; // Create a socketpair for the fork() child to report any errors back to @@ -161,9 +242,14 @@ bool Subprocess::ForkAndExec() { if (type_ == SubprocessType::kPty) { int fd; pid_ = forkpty(&fd, pts_name, nullptr, nullptr); - parent_sfd.Reset(fd); + stdinout_sfd_.Reset(fd); } else { - if (!CreateSocketpair(&parent_sfd, &child_sfd)) { + if (!CreateSocketpair(&stdinout_sfd_, &child_stdinout_sfd)) { + return false; + } + // Raw subprocess + shell protocol allows for splitting stderr. + if (protocol_ == SubprocessProtocol::kShell && + !CreateSocketpair(&stderr_sfd_, &child_stderr_sfd)) { return false; } pid_ = fork(); @@ -179,16 +265,19 @@ bool Subprocess::ForkAndExec() { init_subproc_child(); if (type_ == SubprocessType::kPty) { - child_sfd.Reset(OpenPtyChildFd(pts_name, &child_error_sfd)); + child_stdinout_sfd.Reset(OpenPtyChildFd(pts_name, &child_error_sfd)); } - dup2(child_sfd.fd(), STDIN_FILENO); - dup2(child_sfd.fd(), STDOUT_FILENO); - dup2(child_sfd.fd(), STDERR_FILENO); + dup2(child_stdinout_sfd.fd(), STDIN_FILENO); + dup2(child_stdinout_sfd.fd(), STDOUT_FILENO); + dup2(child_stderr_sfd.valid() ? child_stderr_sfd.fd() : child_stdinout_sfd.fd(), + STDERR_FILENO); // exec doesn't trigger destructors, close the FDs manually. - parent_sfd.Reset(); - child_sfd.Reset(); + stdinout_sfd_.Reset(); + stderr_sfd_.Reset(); + child_stdinout_sfd.Reset(); + child_stderr_sfd.Reset(); parent_error_sfd.Reset(); close_on_exec(child_error_sfd.fd()); @@ -203,7 +292,8 @@ bool Subprocess::ForkAndExec() { } // Subprocess parent. - D("subprocess parent: subprocess FD = %d", parent_sfd.fd()); + D("subprocess parent: stdin/stdout FD = %d, stderr FD = %d", + stdinout_sfd_.fd(), stderr_sfd_.fd()); // Wait to make sure the subprocess exec'd without error. child_error_sfd.Reset(); @@ -213,7 +303,38 @@ bool Subprocess::ForkAndExec() { return false; } - local_socket_sfd_.Reset(parent_sfd.Release()); + if (protocol_ == SubprocessProtocol::kNone) { + // No protocol: all streams pass through the stdinout FD and hook + // directly into the local socket for raw data transfer. + local_socket_sfd_.Reset(stdinout_sfd_.Release()); + } else { + // Shell protocol: create another socketpair to intercept data. + if (!CreateSocketpair(&protocol_sfd_, &local_socket_sfd_)) { + return false; + } + D("protocol FD = %d", protocol_sfd_.fd()); + + input_.reset(new ShellProtocol(protocol_sfd_.fd())); + output_.reset(new ShellProtocol(protocol_sfd_.fd())); + if (!input_ || !output_) { + LOG(ERROR) << "failed to allocate shell protocol objects"; + return false; + } + + // Don't let reads/writes to the subprocess block our thread. This isn't + // likely but could happen under unusual circumstances, such as if we + // write a ton of data to stdin but the subprocess never reads it and + // the pipe fills up. + for (int fd : {stdinout_sfd_.fd(), stderr_sfd_.fd()}) { + if (fd >= 0) { + int flags = fcntl(fd, F_GETFL, 0); + if (flags < 0 || fcntl(fd, F_SETFL, flags | O_NONBLOCK) < 0) { + PLOG(ERROR) << "error making FD " << fd << " non-blocking"; + return false; + } + } + } + } if (!adb_thread_create(ThreadHandler, this)) { PLOG(ERROR) << "failed to create subprocess thread"; @@ -259,6 +380,7 @@ void* Subprocess::ThreadHandler(void* userdata) { adb_thread_setname(android::base::StringPrintf( "shell srvc %d", subprocess->local_socket_fd())); + subprocess->PassDataStreams(); subprocess->WaitForExit(); D("deleting Subprocess"); @@ -267,25 +389,192 @@ void* Subprocess::ThreadHandler(void* userdata) { return nullptr; } +void Subprocess::PassDataStreams() { + if (!protocol_sfd_.valid()) { + return; + } + + // Start by trying to read from the protocol FD, stdout, and stderr. + fd_set master_read_set, master_write_set; + FD_ZERO(&master_read_set); + FD_ZERO(&master_write_set); + for (ScopedFd* sfd : {&protocol_sfd_, &stdinout_sfd_, &stderr_sfd_}) { + if (sfd->valid()) { + FD_SET(sfd->fd(), &master_read_set); + } + } + + // Pass data until the protocol FD or both the subprocess pipes die, at + // which point we can't pass any more data. + while (protocol_sfd_.valid() && + (stdinout_sfd_.valid() || stderr_sfd_.valid())) { + ScopedFd* dead_sfd = SelectLoop(&master_read_set, &master_write_set); + if (dead_sfd) { + D("closing FD %d", dead_sfd->fd()); + FD_CLR(dead_sfd->fd(), &master_read_set); + FD_CLR(dead_sfd->fd(), &master_write_set); + dead_sfd->Reset(); + } + } +} + +namespace { + +inline bool ValidAndInSet(const ScopedFd& sfd, fd_set* set) { + return sfd.valid() && FD_ISSET(sfd.fd(), set); +} + +} // namespace + +ScopedFd* Subprocess::SelectLoop(fd_set* master_read_set_ptr, + fd_set* master_write_set_ptr) { + fd_set read_set, write_set; + int select_n = std::max(std::max(protocol_sfd_.fd(), stdinout_sfd_.fd()), + stderr_sfd_.fd()) + 1; + ScopedFd* dead_sfd = nullptr; + + // Keep calling select() and passing data until an FD closes/errors. + while (!dead_sfd) { + memcpy(&read_set, master_read_set_ptr, sizeof(read_set)); + memcpy(&write_set, master_write_set_ptr, sizeof(write_set)); + if (select(select_n, &read_set, &write_set, nullptr, nullptr) < 0) { + if (errno == EINTR) { + continue; + } else { + PLOG(ERROR) << "select failed, closing subprocess pipes"; + stdinout_sfd_.Reset(); + stderr_sfd_.Reset(); + return nullptr; + } + } + + // Read stdout, write to protocol FD. + if (ValidAndInSet(stdinout_sfd_, &read_set)) { + dead_sfd = PassOutput(&stdinout_sfd_, ShellProtocol::kIdStdout); + } + + // Read stderr, write to protocol FD. + if (!dead_sfd && ValidAndInSet(stderr_sfd_, &read_set)) { + dead_sfd = PassOutput(&stderr_sfd_, ShellProtocol::kIdStderr); + } + + // Read protocol FD, write to stdin. + if (!dead_sfd && ValidAndInSet(protocol_sfd_, &read_set)) { + dead_sfd = PassInput(); + // If we didn't finish writing, block on stdin write. + if (input_bytes_left_) { + FD_CLR(protocol_sfd_.fd(), master_read_set_ptr); + FD_SET(stdinout_sfd_.fd(), master_write_set_ptr); + } + } + + // Continue writing to stdin; only happens if a previous write blocked. + if (!dead_sfd && ValidAndInSet(stdinout_sfd_, &write_set)) { + dead_sfd = PassInput(); + // If we finished writing, go back to blocking on protocol read. + if (!input_bytes_left_) { + FD_SET(protocol_sfd_.fd(), master_read_set_ptr); + FD_CLR(stdinout_sfd_.fd(), master_write_set_ptr); + } + } + } // while (!dead_sfd) + + return dead_sfd; +} + +ScopedFd* Subprocess::PassInput() { + // Only read a new packet if we've finished writing the last one. + if (!input_bytes_left_) { + if (!input_->Read()) { + // Read() uses ReadFdExactly() which sets errno to 0 on EOF. + if (errno != 0) { + PLOG(ERROR) << "error reading protocol FD " + << protocol_sfd_.fd(); + } + return &protocol_sfd_; + } + + // We only care about stdin packets. + if (stdinout_sfd_.valid() && input_->id() == ShellProtocol::kIdStdin) { + input_bytes_left_ = input_->data_length(); + } else { + input_bytes_left_ = 0; + } + } + + if (input_bytes_left_ > 0) { + int index = input_->data_length() - input_bytes_left_; + int bytes = adb_write(stdinout_sfd_.fd(), input_->data() + index, + input_bytes_left_); + if (bytes == 0 || (bytes < 0 && errno != EAGAIN)) { + if (bytes < 0) { + PLOG(ERROR) << "error reading stdin FD " << stdinout_sfd_.fd(); + } + // stdin is done, mark this packet as finished and we'll just start + // dumping any further data received from the protocol FD. + input_bytes_left_ = 0; + return &stdinout_sfd_; + } else if (bytes > 0) { + input_bytes_left_ -= bytes; + } + } + + return nullptr; +} + +ScopedFd* Subprocess::PassOutput(ScopedFd* sfd, ShellProtocol::Id id) { + int bytes = adb_read(sfd->fd(), output_->data(), output_->data_capacity()); + if (bytes == 0 || (bytes < 0 && errno != EAGAIN)) { + if (bytes < 0) { + PLOG(ERROR) << "error reading output FD " << sfd->fd(); + } + return sfd; + } + + if (bytes > 0 && !output_->Write(id, bytes)) { + if (errno != 0) { + PLOG(ERROR) << "error reading protocol FD " << protocol_sfd_.fd(); + } + return &protocol_sfd_; + } + + return nullptr; +} + void Subprocess::WaitForExit() { + int exit_code = 1; + D("waiting for pid %d", pid_); while (true) { int status; if (pid_ == waitpid(pid_, &status, 0)) { D("post waitpid (pid=%d) status=%04x", pid_, status); if (WIFSIGNALED(status)) { + exit_code = 0x80 | WTERMSIG(status); D("subprocess killed by signal %d", WTERMSIG(status)); break; } else if (!WIFEXITED(status)) { D("subprocess didn't exit"); break; } else if (WEXITSTATUS(status) >= 0) { + exit_code = WEXITSTATUS(status); D("subprocess exit code = %d", WEXITSTATUS(status)); break; } } } + // If we have an open protocol FD send an exit packet. + if (protocol_sfd_.valid()) { + output_->data()[0] = exit_code; + if (output_->Write(ShellProtocol::kIdExit, 1)) { + D("wrote the exit code packet: %d", exit_code); + } else { + PLOG(ERROR) << "failed to write the exit code packet"; + } + protocol_sfd_.Reset(); + } + // Pass the local socket FD to the shell cleanup fdevent. if (SHELL_EXIT_NOTIFY_FD >= 0) { int fd = local_socket_sfd_.fd(); @@ -305,11 +594,13 @@ void Subprocess::WaitForExit() { } // namespace -int StartSubprocess(const char *name, SubprocessType type) { - D("starting %s subprocess: '%s'", - type == SubprocessType::kRaw ? "raw" : "PTY", name); +int StartSubprocess(const char *name, SubprocessType type, + SubprocessProtocol protocol) { + D("starting %s subprocess (protocol=%s): '%s'", + type == SubprocessType::kRaw ? "raw" : "PTY", + protocol == SubprocessProtocol::kNone ? "none" : "shell", name); - Subprocess* subprocess = new Subprocess(name, type); + Subprocess* subprocess = new Subprocess(name, type, protocol); if (!subprocess) { LOG(ERROR) << "failed to allocate new subprocess"; return -1; diff --git a/adb/shell_service.h b/adb/shell_service.h index 81d7036a5..8868f1074 100644 --- a/adb/shell_service.h +++ b/adb/shell_service.h @@ -124,11 +124,17 @@ enum class SubprocessType { kRaw, }; +enum class SubprocessProtocol { + kNone, + kShell, +}; + // Forks and starts a new shell subprocess. If |name| is empty an interactive // shell is started, otherwise |name| is executed non-interactively. // // Returns an open FD connected to the subprocess or -1 on failure. -int StartSubprocess(const char* name, SubprocessType type); +int StartSubprocess(const char* name, SubprocessType type, + SubprocessProtocol protocol); #endif // !ADB_HOST diff --git a/adb/shell_service_test.cpp b/adb/shell_service_test.cpp new file mode 100644 index 000000000..20efd84e0 --- /dev/null +++ b/adb/shell_service_test.cpp @@ -0,0 +1,270 @@ +/* + * 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. + */ + +#include "shell_service.h" + +#include + +#include + +#include +#include + +#include + +#include "adb.h" +#include "adb_io.h" +#include "sysdeps.h" + +class ShellServiceTest : public ::testing::Test { + public: + static void SetUpTestCase() { + // This is normally done in main.cpp. + saved_sigpipe_handler_ = signal(SIGPIPE, SIG_IGN); + + } + + static void TearDownTestCase() { + signal(SIGPIPE, saved_sigpipe_handler_); + } + + // Helpers to start and cleanup a subprocess. Cleanup normally does not + // need to be called manually unless multiple subprocesses are run from + // a single test. + void StartTestSubprocess(const char* command, SubprocessType type, + SubprocessProtocol protocol); + void CleanupTestSubprocess(); + + virtual void TearDown() override { + void CleanupTestSubprocess(); + } + + static sighandler_t saved_sigpipe_handler_; + + int subprocess_fd_ = -1; + int shell_exit_receiver_fd_ = -1, saved_shell_exit_fd_; +}; + +sighandler_t ShellServiceTest::saved_sigpipe_handler_ = nullptr; + +void ShellServiceTest::StartTestSubprocess( + const char* command, SubprocessType type, SubprocessProtocol protocol) { + // We want to intercept the shell exit message to make sure it's sent. + saved_shell_exit_fd_ = SHELL_EXIT_NOTIFY_FD; + int fd[2]; + ASSERT_TRUE(adb_socketpair(fd) >= 0); + SHELL_EXIT_NOTIFY_FD = fd[0]; + shell_exit_receiver_fd_ = fd[1]; + + subprocess_fd_ = StartSubprocess(command, type, protocol); + ASSERT_TRUE(subprocess_fd_ >= 0); +} + +void ShellServiceTest::CleanupTestSubprocess() { + if (subprocess_fd_ >= 0) { + // Subprocess should send its FD to SHELL_EXIT_NOTIFY_FD for cleanup. + int notified_fd = -1; + ASSERT_TRUE(ReadFdExactly(shell_exit_receiver_fd_, ¬ified_fd, + sizeof(notified_fd))); + ASSERT_EQ(notified_fd, subprocess_fd_); + + adb_close(subprocess_fd_); + subprocess_fd_ = -1; + + // Restore SHELL_EXIT_NOTIFY_FD. + adb_close(SHELL_EXIT_NOTIFY_FD); + adb_close(shell_exit_receiver_fd_); + shell_exit_receiver_fd_ = -1; + SHELL_EXIT_NOTIFY_FD = saved_shell_exit_fd_; + } +} + +namespace { + +// Reads raw data from |fd| until it closes or errors. +std::string ReadRaw(int fd) { + char buffer[1024]; + char *cur_ptr = buffer, *end_ptr = buffer + sizeof(buffer); + + while (1) { + int bytes = adb_read(fd, cur_ptr, end_ptr - cur_ptr); + if (bytes <= 0) { + return std::string(buffer, cur_ptr); + } + cur_ptr += bytes; + } +} + +// Reads shell protocol data from |fd| until it closes or errors. Fills +// |stdout| and |stderr| with their respective data, and returns the exit code +// read from the protocol or -1 if an exit code packet was not received. +int ReadShellProtocol(int fd, std::string* stdout, std::string* stderr) { + int exit_code = -1; + stdout->clear(); + stderr->clear(); + + ShellProtocol* protocol = new ShellProtocol(fd); + while (protocol->Read()) { + switch (protocol->id()) { + case ShellProtocol::kIdStdout: + stdout->append(protocol->data(), protocol->data_length()); + break; + case ShellProtocol::kIdStderr: + stderr->append(protocol->data(), protocol->data_length()); + break; + case ShellProtocol::kIdExit: + EXPECT_EQ(-1, exit_code) << "Multiple exit packets received"; + EXPECT_EQ(1u, protocol->data_length()); + exit_code = protocol->data()[0]; + break; + default: + ADD_FAILURE() << "Unidentified packet ID: " << protocol->id(); + } + } + delete protocol; + + return exit_code; +} + +// Checks if each line in |lines| exists in the same order in |output|. Blank +// lines in |output| are ignored for simplicity. +bool ExpectLinesEqual(const std::string& output, + const std::vector& lines) { + auto output_lines = android::base::Split(output, "\r\n"); + size_t i = 0; + + for (const std::string& line : lines) { + // Skip empty lines in output. + while (i < output_lines.size() && output_lines[i].empty()) { + ++i; + } + if (i >= output_lines.size()) { + ADD_FAILURE() << "Ran out of output lines"; + return false; + } + EXPECT_EQ(line, output_lines[i]); + ++i; + } + + while (i < output_lines.size() && output_lines[i].empty()) { + ++i; + } + EXPECT_EQ(i, output_lines.size()) << "Found unmatched output lines"; + return true; +} + +} // namespace + +// Tests a raw subprocess with no protocol. +TEST_F(ShellServiceTest, RawNoProtocolSubprocess) { + // [ -t 0 ] checks if stdin is connected to a terminal. + ASSERT_NO_FATAL_FAILURE(StartTestSubprocess( + "echo foo; echo bar >&2; [ -t 0 ]; echo $?", + SubprocessType::kRaw, SubprocessProtocol::kNone)); + + // [ -t 0 ] == 1 means no terminal (raw). + ExpectLinesEqual(ReadRaw(subprocess_fd_), {"foo", "bar", "1"}); +} + +// Tests a PTY subprocess with no protocol. +TEST_F(ShellServiceTest, PtyNoProtocolSubprocess) { + // [ -t 0 ] checks if stdin is connected to a terminal. + ASSERT_NO_FATAL_FAILURE(StartTestSubprocess( + "echo foo; echo bar >&2; [ -t 0 ]; echo $?", + SubprocessType::kPty, SubprocessProtocol::kNone)); + + // [ -t 0 ] == 0 means we have a terminal (PTY). + ExpectLinesEqual(ReadRaw(subprocess_fd_), {"foo", "bar", "0"}); +} + +// Tests a raw subprocess with the shell protocol. +TEST_F(ShellServiceTest, RawShellProtocolSubprocess) { + ASSERT_NO_FATAL_FAILURE(StartTestSubprocess( + "echo foo; echo bar >&2; echo baz; exit 24", + SubprocessType::kRaw, SubprocessProtocol::kShell)); + + std::string stdout, stderr; + EXPECT_EQ(24, ReadShellProtocol(subprocess_fd_, &stdout, &stderr)); + ExpectLinesEqual(stdout, {"foo", "baz"}); + ExpectLinesEqual(stderr, {"bar"}); +} + +// Tests a PTY subprocess with the shell protocol. +TEST_F(ShellServiceTest, PtyShellProtocolSubprocess) { + ASSERT_NO_FATAL_FAILURE(StartTestSubprocess( + "echo foo; echo bar >&2; echo baz; exit 50", + SubprocessType::kPty, SubprocessProtocol::kShell)); + + // PTY always combines stdout and stderr but the shell protocol should + // still give us an exit code. + std::string stdout, stderr; + EXPECT_EQ(50, ReadShellProtocol(subprocess_fd_, &stdout, &stderr)); + ExpectLinesEqual(stdout, {"foo", "bar", "baz"}); + ExpectLinesEqual(stderr, {}); +} + +// Tests an interactive PTY session. +TEST_F(ShellServiceTest, InteractivePtySubprocess) { + ASSERT_NO_FATAL_FAILURE(StartTestSubprocess( + "", SubprocessType::kPty, SubprocessProtocol::kShell)); + + // Use variable substitution so echoed input is different from output. + const char* commands[] = {"TEST_STR=abc123", + "echo --${TEST_STR}--", + "exit"}; + + ShellProtocol* protocol = new ShellProtocol(subprocess_fd_); + for (std::string command : commands) { + // Interactive shell requires a newline to complete each command. + command.push_back('\n'); + memcpy(protocol->data(), command.data(), command.length()); + ASSERT_TRUE(protocol->Write(ShellProtocol::kIdStdin, command.length())); + } + delete protocol; + + std::string stdout, stderr; + EXPECT_EQ(0, ReadShellProtocol(subprocess_fd_, &stdout, &stderr)); + // An unpredictable command prompt makes parsing exact output difficult but + // it should at least contain echoed input and the expected output. + for (const char* command : commands) { + EXPECT_FALSE(stdout.find(command) == std::string::npos); + } + EXPECT_FALSE(stdout.find("--abc123--") == std::string::npos); +} + +// Tests that nothing breaks when the stdin/stdout pipe closes. +TEST_F(ShellServiceTest, CloseStdinStdoutSubprocess) { + ASSERT_NO_FATAL_FAILURE(StartTestSubprocess( + "exec 0<&-; exec 1>&-; echo bar >&2", + SubprocessType::kRaw, SubprocessProtocol::kShell)); + + std::string stdout, stderr; + EXPECT_EQ(0, ReadShellProtocol(subprocess_fd_, &stdout, &stderr)); + ExpectLinesEqual(stdout, {}); + ExpectLinesEqual(stderr, {"bar"}); +} + +// Tests that nothing breaks when the stderr pipe closes. +TEST_F(ShellServiceTest, CloseStderrSubprocess) { + ASSERT_NO_FATAL_FAILURE(StartTestSubprocess( + "exec 2>&-; echo foo", + SubprocessType::kRaw, SubprocessProtocol::kShell)); + + std::string stdout, stderr; + EXPECT_EQ(0, ReadShellProtocol(subprocess_fd_, &stdout, &stderr)); + ExpectLinesEqual(stdout, {"foo"}); + ExpectLinesEqual(stderr, {}); +} diff --git a/adb/sockets.cpp b/adb/sockets.cpp index f8c22cc74..104ad6b12 100644 --- a/adb/sockets.cpp +++ b/adb/sockets.cpp @@ -422,7 +422,8 @@ asocket *create_local_socket(int fd) return s; } -asocket *create_local_service_socket(const char *name) +asocket *create_local_service_socket(const char *name, + const atransport* transport) { #if !ADB_HOST if (!strcmp(name,"jdwp")) { @@ -432,7 +433,7 @@ asocket *create_local_service_socket(const char *name) return create_jdwp_tracker_service_socket(); } #endif - int fd = service_to_fd(name); + int fd = service_to_fd(name, transport); if(fd < 0) return 0; asocket* s = create_local_socket(fd); diff --git a/adb/transport.cpp b/adb/transport.cpp index e97c47986..f6334f7bf 100644 --- a/adb/transport.cpp +++ b/adb/transport.cpp @@ -779,6 +779,11 @@ 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"; + // 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 = { diff --git a/adb/transport.h b/adb/transport.h index 3b56c55e0..999922a38 100644 --- a/adb/transport.h +++ b/adb/transport.h @@ -29,6 +29,8 @@ typedef std::unordered_set FeatureSet; const FeatureSet& supported_features(); +const extern char kFeatureShell2[]; + class atransport { public: // TODO(danalbert): We expose waaaaaaay too much stuff because this was From 606835ae5c4b9519009cdff8b1c33169cff32cb1 Mon Sep 17 00:00:00 2001 From: David Pursell Date: Tue, 8 Sep 2015 17:17:02 -0700 Subject: [PATCH 2/2] adb: add client side shell protocol and enable. Adds the shell protocol functionality to the client side and enables it if the transport supports the feature. Bug:http://b/23031026 Change-Id: I9abe1c8b1d39f8dd09666321b1c761ad708a8854 --- adb/commandline.cpp | 191 ++++++++++++++++++++++++++++++++------------ adb/device.py | 75 +++++++++++++---- adb/test_device.py | 63 +++++++++------ adb/transport.cpp | 2 +- 4 files changed, 243 insertions(+), 88 deletions(-) diff --git a/adb/commandline.cpp b/adb/commandline.cpp index 8d50f465a..c508b3285 100644 --- a/adb/commandline.cpp +++ b/adb/commandline.cpp @@ -31,8 +31,10 @@ #include #include +#include #include +#include #include #if !defined(_WIN32) @@ -46,6 +48,8 @@ #include "adb_io.h" #include "adb_utils.h" #include "file_sync_service.h" +#include "shell_service.h" +#include "transport.h" static int install_app(TransportType t, const char* serial, int argc, const char** argv); static int install_multiple_app(TransportType t, const char* serial, int argc, const char** argv); @@ -256,19 +260,60 @@ static void stdin_raw_restore(int fd) { } #endif -static void read_and_dump(int fd) { +// Reads from |fd| and prints received data. If |use_shell_protocol| is true +// this expects that incoming data will use the shell protocol, in which case +// stdout/stderr are routed independently and the remote exit code will be +// returned. +static int read_and_dump(int fd, bool use_shell_protocol=false) { + int exit_code = 0; + std::unique_ptr protocol; + int length = 0; + FILE* outfile = stdout; + + char raw_buffer[BUFSIZ]; + char* buffer_ptr = raw_buffer; + if (use_shell_protocol) { + protocol.reset(new ShellProtocol(fd)); + if (!protocol) { + LOG(ERROR) << "failed to allocate memory for ShellProtocol object"; + return 1; + } + buffer_ptr = protocol->data(); + } + while (fd >= 0) { - D("read_and_dump(): pre adb_read(fd=%d)", fd); - char buf[BUFSIZ]; - int len = adb_read(fd, buf, sizeof(buf)); - D("read_and_dump(): post adb_read(fd=%d): len=%d", fd, len); - if (len <= 0) { - break; + if (use_shell_protocol) { + if (!protocol->Read()) { + break; + } + switch (protocol->id()) { + case ShellProtocol::kIdStdout: + outfile = stdout; + break; + case ShellProtocol::kIdStderr: + outfile = stderr; + break; + case ShellProtocol::kIdExit: + exit_code = protocol->data()[0]; + continue; + default: + continue; + } + length = protocol->data_length(); + } else { + D("read_and_dump(): pre adb_read(fd=%d)", fd); + length = adb_read(fd, raw_buffer, sizeof(raw_buffer)); + D("read_and_dump(): post adb_read(fd=%d): length=%d", fd, length); + if (length <= 0) { + break; + } } - fwrite(buf, 1, len, stdout); - fflush(stdout); + fwrite(buffer_ptr, 1, length, outfile); + fflush(outfile); } + + return exit_code; } static void read_status_line(int fd, char* buf, size_t count) @@ -362,28 +407,41 @@ static void copy_to_file(int inFd, int outFd) { free(buf); } -static void *stdin_read_thread(void *x) -{ - int fd, fdi; - unsigned char buf[1024]; - int r, n; - int state = 0; +namespace { - int *fds = (int*) x; - fd = fds[0]; - fdi = fds[1]; - free(fds); +// Used to pass multiple values to the stdin read thread. +struct StdinReadArgs { + int stdin_fd, write_fd; + std::unique_ptr protocol; +}; + +} // namespace + +// Loops to read from stdin and push the data to the given FD. +// The argument should be a pointer to a StdinReadArgs object. This function +// will take ownership of the object and delete it when finished. +static void* stdin_read_thread(void* x) { + std::unique_ptr args(reinterpret_cast(x)); + int state = 0; adb_thread_setname("stdin reader"); + char raw_buffer[1024]; + char* buffer_ptr = raw_buffer; + size_t buffer_size = sizeof(raw_buffer); + if (args->protocol) { + buffer_ptr = args->protocol->data(); + buffer_size = args->protocol->data_capacity(); + } + while (true) { - /* fdi is really the client's stdin, so use read, not adb_read here */ - D("stdin_read_thread(): pre unix_read(fdi=%d,...)", fdi); - r = unix_read(fdi, buf, 1024); - D("stdin_read_thread(): post unix_read(fdi=%d,...)", fdi); + // Use unix_read() rather than adb_read() for stdin. + D("stdin_read_thread(): pre unix_read(fdi=%d,...)", args->stdin_fd); + int r = unix_read(args->stdin_fd, buffer_ptr, buffer_size); + D("stdin_read_thread(): post unix_read(fdi=%d,...)", args->stdin_fd); if (r <= 0) break; - for (n = 0; n < r; n++){ - switch(buf[n]) { + for (int n = 0; n < r; n++){ + switch(buffer_ptr[n]) { case '\n': state = 1; break; @@ -396,47 +454,59 @@ static void *stdin_read_thread(void *x) case '.': if(state == 2) { fprintf(stderr,"\n* disconnect *\n"); - stdin_raw_restore(fdi); + stdin_raw_restore(args->stdin_fd); exit(0); } default: state = 0; } } - r = adb_write(fd, buf, r); - if(r <= 0) { - break; + if (args->protocol) { + if (!args->protocol->Write(ShellProtocol::kIdStdin, r)) { + break; + } + } else { + if (!WriteFdExactly(args->write_fd, buffer_ptr, r)) { + break; + } } } - return 0; + + return nullptr; } -static int interactive_shell() { - int fdi; - +static int interactive_shell(bool use_shell_protocol) { std::string error; int fd = adb_connect("shell:", &error); if (fd < 0) { fprintf(stderr,"error: %s\n", error.c_str()); return 1; } - fdi = 0; //dup(0); - int* fds = reinterpret_cast(malloc(sizeof(int) * 2)); - if (fds == nullptr) { - fprintf(stderr, "couldn't allocate fds array: %s\n", strerror(errno)); + StdinReadArgs* args = new StdinReadArgs; + if (!args) { + LOG(ERROR) << "couldn't allocate StdinReadArgs object"; return 1; } + args->stdin_fd = 0; + args->write_fd = fd; + if (use_shell_protocol) { + args->protocol.reset(new ShellProtocol(args->write_fd)); + } - fds[0] = fd; - fds[1] = fdi; + stdin_raw_init(args->stdin_fd); - stdin_raw_init(fdi); + int exit_code = 0; + if (!adb_thread_create(stdin_read_thread, args)) { + PLOG(ERROR) << "error starting stdin read thread"; + exit_code = 1; + delete args; + } else { + exit_code = read_and_dump(fd, use_shell_protocol); + } - adb_thread_create(stdin_read_thread, fds); - read_and_dump(fd); - stdin_raw_restore(fdi); - return 0; + stdin_raw_restore(args->stdin_fd); + return exit_code; } @@ -943,6 +1013,20 @@ static bool _is_valid_ack_reply_fd(const int ack_reply_fd) { #endif } +// 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, + 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 ""; + } + return result; +} + int adb_commandline(int argc, const char **argv) { int no_daemon = 0; int is_daemon = 0; @@ -1156,9 +1240,19 @@ int adb_commandline(int argc, const char **argv) { 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) { D("starting interactive shell"); - r = interactive_shell(); + r = interactive_shell(use_shell_protocol); if (h) { printf("\x1b[0m"); fflush(stdout); @@ -1176,16 +1270,15 @@ int adb_commandline(int argc, const char **argv) { } while (true) { - D("interactive shell loop. cmd=%s", cmd.c_str()); + D("non-interactive shell loop. cmd=%s", cmd.c_str()); std::string error; int fd = adb_connect(cmd, &error); int r; if (fd >= 0) { D("about to read_and_dump(fd=%d)", fd); - read_and_dump(fd); + r = read_and_dump(fd, use_shell_protocol); D("read_and_dump() done."); adb_close(fd); - r = 0; } else { fprintf(stderr,"error: %s\n", error.c_str()); r = -1; @@ -1195,7 +1288,7 @@ int adb_commandline(int argc, const char **argv) { printf("\x1b[0m"); fflush(stdout); } - D("interactive shell loop. return r=%d", r); + D("non-interactive shell loop. return r=%d", r); return r; } } diff --git a/adb/device.py b/adb/device.py index c5b5eea4a..516e88003 100644 --- a/adb/device.py +++ b/adb/device.py @@ -36,6 +36,16 @@ class NoUniqueDeviceError(FindDeviceError): super(NoUniqueDeviceError, self).__init__('No unique device') +class ShellError(RuntimeError): + def __init__(self, cmd, stdout, stderr, exit_code): + super(ShellError, self).__init__( + '`{0}` exited with code {1}'.format(cmd, exit_code)) + self.cmd = cmd + self.stdout = stdout + self.stderr = stderr + self.exit_code = exit_code + + def get_devices(): with open(os.devnull, 'wb') as devnull: subprocess.check_call(['adb', 'start-server'], stdout=devnull, @@ -146,6 +156,9 @@ class AndroidDevice(object): # adb on Windows returns \r\n even if adbd returns \n. _RETURN_CODE_SEARCH_LENGTH = len('{0}255\r\n'.format(_RETURN_CODE_DELIMITER)) + # Shell protocol feature string. + SHELL_PROTOCOL_FEATURE = 'shell_2' + def __init__(self, serial, product=None): self.serial = serial self.product = product @@ -155,6 +168,7 @@ class AndroidDevice(object): if self.product is not None: self.adb_cmd.extend(['-p', product]) self._linesep = None + self._features = None @property def linesep(self): @@ -163,9 +177,20 @@ class AndroidDevice(object): ['shell', 'echo']) return self._linesep + @property + def features(self): + if self._features is None: + try: + self._features = self._simple_call(['features']).splitlines() + except subprocess.CalledProcessError: + self._features = [] + return self._features + def _make_shell_cmd(self, user_cmd): - return (self.adb_cmd + ['shell'] + user_cmd + - ['; ' + self._RETURN_CODE_PROBE_STRING]) + command = self.adb_cmd + ['shell'] + user_cmd + if self.SHELL_PROTOCOL_FEATURE not in self.features: + command.append('; ' + self._RETURN_CODE_PROBE_STRING) + return command def _parse_shell_output(self, out): """Finds the exit code string from shell output. @@ -201,23 +226,43 @@ class AndroidDevice(object): self.adb_cmd + cmd, stderr=subprocess.STDOUT) def shell(self, cmd): - logging.info(' '.join(self.adb_cmd + ['shell'] + cmd)) - cmd = self._make_shell_cmd(cmd) - out = _subprocess_check_output(cmd) - rc, out = self._parse_shell_output(out) - if rc != 0: - error = subprocess.CalledProcessError(rc, cmd) - error.out = out - raise error - return out + """Calls `adb shell` + + Args: + cmd: string shell command to execute. + + Returns: + A (stdout, stderr) tuple. Stderr may be combined into stdout + if the device doesn't support separate streams. + + Raises: + ShellError: the exit code was non-zero. + """ + exit_code, stdout, stderr = self.shell_nocheck(cmd) + if exit_code != 0: + raise ShellError(cmd, stdout, stderr, exit_code) + return stdout, stderr def shell_nocheck(self, cmd): + """Calls `adb shell` + + Args: + cmd: string shell command to execute. + + Returns: + An (exit_code, stdout, stderr) tuple. Stderr may be combined + into stdout if the device doesn't support separate streams. + """ cmd = self._make_shell_cmd(cmd) logging.info(' '.join(cmd)) p = subprocess.Popen( - cmd, stdout=subprocess.PIPE, stderr=subprocess.STDOUT) - out, _ = p.communicate() - return self._parse_shell_output(out) + cmd, stdout=subprocess.PIPE, stderr=subprocess.PIPE) + stdout, stderr = p.communicate() + if self.SHELL_PROTOCOL_FEATURE in self.features: + exit_code = p.returncode + else: + exit_code, stdout = self._parse_shell_output(stdout) + return exit_code, stdout, stderr def install(self, filename, replace=False): cmd = ['install'] @@ -281,7 +326,7 @@ class AndroidDevice(object): return self._simple_call(['wait-for-device']) def get_prop(self, prop_name): - output = self.shell(['getprop', prop_name]).splitlines() + output = self.shell(['getprop', prop_name])[0].splitlines() if len(output) != 1: raise RuntimeError('Too many lines in getprop output:\n' + '\n'.join(output)) diff --git a/adb/test_device.py b/adb/test_device.py index 2006937dd..fedd2d755 100644 --- a/adb/test_device.py +++ b/adb/test_device.py @@ -37,7 +37,7 @@ def requires_root(func): if self.device.get_prop('ro.debuggable') != '1': raise unittest.SkipTest('requires rootable build') - was_root = self.device.shell(['id', '-un']).strip() == 'root' + was_root = self.device.shell(['id', '-un'])[0].strip() == 'root' if not was_root: self.device.root() self.device.wait() @@ -113,7 +113,7 @@ class DeviceTest(unittest.TestCase): class ShellTest(DeviceTest): def test_cat(self): """Check that we can at least cat a file.""" - out = self.device.shell(['cat', '/proc/uptime']).strip() + out = self.device.shell(['cat', '/proc/uptime'])[0].strip() elements = out.split() self.assertEqual(len(elements), 2) @@ -122,20 +122,19 @@ class ShellTest(DeviceTest): self.assertGreater(float(idle), 0.0) def test_throws_on_failure(self): - self.assertRaises(subprocess.CalledProcessError, - self.device.shell, ['false']) + self.assertRaises(adb.ShellError, self.device.shell, ['false']) def test_output_not_stripped(self): - out = self.device.shell(['echo', 'foo']) + out = self.device.shell(['echo', 'foo'])[0] self.assertEqual(out, 'foo' + self.device.linesep) def test_shell_nocheck_failure(self): - rc, out = self.device.shell_nocheck(['false']) + rc, out, _ = self.device.shell_nocheck(['false']) self.assertNotEqual(rc, 0) self.assertEqual(out, '') def test_shell_nocheck_output_not_stripped(self): - rc, out = self.device.shell_nocheck(['echo', 'foo']) + rc, out, _ = self.device.shell_nocheck(['echo', 'foo']) self.assertEqual(rc, 0) self.assertEqual(out, 'foo' + self.device.linesep) @@ -143,7 +142,7 @@ class ShellTest(DeviceTest): # If result checking on ADB shell is naively implemented as # `adb shell ; echo $?`, we would be unable to distinguish the # output from the result for a cmd of `echo -n 1`. - rc, out = self.device.shell_nocheck(['echo', '-n', '1']) + rc, out, _ = self.device.shell_nocheck(['echo', '-n', '1']) self.assertEqual(rc, 0) self.assertEqual(out, '1') @@ -152,7 +151,7 @@ class ShellTest(DeviceTest): Bug: http://b/19735063 """ - output = self.device.shell(['uname']) + output = self.device.shell(['uname'])[0] self.assertEqual(output, 'Linux' + self.device.linesep) def test_pty_logic(self): @@ -180,6 +179,23 @@ class ShellTest(DeviceTest): exit_code = self.device.shell_nocheck(['[ -t 0 ]'])[0] self.assertEqual(exit_code, 1) + def test_shell_protocol(self): + """Tests the shell protocol on the device. + + If the device supports shell protocol, this gives us the ability + to separate stdout/stderr and return the exit code directly. + + Bug: http://b/19734861 + """ + if self.device.SHELL_PROTOCOL_FEATURE not in self.device.features: + raise unittest.SkipTest('shell protocol unsupported on this device') + result = self.device.shell_nocheck( + shlex.split('echo foo; echo bar >&2; exit 17')) + + self.assertEqual(17, result[0]) + self.assertEqual('foo' + self.device.linesep, result[1]) + self.assertEqual('bar' + self.device.linesep, result[2]) + class ArgumentEscapingTest(DeviceTest): def test_shell_escaping(self): @@ -191,25 +207,26 @@ class ArgumentEscapingTest(DeviceTest): # as `sh -c echo` (with an argument to that shell of "hello"), # and then `echo world` back in the first shell. result = self.device.shell( - shlex.split("sh -c 'echo hello; echo world'")) + shlex.split("sh -c 'echo hello; echo world'"))[0] result = result.splitlines() self.assertEqual(['', 'world'], result) # If you really wanted "hello" and "world", here's what you'd do: result = self.device.shell( - shlex.split(r'echo hello\;echo world')).splitlines() + shlex.split(r'echo hello\;echo world'))[0].splitlines() self.assertEqual(['hello', 'world'], result) # http://b/15479704 - result = self.device.shell(shlex.split("'true && echo t'")).strip() + result = self.device.shell(shlex.split("'true && echo t'"))[0].strip() self.assertEqual('t', result) result = self.device.shell( - shlex.split("sh -c 'true && echo t'")).strip() + shlex.split("sh -c 'true && echo t'"))[0].strip() self.assertEqual('t', result) # http://b/20564385 - result = self.device.shell(shlex.split('FOO=a BAR=b echo t')).strip() + result = self.device.shell(shlex.split('FOO=a BAR=b echo t'))[0].strip() self.assertEqual('t', result) - result = self.device.shell(shlex.split(r'echo -n 123\;uname')).strip() + result = self.device.shell( + shlex.split(r'echo -n 123\;uname'))[0].strip() self.assertEqual('123Linux', result) def test_install_argument_escaping(self): @@ -235,19 +252,19 @@ class RootUnrootTest(DeviceTest): if 'adbd cannot run as root in production builds' in message: return self.device.wait() - self.assertEqual('root', self.device.shell(['id', '-un']).strip()) + self.assertEqual('root', self.device.shell(['id', '-un'])[0].strip()) def _test_unroot(self): self.device.unroot() self.device.wait() - self.assertEqual('shell', self.device.shell(['id', '-un']).strip()) + self.assertEqual('shell', self.device.shell(['id', '-un'])[0].strip()) def test_root_unroot(self): """Make sure that adb root and adb unroot work, using id(1).""" if self.device.get_prop('ro.debuggable') != '1': raise unittest.SkipTest('requires rootable build') - original_user = self.device.shell(['id', '-un']).strip() + original_user = self.device.shell(['id', '-un'])[0].strip() try: if original_user == 'root': self._test_unroot() @@ -286,7 +303,7 @@ class SystemPropertiesTest(DeviceTest): self.device.set_prop(prop_name, 'qux') self.assertEqual( - self.device.shell(['getprop', prop_name]).strip(), 'qux') + self.device.shell(['getprop', prop_name])[0].strip(), 'qux') def compute_md5(string): @@ -351,7 +368,7 @@ def make_random_device_files(device, in_dir, num_files): device.shell(['dd', 'if=/dev/urandom', 'of={}'.format(full_path), 'bs={}'.format(size), 'count=1']) - dev_md5, _ = device.shell([get_md5_prog(device), full_path]).split() + dev_md5, _ = device.shell([get_md5_prog(device), full_path])[0].split() files.append(DeviceFile(dev_md5, full_path)) return files @@ -366,7 +383,7 @@ class FileOperationsTest(DeviceTest): self.device.shell(['rm', '-rf', self.DEVICE_TEMP_FILE]) self.device.push(local=local_file, remote=self.DEVICE_TEMP_FILE) dev_md5, _ = self.device.shell([get_md5_prog(self.device), - self.DEVICE_TEMP_FILE]).split() + self.DEVICE_TEMP_FILE])[0].split() self.assertEqual(checksum, dev_md5) self.device.shell(['rm', '-f', self.DEVICE_TEMP_FILE]) @@ -401,7 +418,7 @@ class FileOperationsTest(DeviceTest): 'count={}'.format(kbytes)] self.device.shell(cmd) dev_md5, _ = self.device.shell( - [get_md5_prog(self.device), self.DEVICE_TEMP_FILE]).split() + [get_md5_prog(self.device), self.DEVICE_TEMP_FILE])[0].split() self._test_pull(self.DEVICE_TEMP_FILE, dev_md5) self.device.shell_nocheck(['rm', self.DEVICE_TEMP_FILE]) @@ -449,7 +466,7 @@ class FileOperationsTest(DeviceTest): device_full_path = posixpath.join(self.DEVICE_TEMP_DIR, temp_file.base_name) dev_md5, _ = device.shell( - [get_md5_prog(self.device), device_full_path]).split() + [get_md5_prog(self.device), device_full_path])[0].split() self.assertEqual(temp_file.checksum, dev_md5) self.device.shell(['rm', '-rf', self.DEVICE_TEMP_DIR]) diff --git a/adb/transport.cpp b/adb/transport.cpp index f6334f7bf..db9236edd 100644 --- a/adb/transport.cpp +++ b/adb/transport.cpp @@ -787,7 +787,7 @@ const char kFeatureShell2[] = "shell_2"; // 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 = { - // None yet. + kFeatureShell2, }; const FeatureSet& supported_features() {