From 727b07b2607c712cfa14808de7fcd0d18f5e4923 Mon Sep 17 00:00:00 2001 From: Josh Gao Date: Fri, 13 Sep 2019 00:12:26 +0800 Subject: [PATCH] adb: fix old host transport selection. We regressed handling of the old host transport selection syntax, which broke users that reimplement adb themselves (e.g. Studio via ddmlib). Bug: https://issuetracker.google.com/140369526 Test: adb raw "host-serial:822X0028S:forward:tcp:42929;localabstract:/com.example.ndktest-0/platform-1568299082100.sock" Test: ./test_device.py Change-Id: Iaaec8fde952316fe9bf2a6f6c6c4a3bc9f74bf72 --- adb/adb.cpp | 31 ++++++++++++++++++++++++------- adb/sockets.cpp | 2 ++ adb/test_device.py | 19 +++++++++++++++++++ 3 files changed, 45 insertions(+), 7 deletions(-) diff --git a/adb/adb.cpp b/adb/adb.cpp index ba6df7ac1..1ec145b25 100644 --- a/adb/adb.cpp +++ b/adb/adb.cpp @@ -1131,7 +1131,9 @@ HostRequestResult handle_host_request(std::string_view service, TransportType ty if (service == "features") { std::string error; - atransport* t = acquire_one_transport(type, serial, transport_id, nullptr, &error); + atransport* t = + s->transport ? s->transport + : acquire_one_transport(type, serial, transport_id, nullptr, &error); if (t != nullptr) { SendOkay(reply_fd, FeatureSetToString(t->features())); } else { @@ -1190,7 +1192,9 @@ HostRequestResult handle_host_request(std::string_view service, TransportType ty // These always report "unknown" rather than the actual error, for scripts. if (service == "get-serialno") { std::string error; - atransport* t = acquire_one_transport(type, serial, transport_id, nullptr, &error); + atransport* t = + s->transport ? s->transport + : acquire_one_transport(type, serial, transport_id, nullptr, &error); if (t) { SendOkay(reply_fd, !t->serial.empty() ? t->serial : "unknown"); } else { @@ -1200,7 +1204,9 @@ HostRequestResult handle_host_request(std::string_view service, TransportType ty } if (service == "get-devpath") { std::string error; - atransport* t = acquire_one_transport(type, serial, transport_id, nullptr, &error); + atransport* t = + s->transport ? s->transport + : acquire_one_transport(type, serial, transport_id, nullptr, &error); if (t) { SendOkay(reply_fd, !t->devpath.empty() ? t->devpath : "unknown"); } else { @@ -1210,7 +1216,9 @@ HostRequestResult handle_host_request(std::string_view service, TransportType ty } if (service == "get-state") { std::string error; - atransport* t = acquire_one_transport(type, serial, transport_id, nullptr, &error); + atransport* t = + s->transport ? s->transport + : acquire_one_transport(type, serial, transport_id, nullptr, &error); if (t) { SendOkay(reply_fd, t->connection_state_name()); } else { @@ -1234,7 +1242,9 @@ HostRequestResult handle_host_request(std::string_view service, TransportType ty if (service == "reconnect") { std::string response; - atransport* t = acquire_one_transport(type, serial, transport_id, nullptr, &response, true); + atransport* t = s->transport ? s->transport + : acquire_one_transport(type, serial, transport_id, nullptr, + &response, true); if (t != nullptr) { kick_transport(t, true); response = @@ -1246,8 +1256,15 @@ HostRequestResult handle_host_request(std::string_view service, TransportType ty // TODO: Switch handle_forward_request to string_view. std::string service_str(service); - if (handle_forward_request( - service_str.c_str(), [=](std::string* error) { return s->transport; }, reply_fd)) { + auto transport_acquirer = [=](std::string* error) { + if (s->transport) { + return s->transport; + } else { + std::string error; + return acquire_one_transport(type, serial, transport_id, nullptr, &error); + } + }; + if (handle_forward_request(service_str.c_str(), transport_acquirer, reply_fd)) { return HostRequestResult::Handled; } diff --git a/adb/sockets.cpp b/adb/sockets.cpp index 75993b3ce..e78530c18 100644 --- a/adb/sockets.cpp +++ b/adb/sockets.cpp @@ -757,6 +757,8 @@ static int smart_socket_enqueue(asocket* s, apacket::payload_type data) { #if ADB_HOST service = std::string_view(s->smart_socket_data).substr(4); + + // TODO: These should be handled in handle_host_request. if (android::base::ConsumePrefix(&service, "host-serial:")) { // serial number should follow "host:" and could be a host:port string. if (!internal::parse_host_service(&serial, &service, service)) { diff --git a/adb/test_device.py b/adb/test_device.py index f95a5b3cd..dbd80ed69 100755 --- a/adb/test_device.py +++ b/adb/test_device.py @@ -139,6 +139,25 @@ class ForwardReverseTest(DeviceTest): msg = self.device.forward_list() self.assertEqual('', msg.strip()) + def test_forward_old_protocol(self): + serialno = subprocess.check_output(self.device.adb_cmd + ['get-serialno']).strip() + + msg = self.device.forward_list() + self.assertEqual('', msg.strip(), + 'Forwarding list must be empty to run this test.') + + s = socket.create_connection(("localhost", 5037)) + service = b"host-serial:%s:forward:tcp:5566;tcp:6655" % serialno + cmd = b"%04x%s" % (len(service), service) + s.sendall(cmd) + + msg = self.device.forward_list() + self.assertTrue(re.search(r'tcp:5566.+tcp:6655', msg)) + + self.device.forward_remove_all() + msg = self.device.forward_list() + self.assertEqual('', msg.strip()) + def test_forward_tcp_port_0(self): self.assertEqual('', self.device.forward_list().strip(), 'Forwarding list must be empty to run this test.')