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

@@ -31,8 +31,10 @@
 #include <sys/stat.h>
 #include <sys/types.h>
 
+#include <memory>
 #include <string>
 
+#include <base/logging.h>
 #include <base/stringprintf.h>
 
 #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<ShellProtocol> 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<ShellProtocol> 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<StdinReadArgs> args(reinterpret_cast<StdinReadArgs*>(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<int*>(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;
         }
     }

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

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 <cmd>; 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])

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() {