diff --git a/adb/device.py b/adb/device.py index a15675b80..bc1364b6d 100644 --- a/adb/device.py +++ b/adb/device.py @@ -101,6 +101,19 @@ def get_device(serial=None, product=None): class AndroidDevice(object): + # Delimiter string to indicate the start of the exit code. + _RETURN_CODE_DELIMITER = 'x' + + # Follow any shell command with this string to get the exit + # status of a program since this isn't propagated by adb. + # + # The delimiter is needed because `printf 1; echo $?` would print + # "10", and we wouldn't be able to distinguish the exit code. + _RETURN_CODE_PROBE_STRING = 'echo "{0}$?"'.format(_RETURN_CODE_DELIMITER) + + # Maximum search distance from the output end to find the delimiter. + _RETURN_CODE_SEARCH_LENGTH = len('{0}255\n'.format(_RETURN_CODE_DELIMITER)) + def __init__(self, serial, product=None): self.serial = serial self.product = product @@ -110,40 +123,44 @@ class AndroidDevice(object): if self.product is not None: self.adb_cmd.extend(['-p', product]) self._linesep = None - self._shell_result_pattern = None @property def linesep(self): if self._linesep is None: - self._linesep = subprocess.check_output(['adb', 'shell', 'echo']) + self._linesep = subprocess.check_output(self.adb_cmd + + ['shell', 'echo']) return self._linesep def _make_shell_cmd(self, user_cmd): - # Follow any shell command with `; echo; echo $?` to get the exit - # status of a program since this isn't propagated by adb. - # - # The leading newline is needed because `printf 1; echo $?` would print - # "10", and we wouldn't be able to distinguish the exit code. - rc_probe = '; echo "\n$?"' - return self.adb_cmd + ['shell'] + user_cmd + [rc_probe] + return (self.adb_cmd + ['shell'] + user_cmd + + ['; ' + self._RETURN_CODE_PROBE_STRING]) - def _parse_shell_output(self, out): # pylint: disable=no-self-use + def _parse_shell_output(self, out): + """Finds the exit code string from shell output. + + Args: + out: Shell output string. + + Returns: + An (exit_code, output_string) tuple. The output string is + cleaned of any additional stuff we appended to find the + exit code. + + Raises: + RuntimeError: Could not find the exit code in |out|. + """ search_text = out - max_result_len = len('{0}255{0}'.format(self.linesep)) - if len(search_text) > max_result_len: - # We don't want to regex match over massive amounts of data when we - # know the part we want is right at the end. - search_text = search_text[-max_result_len:] - if self._shell_result_pattern is None: - self._shell_result_pattern = re.compile( - r'({0}\d+{0})$'.format(self.linesep), re.MULTILINE) - m = self._shell_result_pattern.search(search_text) - if m is None: + if len(search_text) > self._RETURN_CODE_SEARCH_LENGTH: + # We don't want to search over massive amounts of data when we know + # the part we want is right at the end. + search_text = search_text[-self._RETURN_CODE_SEARCH_LENGTH:] + partition = search_text.rpartition(self._RETURN_CODE_DELIMITER) + if partition[1] == '': raise RuntimeError('Could not find exit status in shell output.') - - result_text = m.group(1) - result = int(result_text.strip()) - out = out[:-len(result_text)] # Trim the result text from the output. + result = int(partition[2]) + # partition[0] won't contain the full text if search_text was truncated, + # pull from the original string instead. + out = out[:-len(partition[1]) - len(partition[2])] return result, out def _simple_call(self, cmd): diff --git a/adb/services.cpp b/adb/services.cpp index 68545d8bd..255be098b 100644 --- a/adb/services.cpp +++ b/adb/services.cpp @@ -59,6 +59,10 @@ struct stinfo { void *cookie; }; +enum class SubprocessType { + kPty, + kRaw, +}; void *service_bootstrap_func(void *x) { @@ -389,17 +393,27 @@ static void subproc_waiter_service(int fd, void *cookie) } } -static int create_subproc_thread(const char *name, bool pty = false) { +// Starts a subprocess and spawns a thread to wait for the subprocess to finish +// and trigger the necessary cleanup. +// +// |name| is the command to execute in the subprocess; empty string will start +// an interactive session. +// |type| selects between a PTY or raw subprocess. +// +// Returns an open file descriptor tied to the subprocess stdin/stdout/stderr. +static int create_subproc_thread(const char *name, SubprocessType type) { const char *arg0, *arg1; - if (name == 0 || *name == 0) { - arg0 = "-"; arg1 = 0; + if (*name == '\0') { + arg0 = "-"; + arg1 = nullptr; } else { - arg0 = "-c"; arg1 = name; + arg0 = "-c"; + arg1 = name; } pid_t pid = -1; int ret_fd; - if (pty) { + if (type == SubprocessType::kPty) { ret_fd = create_subproc_pty(SHELL_COMMAND, arg0, arg1, &pid); } else { ret_fd = create_subproc_raw(SHELL_COMMAND, arg0, arg1, &pid); @@ -466,9 +480,16 @@ int service_to_fd(const char *name) } else if (!strncmp(name, "jdwp:", 5)) { ret = create_jdwp_connection_fd(atoi(name+5)); } else if(!strncmp(name, "shell:", 6)) { - ret = create_subproc_thread(name + 6, true); + const char* args = name + 6; + if (*args) { + // Non-interactive session uses a raw subprocess. + ret = create_subproc_thread(args, SubprocessType::kRaw); + } else { + // Interactive session uses a PTY subprocess. + ret = create_subproc_thread(args, SubprocessType::kPty); + } } else if(!strncmp(name, "exec:", 5)) { - ret = create_subproc_thread(name + 5); + ret = create_subproc_thread(name + 5, SubprocessType::kRaw); } else if(!strncmp(name, "sync:", 5)) { ret = create_service_thread(file_sync_service, NULL); } else if(!strncmp(name, "remount:", 8)) { @@ -482,10 +503,13 @@ int service_to_fd(const char *name) } else if(!strncmp(name, "unroot:", 7)) { ret = create_service_thread(restart_unroot_service, NULL); } else if(!strncmp(name, "backup:", 7)) { - ret = create_subproc_thread(android::base::StringPrintf("/system/bin/bu backup %s", - (name + 7)).c_str()); + ret = create_subproc_thread( + android::base::StringPrintf("/system/bin/bu backup %s", + (name + 7)).c_str(), + SubprocessType::kRaw); } else if(!strncmp(name, "restore:", 8)) { - ret = create_subproc_thread("/system/bin/bu restore"); + ret = create_subproc_thread("/system/bin/bu restore", + SubprocessType::kRaw); } else if(!strncmp(name, "tcpip:", 6)) { int port; if (sscanf(name + 6, "%d", &port) != 1) { diff --git a/adb/test_device.py b/adb/test_device.py index 48a3f6c30..c893ad42d 100644 --- a/adb/test_device.py +++ b/adb/test_device.py @@ -155,6 +155,31 @@ class ShellTest(DeviceTest): output = self.device.shell(['uname']) self.assertEqual(output, 'Linux' + self.device.linesep) + def test_pty_logic(self): + """Verify PTY logic for shells. + + Interactive shells should use a PTY, non-interactive should not. + + Bug: http://b/21215503 + """ + proc = subprocess.Popen( + self.device.adb_cmd + ['shell'], stdin=subprocess.PIPE, + stdout=subprocess.PIPE, stderr=subprocess.STDOUT) + # [ -t 0 ] is used (rather than `tty`) to provide portability. This + # gives an exit code of 0 iff stdin is connected to a terminal. + # + # Closing host-side stdin doesn't currently trigger the interactive + # shell to exit so we need to explicitly add an exit command to + # close the session from the device side, and append \n to complete + # the interactive command. + result = proc.communicate('[ -t 0 ]; echo x$?; exit 0\n')[0] + partition = result.rpartition('x') + self.assertEqual(partition[1], 'x') + self.assertEqual(int(partition[2]), 0) + + exit_code = self.device.shell_nocheck(['[ -t 0 ]'])[0] + self.assertEqual(exit_code, 1) + class ArgumentEscapingTest(DeviceTest): def test_shell_escaping(self):