From 9b6522b357a8c8f37a84e983558925b5a0d72dd1 Mon Sep 17 00:00:00 2001 From: Josh Gao Date: Tue, 7 Aug 2018 14:31:17 -0700 Subject: [PATCH 1/6] adb: switch test_adb.py to python3. Currently, test_adb.py doesn't work on Windows, because we're selecting on a pipe, which doesn't work on Windows. Python 3.5 adds socketpair support to Windows, so switch over to Python 3 to make it so we can use that. Test: python3 test_adb.py Change-Id: I0fbd1bca0b28324658831d5ef8ee08aefe2b0596 --- adb/Android.bp | 7 +-- adb/test_adb.py | 139 +++++++++++++++++++++++++----------------------- 2 files changed, 75 insertions(+), 71 deletions(-) mode change 100644 => 100755 adb/test_adb.py diff --git a/adb/Android.bp b/adb/Android.bp index dabc5ce60..a18dc1ab8 100644 --- a/adb/Android.bp +++ b/adb/Android.bp @@ -455,17 +455,14 @@ python_test_host { srcs: [ "test_adb.py", ], - libs: [ - "adb_py", - ], test_config: "adb_integration_test_adb.xml", test_suites: ["general-tests"], version: { py2: { - enabled: true, + enabled: false, }, py3: { - enabled: false, + enabled: true, }, }, } diff --git a/adb/test_adb.py b/adb/test_adb.py old mode 100644 new mode 100755 index ddd3ff041..173251a2f --- a/adb/test_adb.py +++ b/adb/test_adb.py @@ -1,4 +1,4 @@ -#!/usr/bin/env python +#!/usr/bin/env python3 # # Copyright (C) 2015 The Android Open Source Project # @@ -19,9 +19,7 @@ This differs from things in test_device.py in that there is no API for these things. Most of these tests involve specific error messages or the help text. """ -from __future__ import print_function -import binascii import contextlib import os import random @@ -32,8 +30,6 @@ import subprocess import threading import unittest -import adb - @contextlib.contextmanager def fake_adbd(protocol=socket.AF_INET, port=0): @@ -50,47 +46,51 @@ def fake_adbd(protocol=socket.AF_INET, port=0): # A pipe that is used to signal the thread that it should terminate. readpipe, writepipe = os.pipe() - def _adb_packet(command, arg0, arg1, data): + def _adb_packet(command: bytes, arg0: int, arg1: int, data: bytes) -> bytes: bin_command = struct.unpack('I', command)[0] buf = struct.pack('IIIIII', bin_command, arg0, arg1, len(data), 0, bin_command ^ 0xffffffff) buf += data return buf - def _handle(): - rlist = [readpipe, serversock] - cnxn_sent = {} - while True: - read_ready, _, _ = select.select(rlist, [], []) - for ready in read_ready: - if ready == readpipe: - # Closure pipe - os.close(ready) - serversock.shutdown(socket.SHUT_RDWR) - serversock.close() - return - elif ready == serversock: - # Server socket - conn, _ = ready.accept() - rlist.append(conn) - else: - # Client socket - data = ready.recv(1024) - if not data or data.startswith('OPEN'): + def _handle(sock): + with contextlib.closing(sock) as serversock: + rlist = [readpipe, serversock] + cnxn_sent = {} + while True: + read_ready, _, _ = select.select(rlist, [], []) + for ready in read_ready: + if ready == readpipe: + # Closure pipe + serversock.shutdown(socket.SHUT_RDWR) + for f in rlist: + if isinstance(f, int): + os.close(f) + else: + f.close() + return + elif ready == serversock: + # Server socket + conn, _ = ready.accept() + rlist.append(conn) + else: + # Client socket + data = ready.recv(1024) + if not data or data.startswith(b'OPEN'): + if ready in cnxn_sent: + del cnxn_sent[ready] + ready.shutdown(socket.SHUT_RDWR) + ready.close() + rlist.remove(ready) + continue if ready in cnxn_sent: - del cnxn_sent[ready] - ready.shutdown(socket.SHUT_RDWR) - ready.close() - rlist.remove(ready) - continue - if ready in cnxn_sent: - continue - cnxn_sent[ready] = True - ready.sendall(_adb_packet('CNXN', 0x01000001, 1024 * 1024, - 'device::ro.product.name=fakeadb')) + continue + cnxn_sent[ready] = True + ready.sendall(_adb_packet(b'CNXN', 0x01000001, 1024 * 1024, + b'device::ro.product.name=fakeadb')) port = serversock.getsockname()[1] - server_thread = threading.Thread(target=_handle) + server_thread = threading.Thread(target=_handle, args=(serversock,)) server_thread.start() try: @@ -108,7 +108,8 @@ def adb_connect(unittest, serial): """ output = subprocess.check_output(['adb', 'connect', serial]) - unittest.assertEqual(output.strip(), 'connected to {}'.format(serial)) + unittest.assertEqual(output.strip(), + 'connected to {}'.format(serial).encode("utf8")) try: yield @@ -131,13 +132,14 @@ def adb_server(): subprocess.check_output(['adb', '-P', str(port), 'kill-server'], stderr=subprocess.STDOUT) read_pipe, write_pipe = os.pipe() + os.set_inheritable(write_pipe, True) proc = subprocess.Popen(['adb', '-L', 'tcp:localhost:{}'.format(port), 'fork-server', 'server', - '--reply-fd', str(write_pipe)]) + '--reply-fd', str(write_pipe)], close_fds=False) try: os.close(write_pipe) greeting = os.read(read_pipe, 1024) - assert greeting == 'OK\n', repr(greeting) + assert greeting == b'OK\n', repr(greeting) yield port finally: proc.terminate() @@ -157,28 +159,30 @@ class CommandlineTest(unittest.TestCase): """Get a version number out of the output of adb.""" lines = subprocess.check_output(['adb', 'version']).splitlines() version_line = lines[0] - self.assertRegexpMatches( - version_line, r'^Android Debug Bridge version \d+\.\d+\.\d+$') + self.assertRegex( + version_line, rb'^Android Debug Bridge version \d+\.\d+\.\d+$') if len(lines) == 2: # Newer versions of ADB have a second line of output for the # version that includes a specific revision (git SHA). revision_line = lines[1] - self.assertRegexpMatches( - revision_line, r'^Revision [0-9a-f]{12}-android$') + self.assertRegex( + revision_line, rb'^Revision [0-9a-f]{12}-android$') def test_tcpip_error_messages(self): """Make sure 'adb tcpip' parsing is sane.""" - proc = subprocess.Popen(['adb', 'tcpip'], stdout=subprocess.PIPE, + proc = subprocess.Popen(['adb', 'tcpip'], + stdout=subprocess.PIPE, stderr=subprocess.STDOUT) out, _ = proc.communicate() self.assertEqual(1, proc.returncode) - self.assertIn('requires an argument', out) + self.assertIn(b'requires an argument', out) - proc = subprocess.Popen(['adb', 'tcpip', 'foo'], stdout=subprocess.PIPE, + proc = subprocess.Popen(['adb', 'tcpip', 'foo'], + stdout=subprocess.PIPE, stderr=subprocess.STDOUT) out, _ = proc.communicate() self.assertEqual(1, proc.returncode) - self.assertIn('invalid port', out) + self.assertIn(b'invalid port', out) class ServerTest(unittest.TestCase): @@ -229,14 +233,12 @@ class ServerTest(unittest.TestCase): stdout_thread = threading.Thread( target=ServerTest._read_pipe_and_set_event, args=(proc.stdout, stdout_event)) - stdout_thread.daemon = True stdout_thread.start() stderr_event = threading.Event() stderr_thread = threading.Thread( target=ServerTest._read_pipe_and_set_event, args=(proc.stderr, stderr_event)) - stderr_thread.daemon = True stderr_thread.start() # Wait for the adb client to finish. Once that has occurred, if @@ -250,7 +252,8 @@ class ServerTest(unittest.TestCase): # probably letting the adb server inherit stdin which would be # wrong. with self.assertRaises(IOError): - proc.stdin.write('x') + proc.stdin.write(b'x') + proc.stdin.flush() # Wait a few seconds for stdout/stderr to be closed (in the success # case, this won't wait at all). If there is a timeout, that means @@ -259,6 +262,8 @@ class ServerTest(unittest.TestCase): # inherit stdout/stderr which would be wrong. self.assertTrue(stdout_event.wait(5), "adb stdout not closed") self.assertTrue(stderr_event.wait(5), "adb stderr not closed") + stdout_thread.join() + stderr_thread.join() finally: # If we started a server, kill it. subprocess.check_output(['adb', '-P', str(port), 'kill-server'], @@ -306,9 +311,9 @@ class EmulatorTest(unittest.TestCase): with contextlib.closing(accepted_connection) as conn: # If WSAECONNABORTED (10053) is raised by any socket calls, # then adb probably isn't reading the data that we sent it. - conn.sendall('Android Console: type \'help\' for a list ' + - 'of commands\r\n') - conn.sendall('OK\r\n') + conn.sendall(('Android Console: type \'help\' for a list ' + 'of commands\r\n').encode("utf8")) + conn.sendall(b'OK\r\n') with contextlib.closing(conn.makefile()) as connf: line = connf.readline() @@ -318,7 +323,7 @@ class EmulatorTest(unittest.TestCase): self.assertEqual('kill\n', line) self.assertEqual('quit\n', connf.readline()) - conn.sendall('OK: killing emulator, bye bye\r\n') + conn.sendall(b'OK: killing emulator, bye bye\r\n') # Use SO_LINGER to send TCP RST segment to test whether adb # ignores WSAECONNRESET on Windows. This happens with the @@ -352,21 +357,21 @@ class EmulatorTest(unittest.TestCase): except subprocess.CalledProcessError as err: self.assertEqual( err.output.strip(), - 'error: device \'{}\' not found'.format(serial)) + 'error: device \'{}\' not found'.format(serial).encode("utf8")) # Let the ADB server know that the emulator has started. with contextlib.closing( - socket.socket(socket.AF_INET, socket.SOCK_STREAM)) as sock: + socket.socket(socket.AF_INET, socket.SOCK_STREAM)) as sock: sock.connect(('localhost', server_port)) - command = 'host:emulator:{}'.format(port) - sock.sendall('%04x%s' % (len(command), command)) + command = 'host:emulator:{}'.format(port).encode("utf8") + sock.sendall(b'%04x%s' % (len(command), command)) # Ensure the emulator is there. subprocess.check_call(['adb', '-P', str(server_port), '-s', serial, 'wait-for-device']) output = subprocess.check_output(['adb', '-P', str(server_port), '-s', serial, 'get-state']) - self.assertEqual(output.strip(), 'device') + self.assertEqual(output.strip(), b'device') class ConnectionTest(unittest.TestCase): @@ -396,7 +401,8 @@ class ConnectionTest(unittest.TestCase): # b/31250450: this always returns 0 but probably shouldn't. output = subprocess.check_output(['adb', 'connect', serial]) self.assertEqual( - output.strip(), 'already connected to {}'.format(serial)) + output.strip(), + 'already connected to {}'.format(serial).encode("utf8")) def test_reconnect(self): """Ensure that a disconnected device reconnects.""" @@ -406,26 +412,27 @@ class ConnectionTest(unittest.TestCase): with adb_connect(self, serial): output = subprocess.check_output(['adb', '-s', serial, 'get-state']) - self.assertEqual(output.strip(), 'device') + self.assertEqual(output.strip(), b'device') # This will fail. proc = subprocess.Popen(['adb', '-s', serial, 'shell', 'true'], stdout=subprocess.PIPE, stderr=subprocess.STDOUT) output, _ = proc.communicate() - self.assertEqual(output.strip(), 'error: closed') + self.assertEqual(output.strip(), b'error: closed') subprocess.check_call(['adb', '-s', serial, 'wait-for-device']) output = subprocess.check_output(['adb', '-s', serial, 'get-state']) - self.assertEqual(output.strip(), 'device') + self.assertEqual(output.strip(), b'device') # Once we explicitly kick a device, it won't attempt to # reconnect. output = subprocess.check_output(['adb', 'disconnect', serial]) self.assertEqual( - output.strip(), 'disconnected {}'.format(serial)) + output.strip(), + 'disconnected {}'.format(serial).encode("utf8")) try: subprocess.check_output(['adb', '-s', serial, 'get-state'], stderr=subprocess.STDOUT) @@ -433,7 +440,7 @@ class ConnectionTest(unittest.TestCase): except subprocess.CalledProcessError as err: self.assertEqual( err.output.strip(), - 'error: device \'{}\' not found'.format(serial)) + 'error: device \'{}\' not found'.format(serial).encode("utf8")) def main(): From 676f375d9b3a8fa74149dd511f38884e0887ce5c Mon Sep 17 00:00:00 2001 From: Josh Gao Date: Tue, 7 Aug 2018 16:07:25 -0700 Subject: [PATCH 2/6] adb: make test_adb.py work on windows. Switch from os.pipe to socket.socketpair for the fake adbd termination termination signaller, that we can select on it on windows. Test: test_adb.py on windows (2 failures on windows) Change-Id: I37df06e465ec8be28cfb18a5c21ef30125195004 --- adb/test_adb.py | 14 +++++--------- 1 file changed, 5 insertions(+), 9 deletions(-) diff --git a/adb/test_adb.py b/adb/test_adb.py index 173251a2f..f82617a9b 100755 --- a/adb/test_adb.py +++ b/adb/test_adb.py @@ -44,7 +44,7 @@ def fake_adbd(protocol=socket.AF_INET, port=0): serversock.listen(1) # A pipe that is used to signal the thread that it should terminate. - readpipe, writepipe = os.pipe() + readsock, writesock = socket.socketpair() def _adb_packet(command: bytes, arg0: int, arg1: int, data: bytes) -> bytes: bin_command = struct.unpack('I', command)[0] @@ -55,19 +55,15 @@ def fake_adbd(protocol=socket.AF_INET, port=0): def _handle(sock): with contextlib.closing(sock) as serversock: - rlist = [readpipe, serversock] + rlist = [readsock, serversock] cnxn_sent = {} while True: read_ready, _, _ = select.select(rlist, [], []) for ready in read_ready: - if ready == readpipe: + if ready == readsock: # Closure pipe - serversock.shutdown(socket.SHUT_RDWR) for f in rlist: - if isinstance(f, int): - os.close(f) - else: - f.close() + f.close() return elif ready == serversock: # Server socket @@ -96,7 +92,7 @@ def fake_adbd(protocol=socket.AF_INET, port=0): try: yield port finally: - os.close(writepipe) + writesock.close() server_thread.join() From b3610734f8d3d8ed369a18afe2510adee5876a25 Mon Sep 17 00:00:00 2001 From: Josh Gao Date: Wed, 8 Aug 2018 13:08:08 -0700 Subject: [PATCH 3/6] adb: switch test_adb.py over to double quotes. Test: ./test_adb.py Change-Id: I3a568361d54f32cc895cea439de0f2c38aee5e2d --- adb/test_adb.py | 142 ++++++++++++++++++++++++------------------------ 1 file changed, 71 insertions(+), 71 deletions(-) diff --git a/adb/test_adb.py b/adb/test_adb.py index f82617a9b..d4c98e413 100755 --- a/adb/test_adb.py +++ b/adb/test_adb.py @@ -38,17 +38,17 @@ def fake_adbd(protocol=socket.AF_INET, port=0): serversock = socket.socket(protocol, socket.SOCK_STREAM) serversock.setsockopt(socket.SOL_SOCKET, socket.SO_REUSEADDR, 1) if protocol == socket.AF_INET: - serversock.bind(('127.0.0.1', port)) + serversock.bind(("127.0.0.1", port)) else: - serversock.bind(('::1', port)) + serversock.bind(("::1", port)) serversock.listen(1) # A pipe that is used to signal the thread that it should terminate. readsock, writesock = socket.socketpair() def _adb_packet(command: bytes, arg0: int, arg1: int, data: bytes) -> bytes: - bin_command = struct.unpack('I', command)[0] - buf = struct.pack('IIIIII', bin_command, arg0, arg1, len(data), 0, + bin_command = struct.unpack("I", command)[0] + buf = struct.pack("IIIIII", bin_command, arg0, arg1, len(data), 0, bin_command ^ 0xffffffff) buf += data return buf @@ -72,7 +72,7 @@ def fake_adbd(protocol=socket.AF_INET, port=0): else: # Client socket data = ready.recv(1024) - if not data or data.startswith(b'OPEN'): + if not data or data.startswith(b"OPEN"): if ready in cnxn_sent: del cnxn_sent[ready] ready.shutdown(socket.SHUT_RDWR) @@ -82,8 +82,8 @@ def fake_adbd(protocol=socket.AF_INET, port=0): if ready in cnxn_sent: continue cnxn_sent[ready] = True - ready.sendall(_adb_packet(b'CNXN', 0x01000001, 1024 * 1024, - b'device::ro.product.name=fakeadb')) + ready.sendall(_adb_packet(b"CNXN", 0x01000001, 1024 * 1024, + b"device::ro.product.name=fakeadb")) port = serversock.getsockname()[1] server_thread = threading.Thread(target=_handle, args=(serversock,)) @@ -103,15 +103,15 @@ def adb_connect(unittest, serial): This automatically disconnects when done with the connection. """ - output = subprocess.check_output(['adb', 'connect', serial]) + output = subprocess.check_output(["adb", "connect", serial]) unittest.assertEqual(output.strip(), - 'connected to {}'.format(serial).encode("utf8")) + "connected to {}".format(serial).encode("utf8")) try: yield finally: # Perform best-effort disconnection. Discard the output. - subprocess.Popen(['adb', 'disconnect', serial], + subprocess.Popen(["adb", "disconnect", serial], stdout=subprocess.PIPE, stderr=subprocess.PIPE).communicate() @@ -120,22 +120,22 @@ def adb_connect(unittest, serial): def adb_server(): """Context manager for an ADB server. - This creates an ADB server and returns the port it's listening on. + This creates an ADB server and returns the port it"s listening on. """ port = 5038 # Kill any existing server on this non-default port. - subprocess.check_output(['adb', '-P', str(port), 'kill-server'], + subprocess.check_output(["adb", "-P", str(port), "kill-server"], stderr=subprocess.STDOUT) read_pipe, write_pipe = os.pipe() os.set_inheritable(write_pipe, True) - proc = subprocess.Popen(['adb', '-L', 'tcp:localhost:{}'.format(port), - 'fork-server', 'server', - '--reply-fd', str(write_pipe)], close_fds=False) + proc = subprocess.Popen(["adb", "-L", "tcp:localhost:{}".format(port), + "fork-server", "server", + "--reply-fd", str(write_pipe)], close_fds=False) try: os.close(write_pipe) greeting = os.read(read_pipe, 1024) - assert greeting == b'OK\n', repr(greeting) + assert greeting == b"OK\n", repr(greeting) yield port finally: proc.terminate() @@ -148,37 +148,37 @@ class CommandlineTest(unittest.TestCase): def test_help(self): """Make sure we get _something_ out of help.""" out = subprocess.check_output( - ['adb', 'help'], stderr=subprocess.STDOUT) + ["adb", "help"], stderr=subprocess.STDOUT) self.assertGreater(len(out), 0) def test_version(self): """Get a version number out of the output of adb.""" - lines = subprocess.check_output(['adb', 'version']).splitlines() + lines = subprocess.check_output(["adb", "version"]).splitlines() version_line = lines[0] self.assertRegex( - version_line, rb'^Android Debug Bridge version \d+\.\d+\.\d+$') + version_line, rb"^Android Debug Bridge version \d+\.\d+\.\d+$") if len(lines) == 2: # Newer versions of ADB have a second line of output for the # version that includes a specific revision (git SHA). revision_line = lines[1] self.assertRegex( - revision_line, rb'^Revision [0-9a-f]{12}-android$') + revision_line, rb"^Revision [0-9a-f]{12}-android$") def test_tcpip_error_messages(self): """Make sure 'adb tcpip' parsing is sane.""" - proc = subprocess.Popen(['adb', 'tcpip'], + proc = subprocess.Popen(["adb", "tcpip"], stdout=subprocess.PIPE, stderr=subprocess.STDOUT) out, _ = proc.communicate() self.assertEqual(1, proc.returncode) - self.assertIn(b'requires an argument', out) + self.assertIn(b"requires an argument", out) - proc = subprocess.Popen(['adb', 'tcpip', 'foo'], + proc = subprocess.Popen(["adb", "tcpip", "foo"], stdout=subprocess.PIPE, stderr=subprocess.STDOUT) out, _ = proc.communicate() self.assertEqual(1, proc.returncode) - self.assertIn(b'invalid port', out) + self.assertIn(b"invalid port", out) class ServerTest(unittest.TestCase): @@ -214,12 +214,12 @@ class ServerTest(unittest.TestCase): port = 5038 # Kill any existing server on this non-default port. - subprocess.check_output(['adb', '-P', str(port), 'kill-server'], + subprocess.check_output(["adb", "-P", str(port), "kill-server"], stderr=subprocess.STDOUT) try: # Run the adb client and have it start the adb server. - proc = subprocess.Popen(['adb', '-P', str(port), 'start-server'], + proc = subprocess.Popen(["adb", "-P", str(port), "start-server"], stdin=subprocess.PIPE, stdout=subprocess.PIPE, stderr=subprocess.PIPE) @@ -248,7 +248,7 @@ class ServerTest(unittest.TestCase): # probably letting the adb server inherit stdin which would be # wrong. with self.assertRaises(IOError): - proc.stdin.write(b'x') + proc.stdin.write(b"x") proc.stdin.flush() # Wait a few seconds for stdout/stderr to be closed (in the success @@ -262,7 +262,7 @@ class ServerTest(unittest.TestCase): stderr_thread.join() finally: # If we started a server, kill it. - subprocess.check_output(['adb', '-P', str(port), 'kill-server'], + subprocess.check_output(["adb", "-P", str(port), "kill-server"], stderr=subprocess.STDOUT) @@ -272,7 +272,7 @@ class EmulatorTest(unittest.TestCase): def _reset_socket_on_close(self, sock): """Use SO_LINGER to cause TCP RST segment to be sent on socket close.""" # The linger structure is two shorts on Windows, but two ints on Unix. - linger_format = 'hh' if os.name == 'nt' else 'ii' + linger_format = "hh" if os.name == "nt" else "ii" l_onoff = 1 l_linger = 0 @@ -293,33 +293,33 @@ class EmulatorTest(unittest.TestCase): # Use SO_REUSEADDR so subsequent runs of the test can grab the port # even if it is in TIME_WAIT. listener.setsockopt(socket.SOL_SOCKET, socket.SO_REUSEADDR, 1) - listener.bind(('127.0.0.1', 0)) + listener.bind(("127.0.0.1", 0)) listener.listen(4) port = listener.getsockname()[1] # Now that listening has started, start adb emu kill, telling it to # connect to our mock emulator. proc = subprocess.Popen( - ['adb', '-s', 'emulator-' + str(port), 'emu', 'kill'], + ["adb", "-s", "emulator-" + str(port), "emu", "kill"], stderr=subprocess.STDOUT) accepted_connection, addr = listener.accept() with contextlib.closing(accepted_connection) as conn: # If WSAECONNABORTED (10053) is raised by any socket calls, # then adb probably isn't reading the data that we sent it. - conn.sendall(('Android Console: type \'help\' for a list ' - 'of commands\r\n').encode("utf8")) - conn.sendall(b'OK\r\n') + conn.sendall(("Android Console: type 'help' for a list " + "of commands\r\n").encode("utf8")) + conn.sendall(b"OK\r\n") with contextlib.closing(conn.makefile()) as connf: line = connf.readline() - if line.startswith('auth'): + if line.startswith("auth"): # Ignore the first auth line. line = connf.readline() - self.assertEqual('kill\n', line) - self.assertEqual('quit\n', connf.readline()) + self.assertEqual("kill\n", line) + self.assertEqual("quit\n", connf.readline()) - conn.sendall(b'OK: killing emulator, bye bye\r\n') + conn.sendall(b"OK: killing emulator, bye bye\r\n") # Use SO_LINGER to send TCP RST segment to test whether adb # ignores WSAECONNRESET on Windows. This happens with the @@ -343,31 +343,31 @@ class EmulatorTest(unittest.TestCase): """ with adb_server() as server_port: with fake_adbd() as port: - serial = 'emulator-{}'.format(port - 1) + serial = "emulator-{}".format(port - 1) # Ensure that the emulator is not there. try: - subprocess.check_output(['adb', '-P', str(server_port), - '-s', serial, 'get-state'], + subprocess.check_output(["adb", "-P", str(server_port), + "-s", serial, "get-state"], stderr=subprocess.STDOUT) - self.fail('Device should not be available') + self.fail("Device should not be available") except subprocess.CalledProcessError as err: self.assertEqual( err.output.strip(), - 'error: device \'{}\' not found'.format(serial).encode("utf8")) + "error: device '{}' not found".format(serial).encode("utf8")) # Let the ADB server know that the emulator has started. with contextlib.closing( socket.socket(socket.AF_INET, socket.SOCK_STREAM)) as sock: - sock.connect(('localhost', server_port)) - command = 'host:emulator:{}'.format(port).encode("utf8") - sock.sendall(b'%04x%s' % (len(command), command)) + sock.connect(("localhost", server_port)) + command = "host:emulator:{}".format(port).encode("utf8") + sock.sendall(b"%04x%s" % (len(command), command)) # Ensure the emulator is there. - subprocess.check_call(['adb', '-P', str(server_port), - '-s', serial, 'wait-for-device']) - output = subprocess.check_output(['adb', '-P', str(server_port), - '-s', serial, 'get-state']) - self.assertEqual(output.strip(), b'device') + subprocess.check_call(["adb", "-P", str(server_port), + "-s", serial, "wait-for-device"]) + output = subprocess.check_output(["adb", "-P", str(server_port), + "-s", serial, "get-state"]) + self.assertEqual(output.strip(), b"device") class ConnectionTest(unittest.TestCase): @@ -381,7 +381,7 @@ class ConnectionTest(unittest.TestCase): for protocol in (socket.AF_INET, socket.AF_INET6): try: with fake_adbd(protocol=protocol) as port: - serial = 'localhost:{}'.format(port) + serial = "localhost:{}".format(port) with adb_connect(self, serial): pass except socket.error: @@ -392,51 +392,51 @@ class ConnectionTest(unittest.TestCase): """Ensure that an already-connected device stays connected.""" with fake_adbd() as port: - serial = 'localhost:{}'.format(port) + serial = "localhost:{}".format(port) with adb_connect(self, serial): # b/31250450: this always returns 0 but probably shouldn't. - output = subprocess.check_output(['adb', 'connect', serial]) + output = subprocess.check_output(["adb", "connect", serial]) self.assertEqual( output.strip(), - 'already connected to {}'.format(serial).encode("utf8")) + "already connected to {}".format(serial).encode("utf8")) def test_reconnect(self): """Ensure that a disconnected device reconnects.""" with fake_adbd() as port: - serial = 'localhost:{}'.format(port) + serial = "localhost:{}".format(port) with adb_connect(self, serial): - output = subprocess.check_output(['adb', '-s', serial, - 'get-state']) - self.assertEqual(output.strip(), b'device') + output = subprocess.check_output(["adb", "-s", serial, + "get-state"]) + self.assertEqual(output.strip(), b"device") # This will fail. - proc = subprocess.Popen(['adb', '-s', serial, 'shell', 'true'], + proc = subprocess.Popen(["adb", "-s", serial, "shell", "true"], stdout=subprocess.PIPE, stderr=subprocess.STDOUT) output, _ = proc.communicate() - self.assertEqual(output.strip(), b'error: closed') + self.assertEqual(output.strip(), b"error: closed") - subprocess.check_call(['adb', '-s', serial, 'wait-for-device']) + subprocess.check_call(["adb", "-s", serial, "wait-for-device"]) - output = subprocess.check_output(['adb', '-s', serial, - 'get-state']) - self.assertEqual(output.strip(), b'device') + output = subprocess.check_output(["adb", "-s", serial, + "get-state"]) + self.assertEqual(output.strip(), b"device") # Once we explicitly kick a device, it won't attempt to # reconnect. - output = subprocess.check_output(['adb', 'disconnect', serial]) + output = subprocess.check_output(["adb", "disconnect", serial]) self.assertEqual( output.strip(), - 'disconnected {}'.format(serial).encode("utf8")) + "disconnected {}".format(serial).encode("utf8")) try: - subprocess.check_output(['adb', '-s', serial, 'get-state'], + subprocess.check_output(["adb", "-s", serial, "get-state"], stderr=subprocess.STDOUT) - self.fail('Device should not be available') + self.fail("Device should not be available") except subprocess.CalledProcessError as err: self.assertEqual( err.output.strip(), - 'error: device \'{}\' not found'.format(serial).encode("utf8")) + "error: device '{}' not found".format(serial).encode("utf8")) def main(): @@ -445,5 +445,5 @@ def main(): unittest.main(verbosity=3) -if __name__ == '__main__': +if __name__ == "__main__": main() From 6c1b42ccd0bc5a65393caf7f9a9091c513fb373a Mon Sep 17 00:00:00 2001 From: Josh Gao Date: Wed, 8 Aug 2018 14:25:41 -0700 Subject: [PATCH 4/6] adb: fix test_device.FileOperationsTest.test_push_empty Test: python test_device.py Change-Id: Iaa5006e39c3d61374de11c05bcaffb4f30b7512e --- adb/test_device.py | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/adb/test_device.py b/adb/test_device.py index 4abe7a7d6..c8a4d9bf9 100644 --- a/adb/test_device.py +++ b/adb/test_device.py @@ -762,9 +762,10 @@ class FileOperationsTest(DeviceTest): os.chmod(host_dir, 0o700) # Create an empty directory. - os.mkdir(os.path.join(host_dir, 'empty')) + empty_dir_path = os.path.join(host_dir, 'empty') + os.mkdir(empty_dir_path); - self.device.push(host_dir, self.DEVICE_TEMP_DIR) + self.device.push(empty_dir_path, self.DEVICE_TEMP_DIR) test_empty_cmd = ['[', '-d', os.path.join(self.DEVICE_TEMP_DIR, 'empty')] From a8db274a697426aad7d77e71da59b97adcdad0bc Mon Sep 17 00:00:00 2001 From: Josh Gao Date: Wed, 8 Aug 2018 14:26:03 -0700 Subject: [PATCH 5/6] adb: disable test_device.FileOperationsTest.test_pull_symlink_dir. selinux prevents us from doing this. Test: python test_device.py Change-Id: I7117dd19348b0764638977b7c958331a4839ea4f --- adb/test_device.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/adb/test_device.py b/adb/test_device.py index c8a4d9bf9..42aadc484 100644 --- a/adb/test_device.py +++ b/adb/test_device.py @@ -1033,7 +1033,8 @@ class FileOperationsTest(DeviceTest): if host_dir is not None: shutil.rmtree(host_dir) - def test_pull_symlink_dir(self): + # selinux prevents adbd from accessing symlinks on /data/local/tmp. + def disabled_test_pull_symlink_dir(self): """Pull a symlink to a directory of symlinks to files.""" try: host_dir = tempfile.mkdtemp() From 362e696bbff91cfd277b5019bbe08c319a49c442 Mon Sep 17 00:00:00 2001 From: Josh Gao Date: Wed, 8 Aug 2018 16:20:14 -0700 Subject: [PATCH 6/6] adb: report connection status when we're unauthorized. Previously, connecting to devices that end up as unauthorized would wait 10 seconds before reporting failure to the user. After this change, notification happens as soon as the adb server realizes. Test: manual Change-Id: If7c8d38f22da3d98b952eee6a334abc8566bb751 --- adb/client/auth.cpp | 1 + adb/transport.cpp | 27 ++++++++++++++++++++------- adb/transport.h | 4 ++-- adb/transport_local.cpp | 10 ++++++---- 4 files changed, 29 insertions(+), 13 deletions(-) diff --git a/adb/client/auth.cpp b/adb/client/auth.cpp index 5fbef0994..71c19b82d 100644 --- a/adb/client/auth.cpp +++ b/adb/client/auth.cpp @@ -465,6 +465,7 @@ void send_auth_response(const char* token, size_t token_size, atransport* t) { if (key == nullptr) { // No more private keys to try, send the public key. t->SetConnectionState(kCsUnauthorized); + t->SetConnectionEstablished(true); send_auth_publickey(t); return; } diff --git a/adb/transport.cpp b/adb/transport.cpp index 3c74c7591..0e8db04c6 100644 --- a/adb/transport.cpp +++ b/adb/transport.cpp @@ -1190,14 +1190,15 @@ void close_usb_devices() { } #endif // ADB_HOST -int register_socket_transport(unique_fd s, std::string serial, int port, int local, - atransport::ReconnectCallback reconnect) { +bool register_socket_transport(unique_fd s, std::string serial, int port, int local, + atransport::ReconnectCallback reconnect, int* error) { atransport* t = new atransport(std::move(reconnect), kCsOffline); D("transport: %s init'ing for socket %d, on port %d", serial.c_str(), s.get(), port); if (init_socket_transport(t, std::move(s), port, local) < 0) { delete t; - return -1; + if (error) *error = errno; + return false; } std::unique_lock lock(transport_lock); @@ -1206,7 +1207,8 @@ int register_socket_transport(unique_fd s, std::string serial, int port, int loc VLOG(TRANSPORT) << "socket transport " << transport->serial << " is already in pending_list and fails to register"; delete t; - return -EALREADY; + if (error) *error = EALREADY; + return false; } } @@ -1215,7 +1217,8 @@ int register_socket_transport(unique_fd s, std::string serial, int port, int loc VLOG(TRANSPORT) << "socket transport " << transport->serial << " is already in transport_list and fails to register"; delete t; - return -EALREADY; + if (error) *error = EALREADY; + return false; } } @@ -1229,10 +1232,20 @@ int register_socket_transport(unique_fd s, std::string serial, int port, int loc if (local == 1) { // Do not wait for emulator transports. - return 0; + return true; } - return waitable->WaitForConnection(std::chrono::seconds(10)) ? 0 : -1; + if (!waitable->WaitForConnection(std::chrono::seconds(10))) { + if (error) *error = ETIMEDOUT; + return false; + } + + if (t->GetConnectionState() == kCsUnauthorized) { + if (error) *error = EPERM; + return false; + } + + return true; } #if ADB_HOST diff --git a/adb/transport.h b/adb/transport.h index 4bfc2ce2b..f362f241e 100644 --- a/adb/transport.h +++ b/adb/transport.h @@ -364,8 +364,8 @@ void register_usb_transport(usb_handle* h, const char* serial, void connect_device(const std::string& address, std::string* response); /* cause new transports to be init'd and added to the list */ -int register_socket_transport(unique_fd s, std::string serial, int port, int local, - atransport::ReconnectCallback reconnect); +bool register_socket_transport(unique_fd s, std::string serial, int port, int local, + atransport::ReconnectCallback reconnect, int* error = nullptr); // This should only be used for transports with connection_state == kCsNoPerm. void unregister_usb_transport(usb_handle* usb); diff --git a/adb/transport_local.cpp b/adb/transport_local.cpp index b20dee943..143125289 100644 --- a/adb/transport_local.cpp +++ b/adb/transport_local.cpp @@ -125,10 +125,12 @@ void connect_device(const std::string& address, std::string* response) { return init_socket_transport(t, std::move(fd), port, 0) >= 0; }; - int ret = register_socket_transport(std::move(fd), serial, port, 0, std::move(reconnect)); - if (ret < 0) { - if (ret == -EALREADY) { + int error; + if (!register_socket_transport(std::move(fd), serial, port, 0, std::move(reconnect), &error)) { + if (error == EALREADY) { *response = android::base::StringPrintf("already connected to %s", serial.c_str()); + } else if (error == EPERM) { + *response = android::base::StringPrintf("failed to authenticate to %s", serial.c_str()); } else { *response = android::base::StringPrintf("failed to connect to %s", serial.c_str()); } @@ -162,7 +164,7 @@ int local_connect_arbitrary_ports(int console_port, int adb_port, std::string* e disable_tcp_nagle(fd.get()); std::string serial = getEmulatorSerialString(console_port); if (register_socket_transport(std::move(fd), std::move(serial), adb_port, 1, - [](atransport*) { return false; }) == 0) { + [](atransport*) { return false; })) { return 0; } }