Merge "adb: parse tcp socket specs with base::ParseNetAddress."
diff --git a/adb/Android.mk b/adb/Android.mk
index b2a0dc4..0114ca3 100644
--- a/adb/Android.mk
+++ b/adb/Android.mk
@@ -62,6 +62,7 @@
adb_listeners_test.cpp \
adb_utils_test.cpp \
fdevent_test.cpp \
+ socket_spec_test.cpp \
socket_test.cpp \
sysdeps_test.cpp \
sysdeps/stat_test.cpp \
diff --git a/adb/socket_spec.cpp b/adb/socket_spec.cpp
index 18e6e6d..14eb16b 100644
--- a/adb/socket_spec.cpp
+++ b/adb/socket_spec.cpp
@@ -20,6 +20,8 @@
#include <unordered_map>
#include <vector>
+#include <android-base/parseint.h>
+#include <android-base/parsenetaddress.h>
#include <android-base/stringprintf.h>
#include <android-base/strings.h>
#include <cutils/sockets.h>
@@ -62,55 +64,47 @@
{ "localfilesystem", { ANDROID_SOCKET_NAMESPACE_FILESYSTEM, !ADB_WINDOWS } },
});
-static bool parse_tcp_spec(const std::string& spec, std::string* hostname, int* port,
+bool parse_tcp_socket_spec(const std::string& spec, std::string* hostname, int* port,
std::string* error) {
- std::vector<std::string> fragments = android::base::Split(spec, ":");
- if (fragments.size() == 1 || fragments.size() > 3) {
- *error = StringPrintf("invalid tcp specification: '%s'", spec.c_str());
- return false;
- }
-
- if (fragments[0] != "tcp") {
+ if (!StartsWith(spec, "tcp:")) {
*error = StringPrintf("specification is not tcp: '%s'", spec.c_str());
return false;
}
- // strtol accepts leading whitespace.
- const std::string& port_str = fragments.back();
- if (port_str.empty() || port_str[0] < '0' || port_str[0] > '9') {
- *error = StringPrintf("invalid port '%s'", port_str.c_str());
- return false;
- }
+ std::string hostname_value;
+ int port_value;
- char* parsed_end;
- long parsed_port = strtol(port_str.c_str(), &parsed_end, 10);
- if (*parsed_end != '\0') {
- *error = StringPrintf("trailing chars in port: '%s'", port_str.c_str());
- return false;
- }
- if (parsed_port > 65535) {
- *error = StringPrintf("invalid port %ld", parsed_port);
- return false;
- }
-
- // tcp:123 is valid, tcp::123 isn't.
- if (fragments.size() == 2) {
- // Empty hostname.
- if (hostname) {
- *hostname = "";
- }
- } else {
- if (fragments[1].empty()) {
- *error = StringPrintf("empty host in '%s'", spec.c_str());
+ // If the spec is tcp:<port>, parse it ourselves.
+ // Otherwise, delegate to android::base::ParseNetAddress.
+ if (android::base::ParseInt(&spec[4], &port_value)) {
+ // Do the range checking ourselves, because ParseInt rejects 'tcp:65536' and 'tcp:foo:1234'
+ // identically.
+ if (port_value < 0 || port_value > 65535) {
+ *error = StringPrintf("bad port number '%d'", port_value);
return false;
}
- if (hostname) {
- *hostname = fragments[1];
+ } else {
+ std::string addr = spec.substr(4);
+ port_value = -1;
+
+ // FIXME: ParseNetAddress rejects port 0. This currently doesn't hurt, because listening
+ // on an address that isn't 'localhost' is unsupported.
+ if (!android::base::ParseNetAddress(addr, &hostname_value, &port_value, nullptr, error)) {
+ return false;
}
+
+ if (port_value == -1) {
+ *error = StringPrintf("missing port in specification: '%s'", spec.c_str());
+ return false;
+ }
+ }
+
+ if (hostname) {
+ *hostname = std::move(hostname_value);
}
if (port) {
- *port = parsed_port;
+ *port = port_value;
}
return true;
@@ -141,7 +135,7 @@
std::string error;
std::string hostname;
- if (!parse_tcp_spec(spec, &hostname, nullptr, &error)) {
+ if (!parse_tcp_socket_spec(spec, &hostname, nullptr, &error)) {
return false;
}
return tcp_host_is_local(hostname);
@@ -151,7 +145,7 @@
if (StartsWith(spec, "tcp:")) {
std::string hostname;
int port;
- if (!parse_tcp_spec(spec, &hostname, &port, error)) {
+ if (!parse_tcp_socket_spec(spec, &hostname, &port, error)) {
return -1;
}
@@ -196,7 +190,7 @@
if (StartsWith(spec, "tcp:")) {
std::string hostname;
int port;
- if (!parse_tcp_spec(spec, &hostname, &port, error)) {
+ if (!parse_tcp_socket_spec(spec, &hostname, &port, error)) {
return -1;
}
diff --git a/adb/socket_spec.h b/adb/socket_spec.h
index 6302da5..6920e91 100644
--- a/adb/socket_spec.h
+++ b/adb/socket_spec.h
@@ -25,3 +25,7 @@
int socket_spec_connect(const std::string& spec, std::string* error);
int socket_spec_listen(const std::string& spec, std::string* error,
int* resolved_tcp_port = nullptr);
+
+// Exposed for testing.
+bool parse_tcp_socket_spec(const std::string& spec, std::string* hostname, int* port,
+ std::string* error);
diff --git a/adb/socket_spec_test.cpp b/adb/socket_spec_test.cpp
new file mode 100644
index 0000000..40ce21c
--- /dev/null
+++ b/adb/socket_spec_test.cpp
@@ -0,0 +1,54 @@
+/*
+ * 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 "socket_spec.h"
+
+#include <string>
+
+#include <gtest/gtest.h>
+
+TEST(socket_spec, parse_tcp_socket_spec) {
+ std::string hostname, error;
+ int port;
+ EXPECT_TRUE(parse_tcp_socket_spec("tcp:5037", &hostname, &port, &error));
+ EXPECT_EQ("", hostname);
+ EXPECT_EQ(5037, port);
+
+ // Bad ports:
+ EXPECT_FALSE(parse_tcp_socket_spec("tcp:", &hostname, &port, &error));
+ EXPECT_FALSE(parse_tcp_socket_spec("tcp:-1", &hostname, &port, &error));
+ EXPECT_FALSE(parse_tcp_socket_spec("tcp:65536", &hostname, &port, &error));
+
+ EXPECT_TRUE(parse_tcp_socket_spec("tcp:localhost:1234", &hostname, &port, &error));
+ EXPECT_EQ("localhost", hostname);
+ EXPECT_EQ(1234, port);
+
+ EXPECT_FALSE(parse_tcp_socket_spec("tcp:localhost", &hostname, &port, &error));
+ EXPECT_FALSE(parse_tcp_socket_spec("tcp:localhost:", &hostname, &port, &error));
+ EXPECT_FALSE(parse_tcp_socket_spec("tcp:localhost:-1", &hostname, &port, &error));
+ EXPECT_FALSE(parse_tcp_socket_spec("tcp:localhost:65536", &hostname, &port, &error));
+
+ // IPv6:
+ EXPECT_TRUE(parse_tcp_socket_spec("tcp:[::1]:1234", &hostname, &port, &error));
+ EXPECT_EQ("::1", hostname);
+ EXPECT_EQ(1234, port);
+
+ EXPECT_FALSE(parse_tcp_socket_spec("tcp:[::1]", &hostname, &port, &error));
+ EXPECT_FALSE(parse_tcp_socket_spec("tcp:[::1]:", &hostname, &port, &error));
+ EXPECT_FALSE(parse_tcp_socket_spec("tcp:[::1]:-1", &hostname, &port, &error));
+ EXPECT_FALSE(parse_tcp_socket_spec("tcp:::1", &hostname, &port, &error));
+ EXPECT_FALSE(parse_tcp_socket_spec("tcp:::1:1234", &hostname, &port, &error));
+}