From 190624301755d366ccc464b2223eeeec5c7ccd32 Mon Sep 17 00:00:00 2001 From: Josh Gao Date: Mon, 30 Jul 2018 18:49:03 -0700 Subject: [PATCH] adb: move list-forward, kill-forward back into handle_forward_request. The daemon-side reverse functions depended on handle_forward_request: move them back instead of duplicating the logic we had in handle_host_request. Accomplish what we originally wanted to do in this change by changing the transport argument of handle_forward_request to a std::function that acquires a transport, either via acquire_one_transport or immediately returning a value that we already have. As a side effect, fix a bug where we would emit spurious errors for host service requests. Bug: http://b/112009742 Test: echo "001chost:connect:127.0.0.1:5555" | nc localhost 5037 Test: python test_device.py Test: python test_adb.py Change-Id: Iccc555575df6dbd7de10382854c4ea2c6f4beeaa --- adb/adb.cpp | 84 +++++++++++++++++++++++------------------ adb/adb.h | 5 ++- adb/daemon/services.cpp | 2 +- 3 files changed, 53 insertions(+), 38 deletions(-) diff --git a/adb/adb.cpp b/adb/adb.cpp index 19300f660..38c6f62c9 100644 --- a/adb/adb.cpp +++ b/adb/adb.cpp @@ -920,13 +920,45 @@ int launch_server(const std::string& socket_spec) { } #endif /* ADB_HOST */ +bool handle_forward_request(const char* service, atransport* transport, int reply_fd) { + return handle_forward_request(service, [transport](std::string*) { return transport; }, + reply_fd); +} + // Try to handle a network forwarding request. -// This returns 1 on success, 0 on failure, and -1 to indicate this is not -// a forwarding-related request. -int handle_forward_request(const char* service, atransport* transport, int reply_fd) { +bool handle_forward_request(const char* service, + std::function transport_acquirer, + int reply_fd) { + if (!strcmp(service, "list-forward")) { + // Create the list of forward redirections. + std::string listeners = format_listeners(); +#if ADB_HOST + SendOkay(reply_fd); +#endif + SendProtocolString(reply_fd, listeners); + return true; + } + + if (!strcmp(service, "killforward-all")) { + remove_all_listeners(); +#if ADB_HOST + /* On the host: 1st OKAY is connect, 2nd OKAY is status */ + SendOkay(reply_fd); +#endif + SendOkay(reply_fd); + return true; + } + if (!strncmp(service, "forward:", 8) || !strncmp(service, "killforward:", 12)) { // killforward:local // forward:(norebind:)?local;remote + std::string error; + atransport* transport = transport_acquirer(&error); + if (!transport) { + SendFail(reply_fd, error); + return true; + } + bool kill_forward = false; bool no_rebind = false; if (android::base::StartsWith(service, "killforward:")) { @@ -946,17 +978,16 @@ int handle_forward_request(const char* service, atransport* transport, int reply // Check killforward: parameter format: '' if (pieces.size() != 1 || pieces[0].empty()) { SendFail(reply_fd, android::base::StringPrintf("bad killforward: %s", service)); - return 1; + return true; } } else { // Check forward: parameter format: ';' if (pieces.size() != 2 || pieces[0].empty() || pieces[1].empty() || pieces[1][0] == '*') { SendFail(reply_fd, android::base::StringPrintf("bad forward: %s", service)); - return 1; + return true; } } - std::string error; InstallStatus r; int resolved_tcp_port = 0; if (kill_forward) { @@ -977,7 +1008,7 @@ int handle_forward_request(const char* service, atransport* transport, int reply SendProtocolString(reply_fd, android::base::StringPrintf("%d", resolved_tcp_port)); } - return 1; + return true; } std::string message; @@ -996,9 +1027,10 @@ int handle_forward_request(const char* service, atransport* transport, int reply break; } SendFail(reply_fd, message); - return 1; + return true; } - return 0; + + return false; } #if ADB_HOST @@ -1186,35 +1218,15 @@ int handle_host_request(const char* service, TransportType type, const char* ser return SendOkay(reply_fd, response); } - if (!strcmp(service, "list-forward")) { - // Create the list of forward redirections. - std::string listeners = format_listeners(); -#if ADB_HOST - SendOkay(reply_fd); -#endif - return SendProtocolString(reply_fd, listeners); + if (handle_forward_request(service, + [=](std::string* error) { + return acquire_one_transport(type, serial, transport_id, nullptr, + error); + }, + reply_fd)) { + return 0; } - if (!strcmp(service, "killforward-all")) { - remove_all_listeners(); -#if ADB_HOST - /* On the host: 1st OKAY is connect, 2nd OKAY is status */ - SendOkay(reply_fd); -#endif - SendOkay(reply_fd); - return 1; - } - - std::string error; - atransport* t = acquire_one_transport(type, serial, transport_id, nullptr, &error); - if (!t) { - SendFail(reply_fd, error); - return 1; - } - - int ret = handle_forward_request(service, t, reply_fd); - if (ret >= 0) - return ret - 1; return -1; } diff --git a/adb/adb.h b/adb/adb.h index 13ca4d7e0..e6af780c4 100644 --- a/adb/adb.h +++ b/adb/adb.h @@ -158,7 +158,10 @@ asocket* create_jdwp_tracker_service_socket(); unique_fd create_jdwp_connection_fd(int jdwp_pid); #endif -int handle_forward_request(const char* service, atransport* transport, int reply_fd); +bool handle_forward_request(const char* service, atransport* transport, int reply_fd); +bool handle_forward_request(const char* service, + std::function transport_acquirer, + int reply_fd); /* packet allocator */ apacket* get_apacket(void); diff --git a/adb/daemon/services.cpp b/adb/daemon/services.cpp index 25024b05a..1f59d6446 100644 --- a/adb/daemon/services.cpp +++ b/adb/daemon/services.cpp @@ -157,7 +157,7 @@ unique_fd reverse_service(const char* command, atransport* transport) { return unique_fd{}; } VLOG(SERVICES) << "service socketpair: " << s[0] << ", " << s[1]; - if (handle_forward_request(command, transport, s[1]) < 0) { + if (!handle_forward_request(command, transport, s[1])) { SendFail(s[1], "not a reverse forwarding command"); } adb_close(s[1]);