From c9ec1a64edd64b9a56b991d1d43544c0829fe746 Mon Sep 17 00:00:00 2001 From: Soheil Hassas Yeganeh Date: Thu, 27 Jun 2019 11:20:30 -0400 Subject: [PATCH] Fix SplitHostPort for empty strings. Add unittests to catch such errors in the future. --- src/core/lib/gprpp/host_port.cc | 8 +++++-- test/core/gprpp/host_port_test.cc | 36 +++++++++++++++++++++++++++++++ 2 files changed, 42 insertions(+), 2 deletions(-) diff --git a/src/core/lib/gprpp/host_port.cc b/src/core/lib/gprpp/host_port.cc index f3f8a73602a..82a3ab1add0 100644 --- a/src/core/lib/gprpp/host_port.cc +++ b/src/core/lib/gprpp/host_port.cc @@ -87,11 +87,15 @@ bool SplitHostPort(StringView name, StringView* host, StringView* port) { bool SplitHostPort(StringView name, UniquePtr* host, UniquePtr* port) { + GPR_DEBUG_ASSERT(host != nullptr && *host == nullptr); + GPR_DEBUG_ASSERT(port != nullptr && *port == nullptr); StringView host_view; StringView port_view; const bool ret = SplitHostPort(name, &host_view, &port_view); - host->reset(host_view.empty() ? nullptr : host_view.dup().release()); - port->reset(port_view.empty() ? nullptr : port_view.dup().release()); + if (ret) { + *host = host_view.dup(); + *port = port_view.dup(); + } return ret; } } // namespace grpc_core diff --git a/test/core/gprpp/host_port_test.cc b/test/core/gprpp/host_port_test.cc index 3474e6dc494..33ea4781491 100644 --- a/test/core/gprpp/host_port_test.cc +++ b/test/core/gprpp/host_port_test.cc @@ -48,11 +48,47 @@ static void test_join_host_port_garbage(void) { join_host_port_expect("::]", 107, "[::]]:107"); } +static void split_host_port_expect(const char* name, const char* host, + const char* port, bool ret) { + grpc_core::UniquePtr actual_host; + grpc_core::UniquePtr actual_port; + const bool actual_ret = + grpc_core::SplitHostPort(name, &actual_host, &actual_port); + GPR_ASSERT(actual_ret == ret); + if (host == nullptr) { + GPR_ASSERT(actual_host == nullptr); + } else { + GPR_ASSERT(strcmp(host, actual_host.get()) == 0); + } + if (port == nullptr) { + GPR_ASSERT(actual_port == nullptr); + } else { + GPR_ASSERT(strcmp(port, actual_port.get()) == 0); + } +} + +static void test_split_host_port() { + split_host_port_expect("", "", "", true); + split_host_port_expect("[a:b]", "a:b", "", true); + split_host_port_expect("1.2.3.4", "1.2.3.4", "", 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); +} + +static void test_split_host_port_invalid() { + split_host_port_expect("[a:b", nullptr, nullptr, false); + split_host_port_expect("[a:b]30", nullptr, nullptr, false); +} + int main(int argc, char** argv) { grpc::testing::TestEnvironment env(argc, argv); test_join_host_port(); test_join_host_port_garbage(); + test_split_host_port(); + test_split_host_port_invalid(); return 0; }