diff --git a/adb/Android.mk b/adb/Android.mk index 67c3eb78c..6d68becef 100644 --- a/adb/Android.mk +++ b/adb/Android.mk @@ -132,8 +132,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/commandline.cpp b/adb/commandline.cpp index db2c90ac1..d9a7eb12b 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); @@ -264,19 +268,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) @@ -370,28 +415,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; @@ -404,47 +462,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; } @@ -951,6 +1021,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; @@ -1164,9 +1248,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); @@ -1184,16 +1278,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; @@ -1203,7 +1296,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/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/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 e97c47986..db9236edd 100644 --- a/adb/transport.cpp +++ b/adb/transport.cpp @@ -779,10 +779,15 @@ 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 = { - // None yet. + kFeatureShell2, }; const FeatureSet& supported_features() { 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