Merge pull request facebook#58 from anirbanr-fb/host_port_fix

make HostPort.ToString compatible with RFC 3986
Hakai-Shin · Jul 12, 2020 · d0c15ec · d0c15ec
2 parents 61b9a28 + 6d9ca5e
commit d0c15ec
Show file tree

Hide file tree

Showing 4 changed files with 21 additions and 6 deletions.
diff --git a/src/kudu/util/net/net_util-test.cc b/src/kudu/util/net/net_util-test.cc
@@ -74,13 +74,13 @@ TEST_F(NetUtilTest, TestParseAddresses) {
   ASSERT_OK(DoParseBindAddresses("[::]:12345", &ret));
   // TODO(mpercy): If this requires square brackets to parse it should generate
   // them as well. For now, it does not.
-  ASSERT_EQ(":::12345", ret);
+  ASSERT_EQ("[::]:12345", ret);
 
   ASSERT_OK(DoParseBindAddresses("[::]", &ret));
-  ASSERT_EQ(":::7150", ret);
+  ASSERT_EQ("[::]:7150", ret);
 
   ASSERT_OK(DoParseBindAddresses("[::]:12345, [::]:12346", &ret));
-  ASSERT_EQ(":::12345,:::12346", ret);
+  ASSERT_EQ("[::]:12345,[::]:12346", ret);
 
   // Test some invalid addresses.
   Status s = DoParseBindAddresses("[::]:xyz", &ret);
@@ -100,7 +100,7 @@ TEST_F(NetUtilTest, TestResolveAddresses) {
   ASSERT_TRUE(!addrs.empty());
   for (const Sockaddr& addr : addrs) {
     LOG(INFO) << "Address: " << addr.ToString();
-    ASSERT_STR_MATCHES(addr.ToString(), "^(127\\.|::1)");
+    ASSERT_STR_MATCHES(addr.ToString(), "^(127\\.|\\[::1])");
     ASSERT_STR_MATCHES(addr.ToString(), ":12345$");
     EXPECT_TRUE(addr.IsAnyLocalAddress());
   }

diff --git a/src/kudu/util/net/net_util.cc b/src/kudu/util/net/net_util.cc
@@ -125,6 +125,14 @@ size_t HostPort::HashCode() const {
   return seed;
 }
 
+bool HostPort::IsHostIPV6Address() const {
+  if (!Initialized())
+    return false;
+
+  int num_colons = strcount(host_, ':');
+  return (num_colons > 1);
+}
+
 Status HostPort::ParseString(const string& str, uint16_t default_port) {
   /*
     The `str` param is supposed to take the following combinations:
@@ -231,7 +239,10 @@ Status HostPort::ParseStrings(const string& comma_sep_addrs,
 }
 
 string HostPort::ToString() const {
-  return Substitute("$0:$1", host_, port_);
+  // to be compatible with RFC 3986, [2001:db8:1f70::999:de8:7648:6e8]:100
+  // style of host:port
+  return IsHostIPV6Address() ? Substitute("[$0]:$1", host_, port_) :
+      Substitute("$0:$1", host_, port_);
 }
 
 string HostPort::ToCommaSeparatedString(const vector<HostPort>& hostports) {

diff --git a/src/kudu/util/net/net_util.h b/src/kudu/util/net/net_util.h
@@ -50,6 +50,10 @@ class HostPort {
   // the host/port pair can be resolved, without returning anything.
   Status ResolveAddresses(std::vector<Sockaddr>* addresses) const;
 
+  // In the common case, hostnames are not used in fb,
+  // but ipv6 addresses
+  bool IsHostIPV6Address() const;
+
   std::string ToString() const;
 
   const std::string& host() const { return host_; }

diff --git a/src/kudu/util/net/sockaddr.cc b/src/kudu/util/net/sockaddr.cc
@@ -104,7 +104,7 @@ const struct sockaddr_in6& Sockaddr::addr() const {
 }
 
 std::string Sockaddr::ToString() const {
-  return Substitute("$0:$1", host(), port());
+  return Substitute("[$0]:$1", host(), port());
 }
 
 // Return true iff two ipv6 address structs are equal.