From 64871bfea254b7694518438920c3376b97235754 Mon Sep 17 00:00:00 2001 From: Soheil Hassas Yeganeh Date: Tue, 2 Jul 2019 14:30:34 -0400 Subject: [PATCH 1/3] Revert "Fix stale comment in split host port." This reverts commit bf9b4c257bdd5f5765fcd9b6c9402d170f75fea8. --- src/core/lib/gprpp/host_port.cc | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/src/core/lib/gprpp/host_port.cc b/src/core/lib/gprpp/host_port.cc index e7f0e4461e9..3fa4719f49a 100644 --- a/src/core/lib/gprpp/host_port.cc +++ b/src/core/lib/gprpp/host_port.cc @@ -105,9 +105,8 @@ bool SplitHostPort(StringView name, UniquePtr* host, bool has_port; const bool ret = DoSplitHostPort(name, &host_view, &port_view, &has_port); if (ret) { - // We always set the host, but port is set only when DoSplitHostPort find a - // port in the string, to remain backward compatible with the old - // gpr_split_host_port API. + // We always set the host, but port is set only when it's non-empty, + // to remain backward compatible with the old split_host_port API. *host = host_view.dup(); if (has_port) { *port = port_view.dup(); From 6376cc9b8f4c1b684ac6b551cb9a8ea1705acd5a Mon Sep 17 00:00:00 2001 From: Soheil Hassas Yeganeh Date: Tue, 2 Jul 2019 14:30:41 -0400 Subject: [PATCH 2/3] Revert "Return empty strings on optional ports for backward compatibility." This reverts commit 01b82d3a39f2d4455025e9176dae7917a82babb2. --- src/core/lib/gprpp/host_port.cc | 18 +++--------------- test/core/gprpp/host_port_test.cc | 2 -- 2 files changed, 3 insertions(+), 17 deletions(-) diff --git a/src/core/lib/gprpp/host_port.cc b/src/core/lib/gprpp/host_port.cc index 3fa4719f49a..13b63eb902b 100644 --- a/src/core/lib/gprpp/host_port.cc +++ b/src/core/lib/gprpp/host_port.cc @@ -44,10 +44,7 @@ int JoinHostPort(UniquePtr* out, const char* host, int port) { return ret; } -namespace { -bool DoSplitHostPort(StringView name, StringView* host, StringView* port, - bool* has_port) { - *has_port = false; +bool SplitHostPort(StringView name, StringView* host, StringView* port) { if (name[0] == '[') { /* Parse a bracketed host, typically an IPv6 literal. */ const size_t rbracket = name.find(']', 1); @@ -61,7 +58,6 @@ bool DoSplitHostPort(StringView name, StringView* host, StringView* port, } else if (name[rbracket + 1] == ':') { /* ]: */ *port = name.substr(rbracket + 2, name.size() - rbracket - 2); - *has_port = true; } else { /* ] */ return false; @@ -80,7 +76,6 @@ bool DoSplitHostPort(StringView name, StringView* host, StringView* port, /* Exactly 1 colon. Split into host:port. */ *host = name.substr(0, colon); *port = name.substr(colon + 1, name.size() - colon - 1); - *has_port = true; } else { /* 0 or 2+ colons. Bare hostname or IPv6 litearal. */ *host = name; @@ -89,12 +84,6 @@ bool DoSplitHostPort(StringView name, StringView* host, StringView* port, } return true; } -} // namespace - -bool SplitHostPort(StringView name, StringView* host, StringView* port) { - bool unused; - return DoSplitHostPort(name, host, port, &unused); -} bool SplitHostPort(StringView name, UniquePtr* host, UniquePtr* port) { @@ -102,13 +91,12 @@ bool SplitHostPort(StringView name, UniquePtr* host, GPR_DEBUG_ASSERT(port != nullptr && *port == nullptr); StringView host_view; StringView port_view; - bool has_port; - const bool ret = DoSplitHostPort(name, &host_view, &port_view, &has_port); + const bool ret = SplitHostPort(name, &host_view, &port_view); if (ret) { // We always set the host, but port is set only when it's non-empty, // to remain backward compatible with the old split_host_port API. *host = host_view.dup(); - if (has_port) { + if (!port_view.empty()) { *port = port_view.dup(); } } diff --git a/test/core/gprpp/host_port_test.cc b/test/core/gprpp/host_port_test.cc index cfe0eddb036..3b392da66e8 100644 --- a/test/core/gprpp/host_port_test.cc +++ b/test/core/gprpp/host_port_test.cc @@ -71,9 +71,7 @@ static void test_split_host_port() { split_host_port_expect("", "", nullptr, true); split_host_port_expect("[a:b]", "a:b", nullptr, true); split_host_port_expect("1.2.3.4", "1.2.3.4", nullptr, true); - split_host_port_expect("0.0.0.0:", "0.0.0.0", "", true); split_host_port_expect("a:b:c::", "a:b:c::", nullptr, true); - split_host_port_expect("[a:b:c::]:", "a:b:c::", "", true); split_host_port_expect("[a:b]:30", "a:b", "30", true); split_host_port_expect("1.2.3.4:30", "1.2.3.4", "30", true); split_host_port_expect(":30", "", "30", true); From def083b2c84f03d588bf0f9e75323f270c549676 Mon Sep 17 00:00:00 2001 From: Soheil Hassas Yeganeh Date: Tue, 2 Jul 2019 14:37:54 -0400 Subject: [PATCH 3/3] Clearly callout the behavior for listening ports. This is to clarify that the port number is a required part of the listening address. --- include/grpcpp/server_builder_impl.h | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/include/grpcpp/server_builder_impl.h b/include/grpcpp/server_builder_impl.h index 7f85e807d07..9c69adc9147 100644 --- a/include/grpcpp/server_builder_impl.h +++ b/include/grpcpp/server_builder_impl.h @@ -112,6 +112,10 @@ class ServerBuilder { /// /// It can be invoked multiple times. /// + /// If port is not provided in the \a addr (e.g., "1.2.3.4:" or "1.2.3.4"), + /// the default port (i.e., https) is used. To request an ephemeral port, + /// \a addr must include 0 as the port number (e.g., "1.2.3.4:0"). + /// /// \param addr_uri The address to try to bind to the server in URI form. If /// the scheme name is omitted, "dns:///" is assumed. To bind to any address, /// please use IPv6 any, i.e., [::]:, which also accepts IPv4