Add feature list to connection banner.
This allows us to test for features explicitly rather than relying on
the protocol version number, allowing us to fall back gracefully if a
feature is not supported.
This will be needed for the upcoming shell upgrades for stdout/stderr
separation and exit code reporting.
Change-Id: Ibb1d8ad2611f7209901ee76d51346b453e9c5873
diff --git a/adb/adb.cpp b/adb/adb.cpp
index a8737f5..180954b 100644
--- a/adb/adb.cpp
+++ b/adb/adb.cpp
@@ -58,6 +58,8 @@
#if !ADB_HOST
const char* adb_device_banner = "device";
static android::base::LogdLogger gLogdLogger;
+#else
+const char* adb_device_banner = "host";
#endif
void AdbLogger(android::base::LogId id, android::base::LogSeverity severity,
@@ -305,45 +307,50 @@
send_packet(p, t);
}
-static size_t fill_connect_data(char *buf, size_t bufsize)
-{
-#if ADB_HOST
- return snprintf(buf, bufsize, "host::") + 1;
-#else
- static const char *cnxn_props[] = {
+std::string get_connection_string() {
+ std::vector<std::string> connection_properties;
+
+#if !ADB_HOST
+ static const char* cnxn_props[] = {
"ro.product.name",
"ro.product.model",
"ro.product.device",
};
- static const int num_cnxn_props = ARRAY_SIZE(cnxn_props);
- int i;
- size_t remaining = bufsize;
- size_t len;
- len = snprintf(buf, remaining, "%s::", adb_device_banner);
- remaining -= len;
- buf += len;
- for (i = 0; i < num_cnxn_props; i++) {
+ for (const auto& prop_name : cnxn_props) {
char value[PROPERTY_VALUE_MAX];
- property_get(cnxn_props[i], value, "");
- len = snprintf(buf, remaining, "%s=%s;", cnxn_props[i], value);
- remaining -= len;
- buf += len;
+ property_get(prop_name, value, "");
+ connection_properties.push_back(
+ android::base::StringPrintf("%s=%s", prop_name, value));
}
-
- return bufsize - remaining + 1;
#endif
+
+ connection_properties.push_back(android::base::StringPrintf(
+ "features=%s", android::base::Join(supported_features(), ',').c_str()));
+
+ return android::base::StringPrintf(
+ "%s::%s", adb_device_banner,
+ android::base::Join(connection_properties, ';').c_str());
}
-void send_connect(atransport *t)
-{
+void send_connect(atransport* t) {
D("Calling send_connect \n");
- apacket *cp = get_apacket();
+ apacket* cp = get_apacket();
cp->msg.command = A_CNXN;
cp->msg.arg0 = t->get_protocol_version();
cp->msg.arg1 = t->get_max_payload();
- cp->msg.data_length = fill_connect_data((char *)cp->data,
- MAX_PAYLOAD_V1);
+
+ std::string connection_str = get_connection_string();
+ // Connect and auth packets are limited to MAX_PAYLOAD_V1 because we don't
+ // yet know how much data the other size is willing to accept.
+ if (connection_str.length() > MAX_PAYLOAD_V1) {
+ LOG(FATAL) << "Connection banner is too long (length = "
+ << connection_str.length() << ")";
+ }
+
+ memcpy(cp->data, connection_str.c_str(), connection_str.length());
+ cp->msg.data_length = connection_str.length();
+
send_packet(cp, t);
}
@@ -356,8 +363,8 @@
*dst = strdup(src.c_str());
}
-void parse_banner(const char* banner, atransport* t) {
- D("parse_banner: %s\n", banner);
+void parse_banner(const std::string& banner, atransport* t) {
+ D("parse_banner: %s\n", banner.c_str());
// The format is something like:
// "device::ro.product.name=x;ro.product.model=y;ro.product.device=z;".
@@ -380,6 +387,10 @@
qual_overwrite(&t->model, value);
} else if (key == "ro.product.device") {
qual_overwrite(&t->device, value);
+ } else if (key == "features") {
+ for (const auto& feature : android::base::Split(value, ",")) {
+ t->add_feature(feature);
+ }
}
}
}
@@ -407,6 +418,29 @@
}
}
+static void handle_new_connection(atransport* t, apacket* p) {
+ if (t->connection_state != kCsOffline) {
+ t->connection_state = kCsOffline;
+ handle_offline(t);
+ }
+
+ t->update_version(p->msg.arg0, p->msg.arg1);
+ std::string banner(reinterpret_cast<const char*>(p->data),
+ p->msg.data_length);
+ parse_banner(banner, t);
+
+#if ADB_HOST
+ handle_online(t);
+#else
+ if (!auth_required) {
+ handle_online(t);
+ send_connect(t);
+ } else {
+ send_auth_request(t);
+ }
+#endif
+}
+
void handle_packet(apacket *p, atransport *t)
{
asocket *s;
@@ -431,25 +465,8 @@
}
return;
- case A_CNXN: /* CONNECT(version, maxdata, "system-id-string") */
- if(t->connection_state != kCsOffline) {
- t->connection_state = kCsOffline;
- handle_offline(t);
- }
-
- t->update_version(p->msg.arg0, p->msg.arg1);
- parse_banner(reinterpret_cast<const char*>(p->data), t);
-
-#if ADB_HOST
- handle_online(t);
-#else
- if (!auth_required) {
- handle_online(t);
- send_connect(t);
- } else {
- send_auth_request(t);
- }
-#endif
+ case A_CNXN: // CONNECT(version, maxdata, "system-id-string")
+ handle_new_connection(t, p);
break;
case A_AUTH:
diff --git a/adb/adb.h b/adb/adb.h
index 9ff830e..6855f3b 100644
--- a/adb/adb.h
+++ b/adb/adb.h
@@ -20,10 +20,10 @@
#include <limits.h>
#include <sys/types.h>
-#include <base/macros.h>
-
#include <string>
+#include <base/macros.h>
+
#include "adb_trace.h"
#include "fdevent.h"
@@ -191,71 +191,6 @@
kCsUnauthorized,
};
-class atransport {
-public:
- // TODO(danalbert): We expose waaaaaaay too much stuff because this was
- // historically just a struct, but making the whole thing a more idiomatic
- // 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() {
- auth_fde = {};
- transport_fde = {};
- protocol_version = A_VERSION;
- max_payload = MAX_PAYLOAD;
- }
-
- virtual ~atransport() {}
-
- int (*read_from_remote)(apacket* p, atransport* t) = nullptr;
- int (*write_to_remote)(apacket* p, atransport* t) = nullptr;
- void (*close)(atransport* t) = nullptr;
- void (*kick)(atransport* t) = nullptr;
-
- int fd = -1;
- int transport_socket = -1;
- fdevent transport_fde;
- int ref_count = 0;
- uint32_t sync_token = 0;
- ConnectionState connection_state = kCsOffline;
- bool online = false;
- TransportType type = kTransportAny;
-
- // USB handle or socket fd as needed.
- usb_handle* usb = nullptr;
- int sfd = -1;
-
- // Used to identify transports for clients.
- char* serial = nullptr;
- char* product = nullptr;
- char* model = nullptr;
- char* device = nullptr;
- char* devpath = nullptr;
- int adb_port = -1; // Use for emulators (local transport)
- bool kicked = false;
-
- // A list of adisconnect callbacks called when the transport is kicked.
- adisconnect disconnects = {};
-
- void* key = nullptr;
- unsigned char token[TOKEN_SIZE] = {};
- fdevent auth_fde;
- size_t failed_auth_attempts = 0;
-
- const char* connection_state_name() const;
-
- void update_version(int version, size_t payload);
- int get_protocol_version() const;
- size_t get_max_payload() const;
-
-private:
- int protocol_version;
- size_t max_payload;
-
- DISALLOW_COPY_AND_ASSIGN(atransport);
-};
-
-
/* 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
@@ -380,7 +315,8 @@
ConnectionState connection_state(atransport *t);
-extern const char *adb_device_banner;
+extern const char* adb_device_banner;
+
#if !ADB_HOST
extern int SHELL_EXIT_NOTIFY_FD;
#endif // !ADB_HOST
@@ -405,4 +341,6 @@
void send_connect(atransport *t);
+void parse_banner(const std::string&, atransport* t);
+
#endif
diff --git a/adb/transport.cpp b/adb/transport.cpp
index 4a273c4..2ea4d44 100644
--- a/adb/transport.cpp
+++ b/adb/transport.cpp
@@ -29,6 +29,7 @@
#include <list>
#include <base/stringprintf.h>
+#include <base/strings.h>
#include "adb.h"
#include "adb_utils.h"
@@ -535,7 +536,7 @@
t = m.transport;
- if(m.action == 0){
+ if (m.action == 0) {
D("transport: %s removing and free'ing %d\n", t->serial, t->transport_socket);
/* IMPORTANT: the remove closes one half of the
@@ -845,6 +846,29 @@
return max_payload;
}
+// The list of features supported by the current system. Will be sent to the
+// other side of the connection in the banner.
+static const FeatureSet gSupportedFeatures = {
+ // None yet.
+};
+
+const FeatureSet& supported_features() {
+ return gSupportedFeatures;
+}
+
+bool atransport::has_feature(const std::string& feature) const {
+ return features_.count(feature) > 0;
+}
+
+void atransport::add_feature(const std::string& feature) {
+ features_.insert(feature);
+}
+
+bool atransport::CanUseFeature(const std::string& feature) const {
+ return has_feature(feature) &&
+ supported_features().count(feature) > 0;
+}
+
#if ADB_HOST
static void append_transport_info(std::string* result, const char* key,
@@ -879,6 +903,9 @@
append_transport_info(result, "product:", t->product, false);
append_transport_info(result, "model:", t->model, true);
append_transport_info(result, "device:", t->device, false);
+ append_transport_info(result, "features:",
+ android::base::Join(t->features(), ',').c_str(),
+ false);
}
*result += '\n';
}
diff --git a/adb/transport.h b/adb/transport.h
index edcc99d..e809407 100644
--- a/adb/transport.h
+++ b/adb/transport.h
@@ -20,9 +20,92 @@
#include <sys/types.h>
#include <string>
+#include <unordered_set>
#include "adb.h"
+typedef std::unordered_set<std::string> FeatureSet;
+
+const FeatureSet& supported_features();
+
+class atransport {
+public:
+ // TODO(danalbert): We expose waaaaaaay too much stuff because this was
+ // historically just a struct, but making the whole thing a more idiomatic
+ // 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() {
+ auth_fde = {};
+ transport_fde = {};
+ protocol_version = A_VERSION;
+ max_payload = MAX_PAYLOAD;
+ }
+
+ virtual ~atransport() {}
+
+ int (*read_from_remote)(apacket* p, atransport* t) = nullptr;
+ int (*write_to_remote)(apacket* p, atransport* t) = nullptr;
+ void (*close)(atransport* t) = nullptr;
+ void (*kick)(atransport* t) = nullptr;
+
+ int fd = -1;
+ int transport_socket = -1;
+ fdevent transport_fde;
+ int ref_count = 0;
+ uint32_t sync_token = 0;
+ ConnectionState connection_state = kCsOffline;
+ bool online = false;
+ TransportType type = kTransportAny;
+
+ // USB handle or socket fd as needed.
+ usb_handle* usb = nullptr;
+ int sfd = -1;
+
+ // Used to identify transports for clients.
+ char* serial = nullptr;
+ char* product = nullptr;
+ char* model = nullptr;
+ char* device = nullptr;
+ char* devpath = nullptr;
+ int adb_port = -1; // Use for emulators (local transport)
+ bool kicked = false;
+
+ // A list of adisconnect callbacks called when the transport is kicked.
+ adisconnect disconnects = {};
+
+ void* key = nullptr;
+ unsigned char token[TOKEN_SIZE] = {};
+ fdevent auth_fde;
+ size_t failed_auth_attempts = 0;
+
+ const char* connection_state_name() const;
+
+ void update_version(int version, size_t payload);
+ int get_protocol_version() const;
+ size_t get_max_payload() const;
+
+ inline const FeatureSet features() const {
+ return features_;
+ }
+
+ bool has_feature(const std::string& feature) const;
+ void add_feature(const std::string& feature);
+
+ // Returns true if both we and the other end of the transport support the
+ // feature.
+ bool CanUseFeature(const std::string& feature) const;
+
+private:
+ // A set of features transmitted in the banner with the initial connection.
+ // This is stored in the banner as 'features=feature0,feature1,etc'.
+ FeatureSet features_;
+ int protocol_version;
+ size_t max_payload;
+
+ DISALLOW_COPY_AND_ASSIGN(atransport);
+};
+
/*
* Obtain a transport from the available transports.
* If state is != kCsAny, only transports in that state are considered.
diff --git a/adb/transport_test.cpp b/adb/transport_test.cpp
index 49deb73..743d97d 100644
--- a/adb/transport_test.cpp
+++ b/adb/transport_test.cpp
@@ -59,6 +59,8 @@
EXPECT_EQ(0, memcmp(&auth_fde, &rhs.auth_fde, sizeof(fdevent)));
EXPECT_EQ(failed_auth_attempts, rhs.failed_auth_attempts);
+ EXPECT_EQ(features(), rhs.features());
+
return true;
}
};
@@ -120,6 +122,73 @@
// want to make sure I understand how this is working at all before I try fixing
// that.
TEST(transport, DISABLED_run_transport_disconnects_zeroed_atransport) {
- atransport t;
- run_transport_disconnects(&t);
+ atransport t;
+ run_transport_disconnects(&t);
+}
+
+TEST(transport, add_feature) {
+ atransport t;
+ ASSERT_EQ(0U, t.features().size());
+
+ t.add_feature("foo");
+ ASSERT_EQ(1U, t.features().size());
+ ASSERT_TRUE(t.has_feature("foo"));
+
+ t.add_feature("bar");
+ ASSERT_EQ(2U, t.features().size());
+ ASSERT_TRUE(t.has_feature("foo"));
+ ASSERT_TRUE(t.has_feature("bar"));
+
+ t.add_feature("foo");
+ ASSERT_EQ(2U, t.features().size());
+ ASSERT_TRUE(t.has_feature("foo"));
+ ASSERT_TRUE(t.has_feature("bar"));
+}
+
+TEST(transport, parse_banner_no_features) {
+ atransport t;
+
+ parse_banner("host::", &t);
+
+ ASSERT_EQ(0U, t.features().size());
+ ASSERT_EQ(kCsHost, t.connection_state);
+
+ ASSERT_EQ(nullptr, t.product);
+ ASSERT_EQ(nullptr, t.model);
+ ASSERT_EQ(nullptr, t.device);
+}
+
+TEST(transport, parse_banner_product_features) {
+ atransport t;
+
+ const char banner[] =
+ "host::ro.product.name=foo;ro.product.model=bar;ro.product.device=baz;";
+ parse_banner(banner, &t);
+
+ ASSERT_EQ(kCsHost, t.connection_state);
+
+ ASSERT_EQ(0U, t.features().size());
+
+ ASSERT_EQ(std::string("foo"), t.product);
+ ASSERT_EQ(std::string("bar"), t.model);
+ ASSERT_EQ(std::string("baz"), t.device);
+}
+
+TEST(transport, parse_banner_features) {
+ atransport t;
+
+ const char banner[] =
+ "host::ro.product.name=foo;ro.product.model=bar;ro.product.device=baz;"
+ "features=woodly,doodly";
+ parse_banner(banner, &t);
+
+ ASSERT_EQ(kCsHost, t.connection_state);
+
+ ASSERT_EQ(2U, t.features().size());
+ ASSERT_TRUE(t.has_feature("woodly"));
+ ASSERT_TRUE(t.has_feature("doodly"));
+
+ ASSERT_EQ(std::string("foo"), t.product);
+ ASSERT_EQ(std::string("bar"), t.model);
+ ASSERT_EQ(std::string("baz"), t.device);
}