Donate to e Foundation | Murena handsets with /e/OS | Own a part of Murena! Learn more

Commit d4093f19 authored by David Pursell's avatar David Pursell
Browse files

adb: Fix PTY logic for non-interactive shells. 

Change `adb shell` so that interactive sessions use a PTY but
non-interactive do not. This matches `ssh` functionality better
and also enables future work to split stdout/stderr for
non-interactive sessions.

A test to verify this behavior is added to test_device.py with
supporting modifications in device.py.

Bug: http://b/21215503
Change-Id: Ib4ba40df85f82ddef4e0dd557952271c859d1c7b
parent 317acfb8

adb/device.py

+42 −25
Original line number Diff line number Diff line
@@ -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):

        """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.



    def _parse_shell_output(self, out):  # pylint: disable=no-self-use

        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):

adb/services.cpp

+34 −10
Original line number Diff line number Diff line
@@ -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) {

adb/test_device.py

+25 −0
Original line number Diff line number Diff line
@@ -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):