diff options
author | Josh Gao <jmgao@google.com> | 2018-05-04 16:04:49 -0700 |
---|---|---|
committer | Josh Gao <jmgao@google.com> | 2018-05-04 18:05:18 -0700 |
commit | 704494b0707a49e064b63192619a73336c1be05a (patch) | |
tree | 4359d4c0bcc8bff18ab5275982ade02abbb9190e /adb | |
parent | 48cf760bea211b43aa79675c6b73f846202865f9 (diff) | |
download | system_core-704494b0707a49e064b63192619a73336c1be05a.tar.gz system_core-704494b0707a49e064b63192619a73336c1be05a.tar.bz2 system_core-704494b0707a49e064b63192619a73336c1be05a.zip |
adb: add authorizing, connecting states to transport.
Add two states: connecting and authorizing, to disambiguate the offline
and unauthorized states, respectively.
Previously, devices would transition as follows:
offline -> unauthorized -> offline -> online
offline -> unauthorized (when actually unauthorized)
With this patch:
connecting -> authorizing -> online
connecting -> authorizing -> unauthorized (when actually unauthorized)
This allows test automation and the like to distinguish between offline
devices, unauthorized devices, and working devices without having to
do retry loops with arbitrary sleeps on their end.
Bug: http://b/79257434
Test: adb_test
Test: adbd_test
Test: manually plugging in a device with `while true; do adb shell echo foo; done`
Change-Id: I036d9b593b51a27a59ac3fc57da966fd52658567
Diffstat (limited to 'adb')
-rw-r--r-- | adb/adb.cpp | 15 | ||||
-rw-r--r-- | adb/adb.h | 23 | ||||
-rw-r--r-- | adb/client/auth.cpp | 1 | ||||
-rw-r--r-- | adb/sockets.cpp | 2 | ||||
-rw-r--r-- | adb/transport.cpp | 55 | ||||
-rw-r--r-- | adb/transport.h | 2 |
6 files changed, 68 insertions, 30 deletions
diff --git a/adb/adb.cpp b/adb/adb.cpp index 3bf281c6d..76ca19a79 100644 --- a/adb/adb.cpp +++ b/adb/adb.cpp @@ -365,8 +365,8 @@ void handle_packet(apacket *p, atransport *t) switch (p->msg.arg0) { #if ADB_HOST case ADB_AUTH_TOKEN: - if (t->GetConnectionState() == kCsOffline) { - t->SetConnectionState(kCsUnauthorized); + if (t->GetConnectionState() != kCsAuthorizing) { + t->SetConnectionState(kCsAuthorizing); } send_auth_response(p->payload.data(), p->msg.data_length, t); break; @@ -1103,14 +1103,11 @@ int handle_host_request(const char* service, TransportType type, const char* ser if (!strcmp(service, "reconnect-offline")) { std::string response; close_usb_devices([&response](const atransport* transport) { - switch (transport->GetConnectionState()) { - case kCsOffline: - case kCsUnauthorized: - response += "reconnecting " + transport->serial_name() + "\n"; - return true; - default: - return false; + if (!ConnectionStateIsOnline(transport->GetConnectionState())) { + response += "reconnecting " + transport->serial_name() + "\n"; + return true; } + return false; }); if (!response.empty()) { response.resize(response.size() - 1); @@ -95,16 +95,33 @@ enum TransportType { enum ConnectionState { kCsAny = -1, - kCsOffline = 0, + + kCsConnecting = 0, // Haven't received a response from the device yet. + kCsAuthorizing, // Authorizing with keys from ADB_VENDOR_KEYS. + kCsUnauthorized, // ADB_VENDOR_KEYS exhausted, fell back to user prompt. + kCsNoPerm, // Insufficient permissions to communicate with the device. + kCsOffline, + kCsBootloader, kCsDevice, kCsHost, kCsRecovery, - kCsNoPerm, // Insufficient permissions to communicate with the device. kCsSideload, - kCsUnauthorized, }; +inline bool ConnectionStateIsOnline(ConnectionState state) { + switch (state) { + case kCsBootloader: + case kCsDevice: + case kCsHost: + case kCsRecovery: + case kCsSideload: + return true; + default: + return false; + } +} + void print_packet(const char* label, apacket* p); // These use the system (v)fprintf, not the adb prefixed ones defined in sysdeps.h, so they diff --git a/adb/client/auth.cpp b/adb/client/auth.cpp index ade2623b8..0f4dd3397 100644 --- a/adb/client/auth.cpp +++ b/adb/client/auth.cpp @@ -464,6 +464,7 @@ void send_auth_response(const char* token, size_t token_size, atransport* t) { std::shared_ptr<RSA> key = t->NextKey(); if (key == nullptr) { // No more private keys to try, send the public key. + t->SetConnectionState(kCsUnauthorized); send_auth_publickey(t); return; } diff --git a/adb/sockets.cpp b/adb/sockets.cpp index 7bc0165b0..e567ff4f9 100644 --- a/adb/sockets.cpp +++ b/adb/sockets.cpp @@ -750,7 +750,7 @@ static int smart_socket_enqueue(asocket* s, apacket::payload_type data) { if (!s->transport) { SendFail(s->peer->fd, "device offline (no transport)"); goto fail; - } else if (s->transport->GetConnectionState() == kCsOffline) { + } else if (!ConnectionStateIsOnline(s->transport->GetConnectionState())) { /* if there's no remote we fail the connection ** right here and terminate it */ diff --git a/adb/transport.cpp b/adb/transport.cpp index fa7cc8c69..be7f8fe7f 100644 --- a/adb/transport.cpp +++ b/adb/transport.cpp @@ -708,22 +708,41 @@ atransport* acquire_one_transport(TransportType type, const char* serial, Transp } lock.unlock(); - // Don't return unauthorized devices; the caller can't do anything with them. - if (result && result->GetConnectionState() == kCsUnauthorized && !accept_any_state) { - *error_out = "device unauthorized.\n"; - char* ADB_VENDOR_KEYS = getenv("ADB_VENDOR_KEYS"); - *error_out += "This adb server's $ADB_VENDOR_KEYS is "; - *error_out += ADB_VENDOR_KEYS ? ADB_VENDOR_KEYS : "not set"; - *error_out += "\n"; - *error_out += "Try 'adb kill-server' if that seems wrong.\n"; - *error_out += "Otherwise check for a confirmation dialog on your device."; - result = nullptr; - } + if (result && !accept_any_state) { + // The caller requires an active transport. + // Make sure that we're actually connected. + ConnectionState state = result->GetConnectionState(); + switch (state) { + case kCsConnecting: + *error_out = "device still connecting"; + result = nullptr; + break; + + case kCsAuthorizing: + *error_out = "device still authorizing"; + result = nullptr; + break; - // Don't return offline devices; the caller can't do anything with them. - if (result && result->GetConnectionState() == kCsOffline && !accept_any_state) { - *error_out = "device offline"; - result = nullptr; + case kCsUnauthorized: { + *error_out = "device unauthorized.\n"; + char* ADB_VENDOR_KEYS = getenv("ADB_VENDOR_KEYS"); + *error_out += "This adb server's $ADB_VENDOR_KEYS is "; + *error_out += ADB_VENDOR_KEYS ? ADB_VENDOR_KEYS : "not set"; + *error_out += "\n"; + *error_out += "Try 'adb kill-server' if that seems wrong.\n"; + *error_out += "Otherwise check for a confirmation dialog on your device."; + result = nullptr; + break; + } + + case kCsOffline: + *error_out = "device offline"; + result = nullptr; + break; + + default: + break; + } } if (result) { @@ -802,6 +821,10 @@ std::string atransport::connection_state_name() const { return "sideload"; case kCsUnauthorized: return "unauthorized"; + case kCsAuthorizing: + return "authorizing"; + case kCsConnecting: + return "connecting"; default: return "unknown"; } @@ -1080,7 +1103,7 @@ void kick_all_tcp_devices() { void register_usb_transport(usb_handle* usb, const char* serial, const char* devpath, unsigned writeable) { - atransport* t = new atransport((writeable ? kCsOffline : kCsNoPerm)); + atransport* t = new atransport((writeable ? kCsConnecting : kCsNoPerm)); D("transport: %p init'ing for usb_handle %p (sn='%s')", t, usb, serial ? serial : ""); init_usb_transport(t, usb); diff --git a/adb/transport.h b/adb/transport.h index ebc186bf9..e1cbc092d 100644 --- a/adb/transport.h +++ b/adb/transport.h @@ -198,7 +198,7 @@ class atransport { // class in one go is a very large change. Given how bad our testing is, // it's better to do this piece by piece. - atransport(ConnectionState state = kCsOffline) + atransport(ConnectionState state = kCsConnecting) : id(NextTransportId()), connection_state_(state), connection_waitable_(std::make_shared<ConnectionWaitable>()), |