From eaae97e127717750b4264d9b6617b845f9bc701f Mon Sep 17 00:00:00 2001 From: David Pursell Date: Thu, 7 Apr 2016 11:25:48 -0700 Subject: [PATCH] adb: support forwarding TCP port 0. This CL adds support to forward or reverse TCP port 0 to allow the system to automatically select an open port. The resolved port number will be printed to stdout: $ adb forward tcp:0 tcp:8000 12345 $ adb reverse tcp:0 tcp:9000 23456 This allows testing to be more robust by not hardcoding TCP ports which may already be in use. Forwarding port 0 is a host-only change and will work with any device, but reversing port 0 requires the device to be updated with a new adbd binary. This CL also does a little bit of cleanup such as moving the alistener class out of adb.h, and adds some error checking and additional tests. Bug: 28051746 Test: python -m unittest discover Test: adb_test Test: `adb forward` and `adb reverse` with tcp:0 Change-Id: Icaa87346685b403ab5da7f0e6aa186aa091da572 --- adb/Android.mk | 1 + adb/adb.cpp | 13 ++- adb/adb.h | 25 +--- adb/adb_listeners.cpp | 233 ++++++++++++++++++------------------- adb/adb_listeners.h | 8 +- adb/adb_listeners_test.cpp | 166 ++++++++++++++++++++++++++ adb/adb_utils.cpp | 24 ++++ adb/adb_utils.h | 7 ++ adb/adb_utils_test.cpp | 21 ++++ adb/client/main.cpp | 2 +- adb/commandline.cpp | 35 +++++- adb/daemon/main.cpp | 6 +- adb/sysdeps.h | 7 ++ adb/sysdeps_win32.cpp | 38 +++--- adb/test_device.py | 32 +++++ 15 files changed, 443 insertions(+), 175 deletions(-) create mode 100644 adb/adb_listeners_test.cpp diff --git a/adb/Android.mk b/adb/Android.mk index 6693619fa..71d5aaffc 100644 --- a/adb/Android.mk +++ b/adb/Android.mk @@ -58,6 +58,7 @@ LIBADB_SRC_FILES := \ LIBADB_TEST_SRCS := \ adb_io_test.cpp \ + adb_listeners_test.cpp \ adb_utils_test.cpp \ fdevent_test.cpp \ socket_test.cpp \ diff --git a/adb/adb.cpp b/adb/adb.cpp index 49d293664..11e9c6863 100644 --- a/adb/adb.cpp +++ b/adb/adb.cpp @@ -955,18 +955,25 @@ int handle_forward_request(const char* service, TransportType type, const char* std::string error; InstallStatus r; + int resolved_tcp_port = 0; if (kill_forward) { r = remove_listener(pieces[0].c_str(), transport); } else { - r = install_listener(pieces[0], pieces[1].c_str(), transport, - no_rebind, &error); + r = install_listener(pieces[0], pieces[1].c_str(), transport, no_rebind, + &resolved_tcp_port, &error); } if (r == INSTALL_STATUS_OK) { #if ADB_HOST - /* On the host: 1st OKAY is connect, 2nd OKAY is status */ + // On the host: 1st OKAY is connect, 2nd OKAY is status. SendOkay(reply_fd); #endif SendOkay(reply_fd); + + // If a TCP port was resolved, send the actual port number back. + if (resolved_tcp_port != 0) { + SendProtocolString(reply_fd, android::base::StringPrintf("%d", resolved_tcp_port)); + } + return 1; } diff --git a/adb/adb.h b/adb/adb.h index ea208004d..cb38e6148 100644 --- a/adb/adb.h +++ b/adb/adb.h @@ -50,7 +50,7 @@ constexpr size_t MAX_PAYLOAD = MAX_PAYLOAD_V2; std::string adb_version(); // Increment this when we want to force users to start a new adb server. -#define ADB_SERVER_VERSION 36 +#define ADB_SERVER_VERSION 37 class atransport; struct usb_handle; @@ -116,29 +116,6 @@ enum ConnectionState { kCsUnauthorized, }; -/* A listener is an entity which binds to a local port -** and, upon receiving a connection on that port, creates -** an asocket to connect the new local connection to a -** specific remote service. -** -** TODO: some listeners read from the new connection to -** determine what exact service to connect to on the far -** side. -*/ -struct alistener -{ - alistener *next; - alistener *prev; - - fdevent fde; - int fd; - - char *local_name; - char *connect_to; - atransport *transport; - adisconnect disconnect; -}; - void print_packet(const char *label, apacket *p); diff --git a/adb/adb_listeners.cpp b/adb/adb_listeners.cpp index e8c2338a4..f54603cad 100644 --- a/adb/adb_listeners.cpp +++ b/adb/adb_listeners.cpp @@ -20,18 +20,55 @@ #include #include +#include #include #include "sysdeps.h" #include "transport.h" -int gListenAll = 0; /* Not static because it is used in commandline.c. */ +// Not static because it is used in commandline.c. +int gListenAll = 0; -static alistener listener_list = { - .next = &listener_list, - .prev = &listener_list, +// A listener is an entity which binds to a local port and, upon receiving a connection on that +// port, creates an asocket to connect the new local connection to a specific remote service. +// +// TODO: some listeners read from the new connection to determine what exact service to connect to +// on the far side. +class alistener { + public: + alistener(const std::string& _local_name, const std::string& _connect_to); + ~alistener(); + + fdevent fde; + int fd = -1; + + std::string local_name; + std::string connect_to; + atransport* transport = nullptr; + adisconnect disconnect; + + private: + DISALLOW_COPY_AND_ASSIGN(alistener); }; +alistener::alistener(const std::string& _local_name, const std::string& _connect_to) + : local_name(_local_name), connect_to(_connect_to) { +} + +alistener::~alistener() { + // Closes the corresponding fd. + fdevent_remove(&fde); + + if (transport) { + transport->RemoveDisconnect(&disconnect); + } +} + +// listener_list retains ownership of all created alistener objects. Removing an alistener from +// this list will cause it to be deleted. +typedef std::list> ListenerList; +static ListenerList& listener_list = *new ListenerList(); + static void ss_listener_event_func(int _fd, unsigned ev, void *_l) { if (ev & FDE_READ) { sockaddr_storage ss; @@ -73,7 +110,7 @@ static void listener_event_func(int _fd, unsigned ev, void* _l) s = create_local_socket(fd); if (s) { s->transport = listener->transport; - connect_to_remote(s, listener->connect_to); + connect_to_remote(s, listener->connect_to.c_str()); return; } @@ -81,66 +118,63 @@ static void listener_event_func(int _fd, unsigned ev, void* _l) } } -static void free_listener(alistener* l) -{ - if (l->next) { - l->next->prev = l->prev; - l->prev->next = l->next; - l->next = l->prev = l; - } - - // closes the corresponding fd - fdevent_remove(&l->fde); - - if (l->local_name) - free((char*)l->local_name); - - if (l->connect_to) - free((char*)l->connect_to); - - if (l->transport) { - l->transport->RemoveDisconnect(&l->disconnect); - } - free(l); -} - +// Called as a transport disconnect function. |arg| is the raw alistener*. static void listener_disconnect(void* arg, atransport*) { - alistener* listener = reinterpret_cast(arg); - listener->transport = nullptr; - free_listener(listener); -} - -static int local_name_to_fd(const char* name, std::string* error) { - if (!strncmp("tcp:", name, 4)) { - int port = atoi(name + 4); - if (gListenAll > 0) { - return network_inaddr_any_server(port, SOCK_STREAM, error); - } else { - return network_loopback_server(port, SOCK_STREAM, error); + for (auto iter = listener_list.begin(); iter != listener_list.end(); ++iter) { + if (iter->get() == arg) { + (*iter)->transport = nullptr; + listener_list.erase(iter); + return; } } +} + +int local_name_to_fd(alistener* listener, int* resolved_tcp_port, std::string* error) { + if (android::base::StartsWith(listener->local_name, "tcp:")) { + int requested_port = atoi(&listener->local_name[4]); + int sock = -1; + if (gListenAll > 0) { + sock = network_inaddr_any_server(requested_port, SOCK_STREAM, error); + } else { + sock = network_loopback_server(requested_port, SOCK_STREAM, error); + } + + // If the caller requested port 0, update the listener name with the resolved port. + if (sock >= 0 && requested_port == 0) { + int local_port = adb_socket_get_local_port(sock); + if (local_port > 0) { + listener->local_name = android::base::StringPrintf("tcp:%d", local_port); + if (resolved_tcp_port != nullptr) { + *resolved_tcp_port = local_port; + } + } + } + + return sock; + } #if !defined(_WIN32) // No Unix-domain sockets on Windows. - // It's nonsensical to support the "reserved" space on the adb host side - if (!strncmp(name, "local:", 6)) { - return network_local_server(name + 6, - ANDROID_SOCKET_NAMESPACE_ABSTRACT, SOCK_STREAM, error); - } else if (!strncmp(name, "localabstract:", 14)) { - return network_local_server(name + 14, - ANDROID_SOCKET_NAMESPACE_ABSTRACT, SOCK_STREAM, error); - } else if (!strncmp(name, "localfilesystem:", 16)) { - return network_local_server(name + 16, - ANDROID_SOCKET_NAMESPACE_FILESYSTEM, SOCK_STREAM, error); + // It's nonsensical to support the "reserved" space on the adb host side. + if (android::base::StartsWith(listener->local_name, "local:")) { + return network_local_server(&listener->local_name[6], ANDROID_SOCKET_NAMESPACE_ABSTRACT, + SOCK_STREAM, error); + } else if (android::base::StartsWith(listener->local_name, "localabstract:")) { + return network_local_server(&listener->local_name[14], ANDROID_SOCKET_NAMESPACE_ABSTRACT, + SOCK_STREAM, error); + } else if (android::base::StartsWith(listener->local_name, "localfilesystem:")) { + return network_local_server(&listener->local_name[16], ANDROID_SOCKET_NAMESPACE_FILESYSTEM, + SOCK_STREAM, error); } #endif - *error = android::base::StringPrintf("unknown local portname '%s'", name); + *error = android::base::StringPrintf("unknown local portname '%s'", + listener->local_name.c_str()); return -1; } // Write the list of current listeners (network redirections) into a string. std::string format_listeners() { std::string result; - for (alistener* l = listener_list.next; l != &listener_list; l = l->next) { + for (auto& l : listener_list) { // Ignore special listeners like those for *smartsocket* if (l->connect_to[0] == '*') { continue; @@ -149,65 +183,51 @@ std::string format_listeners() { // Entries from "adb reverse" have no serial. android::base::StringAppendF(&result, "%s %s %s\n", l->transport->serial ? l->transport->serial : "(reverse)", - l->local_name, l->connect_to); + l->local_name.c_str(), l->connect_to.c_str()); } return result; } -InstallStatus remove_listener(const char *local_name, atransport* transport) { - alistener *l; - - for (l = listener_list.next; l != &listener_list; l = l->next) { - if (!strcmp(local_name, l->local_name)) { - free_listener(l); +InstallStatus remove_listener(const char* local_name, atransport* transport) { + for (auto iter = listener_list.begin(); iter != listener_list.end(); ++iter) { + if (local_name == (*iter)->local_name) { + listener_list.erase(iter); return INSTALL_STATUS_OK; } } return INSTALL_STATUS_LISTENER_NOT_FOUND; } -void remove_all_listeners(void) -{ - alistener *l, *l_next; - for (l = listener_list.next; l != &listener_list; l = l_next) { - l_next = l->next; +void remove_all_listeners() { + auto iter = listener_list.begin(); + while (iter != listener_list.end()) { // Never remove smart sockets. - if (l->connect_to[0] == '*') - continue; - free_listener(l); + if ((*iter)->connect_to[0] == '*') { + ++iter; + } else { + iter = listener_list.erase(iter); + } } } -InstallStatus install_listener(const std::string& local_name, - const char *connect_to, - atransport* transport, - int no_rebind, - std::string* error) -{ - for (alistener* l = listener_list.next; l != &listener_list; l = l->next) { +InstallStatus install_listener(const std::string& local_name, const char* connect_to, + atransport* transport, int no_rebind, int* resolved_tcp_port, + std::string* error) { + for (auto& l : listener_list) { if (local_name == l->local_name) { - char* cto; - - /* can't repurpose a smartsocket */ + // Can't repurpose a smartsocket. if(l->connect_to[0] == '*') { *error = "cannot repurpose smartsocket"; return INSTALL_STATUS_INTERNAL_ERROR; } - /* can't repurpose a listener if 'no_rebind' is true */ + // Can't repurpose a listener if 'no_rebind' is true. if (no_rebind) { *error = "cannot rebind"; return INSTALL_STATUS_CANNOT_REBIND; } - cto = strdup(connect_to); - if(cto == 0) { - *error = "cannot duplicate string"; - return INSTALL_STATUS_INTERNAL_ERROR; - } - - free((void*) l->connect_to); - l->connect_to = cto; + l->connect_to = connect_to; if (l->transport != transport) { l->transport->RemoveDisconnect(&l->disconnect); l->transport = transport; @@ -217,54 +237,29 @@ InstallStatus install_listener(const std::string& local_name, } } - alistener* listener = reinterpret_cast( - calloc(1, sizeof(alistener))); - if (listener == nullptr) { - goto nomem; - } + std::unique_ptr listener(new alistener(local_name, connect_to)); - listener->local_name = strdup(local_name.c_str()); - if (listener->local_name == nullptr) { - goto nomem; - } - - listener->connect_to = strdup(connect_to); - if (listener->connect_to == nullptr) { - goto nomem; - } - - listener->fd = local_name_to_fd(listener->local_name, error); + listener->fd = local_name_to_fd(listener.get(), resolved_tcp_port, error); if (listener->fd < 0) { - free(listener->local_name); - free(listener->connect_to); - free(listener); return INSTALL_STATUS_CANNOT_BIND; } close_on_exec(listener->fd); - if (!strcmp(listener->connect_to, "*smartsocket*")) { - fdevent_install(&listener->fde, listener->fd, ss_listener_event_func, - listener); + if (listener->connect_to == "*smartsocket*") { + fdevent_install(&listener->fde, listener->fd, ss_listener_event_func, listener.get()); } else { - fdevent_install(&listener->fde, listener->fd, listener_event_func, - listener); + fdevent_install(&listener->fde, listener->fd, listener_event_func, listener.get()); } fdevent_set(&listener->fde, FDE_READ); - listener->next = &listener_list; - listener->prev = listener_list.prev; - listener->next->prev = listener; - listener->prev->next = listener; listener->transport = transport; if (transport) { - listener->disconnect.opaque = listener; + listener->disconnect.opaque = listener.get(); listener->disconnect.func = listener_disconnect; transport->AddDisconnect(&listener->disconnect); } - return INSTALL_STATUS_OK; -nomem: - fatal("cannot allocate listener"); - return INSTALL_STATUS_INTERNAL_ERROR; + listener_list.push_back(std::move(listener)); + return INSTALL_STATUS_OK; } diff --git a/adb/adb_listeners.h b/adb/adb_listeners.h index fa98eed59..8eba00a5e 100644 --- a/adb/adb_listeners.h +++ b/adb/adb_listeners.h @@ -21,6 +21,8 @@ #include +#include + // error/status codes for install_listener. enum InstallStatus { INSTALL_STATUS_OK = 0, @@ -30,10 +32,8 @@ enum InstallStatus { INSTALL_STATUS_LISTENER_NOT_FOUND = -4, }; -InstallStatus install_listener(const std::string& local_name, - const char* connect_to, - atransport* transport, - int no_rebind, +InstallStatus install_listener(const std::string& local_name, const char* connect_to, + atransport* transport, int no_rebind, int* resolved_tcp_port, std::string* error); std::string format_listeners(); diff --git a/adb/adb_listeners_test.cpp b/adb/adb_listeners_test.cpp new file mode 100644 index 000000000..b697769d2 --- /dev/null +++ b/adb/adb_listeners_test.cpp @@ -0,0 +1,166 @@ +/* + * Copyright (C) 2016 The Android Open Source Project + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +#include "adb_listeners.h" + +#include + +#include +#include + +#include "fdevent.h" +#include "sysdeps.h" +#include "transport.h" + +// Returns true if the given listener is present in format_listeners(). Empty parameters will +// be ignored. +static bool listener_is_installed(const std::string& serial, const std::string& source, + const std::string& dest) { + // format_listeners() gives lines of " \n". + for (const std::string& line : android::base::Split(format_listeners(), "\n")) { + std::vector info = android::base::Split(line, " "); + if (info.size() == 3 && + (serial.empty() || info[0] == serial) && + (source.empty() || info[1] == source) && + (dest.empty() || info[2] == dest)) { + return true; + } + } + + return false; +} + +class AdbListenersTest : public ::testing::Test { + public: + void SetUp() override { + // We don't need an fdevent loop, but adding/removing listeners must be done from the + // fdevent thread if one exists. Since previously run tests may have created an fdevent + // thread, we need to reset to prevent the thread check. + fdevent_reset(); + } + + void TearDown() override { + // Clean up any listeners that may have been installed. + remove_all_listeners(); + + // Make sure we didn't leave any dangling events. + ASSERT_EQ(0u, fdevent_installed_count()); + } + + protected: + atransport transport_; +}; + +TEST_F(AdbListenersTest, test_install_listener) { + std::string error; + + ASSERT_EQ(INSTALL_STATUS_OK, + install_listener("tcp:9000", "tcp:9000", &transport_, false, nullptr, &error)); + ASSERT_TRUE(error.empty()); + + ASSERT_TRUE(listener_is_installed("", "tcp:9000", "tcp:9000")); +} + +TEST_F(AdbListenersTest, test_install_listener_rebind) { + std::string error; + + ASSERT_EQ(INSTALL_STATUS_OK, + install_listener("tcp:9000", "tcp:9000", &transport_, false, nullptr, &error)); + ASSERT_TRUE(error.empty()); + + ASSERT_EQ(INSTALL_STATUS_OK, + install_listener("tcp:9000", "tcp:9001", &transport_, false, nullptr, &error)); + ASSERT_TRUE(error.empty()); + + ASSERT_TRUE(listener_is_installed("", "tcp:9000", "tcp:9001")); +} + +TEST_F(AdbListenersTest, test_install_listener_no_rebind) { + std::string error; + + ASSERT_EQ(INSTALL_STATUS_OK, + install_listener("tcp:9000", "tcp:9000", &transport_, true, nullptr, &error)); + ASSERT_TRUE(error.empty()); + + ASSERT_EQ(INSTALL_STATUS_CANNOT_REBIND, + install_listener("tcp:9000", "tcp:9001", &transport_, true, nullptr, &error)); + ASSERT_FALSE(error.empty()); + + ASSERT_TRUE(listener_is_installed("", "tcp:9000", "tcp:9000")); +} + +TEST_F(AdbListenersTest, test_install_listener_tcp_port_0) { + int port = 0; + std::string error; + + ASSERT_EQ(INSTALL_STATUS_OK, + install_listener("tcp:0", "tcp:9000", &transport_, true, &port, &error)); + ASSERT_TRUE(error.empty()); + + ASSERT_TRUE(listener_is_installed("", android::base::StringPrintf("tcp:%d", port), "tcp:9000")); +} + +TEST_F(AdbListenersTest, test_remove_listener) { + std::string error; + + ASSERT_EQ(INSTALL_STATUS_OK, + install_listener("tcp:9000", "tcp:9000", &transport_, false, nullptr, &error)); + ASSERT_TRUE(error.empty()); + + ASSERT_EQ(INSTALL_STATUS_OK, remove_listener("tcp:9000", &transport_)); + ASSERT_TRUE(format_listeners().empty()); +} + +TEST_F(AdbListenersTest, test_remove_nonexistent_listener) { + std::string error; + + ASSERT_EQ(INSTALL_STATUS_OK, + install_listener("tcp:9000", "tcp:9000", &transport_, false, nullptr, &error)); + ASSERT_TRUE(error.empty()); + + ASSERT_EQ(INSTALL_STATUS_LISTENER_NOT_FOUND, remove_listener("tcp:1", &transport_)); + ASSERT_TRUE(listener_is_installed("", "tcp:9000", "tcp:9000")); +} + +TEST_F(AdbListenersTest, test_remove_all_listeners) { + std::string error; + + ASSERT_EQ(INSTALL_STATUS_OK, + install_listener("tcp:9000", "tcp:9000", &transport_, false, nullptr, &error)); + ASSERT_TRUE(error.empty()); + + ASSERT_EQ(INSTALL_STATUS_OK, + install_listener("tcp:9001", "tcp:9001", &transport_, false, nullptr, &error)); + ASSERT_TRUE(error.empty()); + + remove_all_listeners(); + ASSERT_TRUE(format_listeners().empty()); +} + +TEST_F(AdbListenersTest, test_transport_disconnect) { + std::string error; + + ASSERT_EQ(INSTALL_STATUS_OK, + install_listener("tcp:9000", "tcp:9000", &transport_, false, nullptr, &error)); + ASSERT_TRUE(error.empty()); + + ASSERT_EQ(INSTALL_STATUS_OK, + install_listener("tcp:9001", "tcp:9001", &transport_, false, nullptr, &error)); + ASSERT_TRUE(error.empty()); + + transport_.RunDisconnects(); + ASSERT_TRUE(format_listeners().empty()); +} diff --git a/adb/adb_utils.cpp b/adb/adb_utils.cpp index 3333fc61b..5d4755f90 100644 --- a/adb/adb_utils.cpp +++ b/adb/adb_utils.cpp @@ -27,6 +27,7 @@ #include #include +#include #include #include @@ -230,3 +231,26 @@ bool set_file_block_mode(int fd, bool block) { return true; } #endif + +bool forward_targets_are_valid(const std::string& source, const std::string& dest, + std::string* error) { + if (android::base::StartsWith(source, "tcp:")) { + // The source port may be 0 to allow the system to select an open port. + int port; + if (!android::base::ParseInt(&source[4], &port) || port < 0) { + *error = android::base::StringPrintf("Invalid source port: '%s'", &source[4]); + return false; + } + } + + if (android::base::StartsWith(dest, "tcp:")) { + // The destination port must be > 0. + int port; + if (!android::base::ParseInt(&dest[4], &port) || port <= 0) { + *error = android::base::StringPrintf("Invalid destination port: '%s'", &dest[4]); + return false; + } + } + + return true; +} diff --git a/adb/adb_utils.h b/adb/adb_utils.h index 89fcd66ec..abf481b31 100644 --- a/adb/adb_utils.h +++ b/adb/adb_utils.h @@ -43,6 +43,13 @@ bool set_file_block_mode(int fd, bool block); extern int adb_close(int fd); +// Given forward/reverse targets, returns true if they look sane. If an error is found, fills +// |error| and returns false. +// Currently this only checks "tcp:" targets. Additional checking could be added for other targets +// if needed. +bool forward_targets_are_valid(const std::string& source, const std::string& dest, + std::string* error); + // Helper to automatically close an FD when it goes out of scope. class ScopedFd { public: diff --git a/adb/adb_utils_test.cpp b/adb/adb_utils_test.cpp index f1ebaa130..aabc5d733 100644 --- a/adb/adb_utils_test.cpp +++ b/adb/adb_utils_test.cpp @@ -149,3 +149,24 @@ TEST(adb_utils, set_file_block_mode) { ASSERT_EQ(0, adb_close(fd)); } #endif + +TEST(adb_utils, test_forward_targets_are_valid) { + std::string error; + + // Source port can be >= 0. + EXPECT_FALSE(forward_targets_are_valid("tcp:-1", "tcp:9000", &error)); + EXPECT_TRUE(forward_targets_are_valid("tcp:0", "tcp:9000", &error)); + EXPECT_TRUE(forward_targets_are_valid("tcp:8000", "tcp:9000", &error)); + + // Destination port must be >0. + EXPECT_FALSE(forward_targets_are_valid("tcp:8000", "tcp:-1", &error)); + EXPECT_FALSE(forward_targets_are_valid("tcp:8000", "tcp:0", &error)); + + // Port must be a number. + EXPECT_FALSE(forward_targets_are_valid("tcp:", "tcp:9000", &error)); + EXPECT_FALSE(forward_targets_are_valid("tcp:a", "tcp:9000", &error)); + EXPECT_FALSE(forward_targets_are_valid("tcp:22x", "tcp:9000", &error)); + EXPECT_FALSE(forward_targets_are_valid("tcp:8000", "tcp:", &error)); + EXPECT_FALSE(forward_targets_are_valid("tcp:8000", "tcp:a", &error)); + EXPECT_FALSE(forward_targets_are_valid("tcp:8000", "tcp:22x", &error)); +} diff --git a/adb/client/main.cpp b/adb/client/main.cpp index 27b7109e2..65640ad29 100644 --- a/adb/client/main.cpp +++ b/adb/client/main.cpp @@ -117,7 +117,7 @@ int adb_server_main(int is_daemon, int server_port, int ack_reply_fd) { std::string error; std::string local_name = android::base::StringPrintf("tcp:%d", server_port); - if (install_listener(local_name, "*smartsocket*", nullptr, 0, &error)) { + if (install_listener(local_name, "*smartsocket*", nullptr, 0, nullptr, &error)) { fatal("could not install *smartsocket* listener: %s", error.c_str()); } diff --git a/adb/commandline.cpp b/adb/commandline.cpp index 45c6142e5..28dbb7888 100644 --- a/adb/commandline.cpp +++ b/adb/commandline.cpp @@ -126,7 +126,7 @@ static void help() { " \" \" \" \" \"\\n\"\n" " adb forward - forward socket connections\n" " forward specs are one of: \n" - " tcp:\n" + " tcp: ( may be \"tcp:0\" to pick any open port)\n" " localabstract:\n" " localreserved:\n" " localfilesystem:\n" @@ -140,7 +140,7 @@ static void help() { " adb reverse --list - list all reverse socket connections from device\n" " adb reverse - reverse socket connections\n" " reverse specs are one of:\n" - " tcp:\n" + " tcp: ( may be \"tcp:0\" to pick any open port)\n" " localabstract:\n" " localreserved:\n" " localfilesystem:\n" @@ -1719,7 +1719,7 @@ int adb_commandline(int argc, const char **argv) { } } - std::string cmd; + std::string cmd, error; if (strcmp(argv[0], "--list") == 0) { if (argc != 1) return usage(); return adb_query_command(host_prefix + ":list-forward"); @@ -1733,14 +1733,37 @@ int adb_commandline(int argc, const char **argv) { } else if (strcmp(argv[0], "--no-rebind") == 0) { // forward --no-rebind if (argc != 3) return usage(); - cmd = host_prefix + ":forward:norebind:" + argv[1] + ";" + argv[2]; + if (forward_targets_are_valid(argv[1], argv[2], &error)) { + cmd = host_prefix + ":forward:norebind:" + argv[1] + ";" + argv[2]; + } } else { // forward if (argc != 2) return usage(); - cmd = host_prefix + ":forward:" + argv[0] + ";" + argv[1]; + if (forward_targets_are_valid(argv[0], argv[1], &error)) { + cmd = host_prefix + ":forward:" + argv[0] + ";" + argv[1]; + } } - return adb_command(cmd) ? 0 : 1; + if (!error.empty()) { + fprintf(stderr, "error: %s\n", error.c_str()); + return 1; + } + + int fd = adb_connect(cmd, &error); + if (fd < 0 || !adb_status(fd, &error)) { + adb_close(fd); + fprintf(stderr, "error: %s\n", error.c_str()); + return 1; + } + + // Server or device may optionally return a resolved TCP port number. + std::string resolved_port; + if (ReadProtocolString(fd, &resolved_port, &error) && !resolved_port.empty()) { + printf("%s\n", resolved_port.c_str()); + } + + ReadOrderlyShutdown(fd); + return 0; } /* do_sync_*() commands */ else if (!strcmp(argv[0], "ls")) { diff --git a/adb/daemon/main.cpp b/adb/daemon/main.cpp index 7f40b96b2..916bedfe8 100644 --- a/adb/daemon/main.cpp +++ b/adb/daemon/main.cpp @@ -142,10 +142,8 @@ static void drop_privileges(int server_port) { std::string error; std::string local_name = android::base::StringPrintf("tcp:%d", server_port); - if (install_listener(local_name, "*smartsocket*", nullptr, 0, - &error)) { - LOG(FATAL) << "Could not install *smartsocket* listener: " - << error; + if (install_listener(local_name, "*smartsocket*", nullptr, 0, nullptr, &error)) { + LOG(FATAL) << "Could not install *smartsocket* listener: " << error; } } } diff --git a/adb/sysdeps.h b/adb/sysdeps.h index 81d201e65..3586da861 100644 --- a/adb/sysdeps.h +++ b/adb/sysdeps.h @@ -287,6 +287,9 @@ extern int adb_socket_accept(int serverfd, struct sockaddr* addr, socklen_t #undef accept #define accept ___xxx_accept +// Returns the local port number of a bound socket, or -1 on failure. +int adb_socket_get_local_port(int fd); + extern int adb_setsockopt(int fd, int level, int optname, const void* optval, socklen_t optlen); #undef setsockopt @@ -691,6 +694,10 @@ static __inline__ int adb_socket_accept(int serverfd, struct sockaddr* addr, #undef accept #define accept ___xxx_accept +inline int adb_socket_get_local_port(int fd) { + return socket_get_local_port(fd); +} + // Operate on a file descriptor returned from unix_open() or a well-known file // descriptor such as STDIN_FILENO, STDOUT_FILENO, STDERR_FILENO. // diff --git a/adb/sysdeps_win32.cpp b/adb/sysdeps_win32.cpp index bc09fdcfb..faf7f3e13 100644 --- a/adb/sysdeps_win32.cpp +++ b/adb/sysdeps_win32.cpp @@ -1128,6 +1128,24 @@ int adb_getsockname(int fd, struct sockaddr* sockaddr, socklen_t* optlen) { return result; } +int adb_socket_get_local_port(int fd) { + sockaddr_storage addr_storage; + socklen_t addr_len = sizeof(addr_storage); + + if (adb_getsockname(fd, reinterpret_cast(&addr_storage), &addr_len) < 0) { + D("adb_socket_get_local_port: adb_getsockname failed: %s", strerror(errno)); + return -1; + } + + if (!(addr_storage.ss_family == AF_INET || addr_storage.ss_family == AF_INET6)) { + D("adb_socket_get_local_port: unknown address family received: %d", addr_storage.ss_family); + errno = ECONNABORTED; + return -1; + } + + return ntohs(reinterpret_cast(&addr_storage)->sin_port); +} + int adb_shutdown(int fd) { FH f = _fh_from_int(fd, __func__); @@ -1154,9 +1172,7 @@ int adb_socketpair(int sv[2]) { int server = -1; int client = -1; int accepted = -1; - sockaddr_storage addr_storage; - socklen_t addr_len = sizeof(addr_storage); - sockaddr_in* addr = nullptr; + int local_port = -1; std::string error; server = network_loopback_server(0, SOCK_STREAM, &error); @@ -1165,20 +1181,14 @@ int adb_socketpair(int sv[2]) { goto fail; } - if (adb_getsockname(server, reinterpret_cast(&addr_storage), &addr_len) < 0) { - D("adb_socketpair: adb_getsockname failed: %s", strerror(errno)); + local_port = adb_socket_get_local_port(server); + if (local_port < 0) { + D("adb_socketpair: failed to get server port number: %s", error.c_str()); goto fail; } + D("adb_socketpair: bound on port %d", local_port); - if (addr_storage.ss_family != AF_INET) { - D("adb_socketpair: unknown address family received: %d", addr_storage.ss_family); - errno = ECONNABORTED; - goto fail; - } - - addr = reinterpret_cast(&addr_storage); - D("adb_socketpair: bound on port %d", ntohs(addr->sin_port)); - client = network_loopback_client(ntohs(addr->sin_port), SOCK_STREAM, &error); + client = network_loopback_client(local_port, SOCK_STREAM, &error); if (client < 0) { D("adb_socketpair: failed to connect client: %s", error.c_str()); goto fail; diff --git a/adb/test_device.py b/adb/test_device.py index 2a3be8899..cdc57c6bc 100644 --- a/adb/test_device.py +++ b/adb/test_device.py @@ -191,6 +191,22 @@ class ForwardReverseTest(DeviceTest): 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.') + + try: + # If resolving TCP port 0 is supported, `adb forward` will print + # the actual port number. + port = self.device.forward('tcp:0', 'tcp:8888').strip() + if not port: + raise unittest.SkipTest('Forwarding tcp:0 is not available.') + + self.assertTrue(re.search(r'tcp:{}.+tcp:8888'.format(port), + self.device.forward_list())) + finally: + self.device.forward_remove_all() + def test_reverse(self): msg = self.device.reverse_list() self.assertEqual('', msg.strip(), @@ -210,6 +226,22 @@ class ForwardReverseTest(DeviceTest): msg = self.device.reverse_list() self.assertEqual('', msg.strip()) + def test_reverse_tcp_port_0(self): + self.assertEqual('', self.device.reverse_list().strip(), + 'Reverse list must be empty to run this test.') + + try: + # If resolving TCP port 0 is supported, `adb reverse` will print + # the actual port number. + port = self.device.reverse('tcp:0', 'tcp:8888').strip() + if not port: + raise unittest.SkipTest('Reversing tcp:0 is not available.') + + self.assertTrue(re.search(r'tcp:{}.+tcp:8888'.format(port), + self.device.reverse_list())) + finally: + self.device.reverse_remove_all() + # Note: If you run this test when adb connect'd to a physical device over # TCP, it will fail in adb reverse due to https://code.google.com/p/android/issues/detail?id=189821 def test_forward_reverse_echo(self):