address: removing the throwing variant of parseInternetAddress

Signed-off-by: Alyssa Wilk <alyssar@chromium.org>
alyssawilk · Jun 10, 2024 · 9269412 · 9269412
1 parent cf1b986
commit 9269412
Show file tree

Hide file tree

Showing 19 changed files with 72 additions and 54 deletions.
diff --git a/source/common/network/resolver_impl.cc b/source/common/network/resolver_impl.cc
@@ -25,13 +25,15 @@ class IpResolver : public Resolver {
     switch (socket_address.port_specifier_case()) {
     case envoy::config::core::v3::SocketAddress::PortSpecifierCase::kPortValue:
     // Default to port 0 if no port value is specified.
-    case envoy::config::core::v3::SocketAddress::PortSpecifierCase::PORT_SPECIFIER_NOT_SET:
+    case envoy::config::core::v3::SocketAddress::PortSpecifierCase::PORT_SPECIFIER_NOT_SET: {
       auto addr = Network::Utility::parseInternetAddressNoThrow(
           socket_address.address(), socket_address.port_value(), !socket_address.ipv4_compat());
       if (!addr) {
-        return absl::InvalidArgumentError(absl::StrCat("malformed IP address: ", socket_address.address()));
+        return absl::InvalidArgumentError(
+            absl::StrCat("malformed IP address: ", socket_address.address()));
       }
       return addr;
+    }
     case envoy::config::core::v3::SocketAddress::PortSpecifierCase::kNamedPort:
       break;
     }

diff --git a/source/common/network/utility.cc b/source/common/network/utility.cc
@@ -436,12 +436,12 @@ absl::uint128 Utility::flipOrder(const absl::uint128& input) {
 }
 
 Address::InstanceConstSharedPtr
-Utility::protobufAddressToAddress(const envoy::config::core::v3::Address& proto_address) {
+Utility::protobufAddressToAddressNoThrow(const envoy::config::core::v3::Address& proto_address) {
   switch (proto_address.address_case()) {
   case envoy::config::core::v3::Address::AddressCase::kSocketAddress:
-    return Utility::parseInternetAddress(proto_address.socket_address().address(),
-                                         proto_address.socket_address().port_value(),
-                                         !proto_address.socket_address().ipv4_compat());
+    return Utility::parseInternetAddressNoThrow(proto_address.socket_address().address(),
+                                                proto_address.socket_address().port_value(),
+                                                !proto_address.socket_address().ipv4_compat());
   case envoy::config::core::v3::Address::AddressCase::kPipe:
     return std::make_shared<Address::PipeInstance>(proto_address.pipe().path(),
                                                    proto_address.pipe().mode());

diff --git a/source/common/network/utility.h b/source/common/network/utility.h
@@ -263,8 +263,12 @@ class Utility {
    */
   static absl::uint128 Ip6htonl(const absl::uint128& address);
 
+  /**
+   * @param proto_address supplies the proto address to convert
+   * @return the InstanceConstSharedPtr for the address, or null if proto_address is invalid.
+   */
   static Address::InstanceConstSharedPtr
-  protobufAddressToAddress(const envoy::config::core::v3::Address& proto_address);
+  protobufAddressToAddressNoThrow(const envoy::config::core::v3::Address& proto_address);
 
   /**
    * Copies the address instance into the protobuf representation of an address.

diff --git a/source/common/upstream/upstream_impl.cc b/source/common/upstream/upstream_impl.cc
@@ -1793,7 +1793,7 @@ ClusterImplBase::resolveProtoAddress(const envoy::config::core::v3::Address& add
   absl::Status resolve_status;
   TRY_ASSERT_MAIN_THREAD {
     auto address_or_error = Network::Address::resolveProtoAddress(address);
-    if (address_or_error.value()) {
+    if (address_or_error.status().ok()) {
       return address_or_error.value();
     }
     resolve_status = address_or_error.status();

diff --git a/source/extensions/quic/server_preferred_address/fixed_server_preferred_address_config.cc b/source/extensions/quic/server_preferred_address/fixed_server_preferred_address_config.cc
@@ -34,7 +34,11 @@ quic::QuicSocketAddress parseSocketAddress(const envoy::config::core::v3::Socket
   // existing helpers.
   envoy::config::core::v3::Address outer;
   *outer.mutable_socket_address() = addr;
-  auto envoy_addr = Network::Utility::protobufAddressToAddress(outer);
+  auto envoy_addr = Network::Utility::protobufAddressToAddressNoThrow(outer);
+  if (!envoy_addr) {
+    ProtoExceptionUtil::throwProtoValidationException(absl::StrCat("Invalid address ", outer),
+                                                      message);
+  }
   ASSERT(envoy_addr != nullptr,
          "Network::Utility::protobufAddressToAddress throws on failure so this can't be nullptr");
   if (envoy_addr->ip() == nullptr || envoy_addr->ip()->version() != version) {

diff --git a/source/extensions/tracers/xray/daemon_broker.cc b/source/extensions/tracers/xray/daemon_broker.cc
@@ -32,7 +32,7 @@ DaemonBrokerImpl::DaemonBrokerImpl(const std::string& daemon_endpoint)
           Network::Utility::parseInternetAddressAndPortNoThrow(daemon_endpoint, false /*v6only*/)),
       io_handle_(Network::ioHandleForAddr(Network::Socket::Type::Datagram, address_, {})) {
   if (address_ == nullptr) {
-    throwEnvoyExceptionOrPanic(absl::StrCat("malformed IP address: ", daemon_endpoint));
+    throw EnvoyException(absl::StrCat("malformed IP address: ", daemon_endpoint));
   }
 }
 

diff --git a/test/common/listener_manager/listener_manager_impl_test.cc b/test/common/listener_manager/listener_manager_impl_test.cc
@@ -6519,7 +6519,8 @@ TEST_P(ListenerManagerImplWithRealFiltersTest, AddressResolver) {
   NiceMock<Network::MockAddressResolver> mock_resolver;
   EXPECT_CALL(mock_resolver, resolve(_))
       .Times(2)
-      .WillRepeatedly(Return(Network::Utility::parseInternetAddressNoThrow("127.0.0.1", 1111, false)));
+      .WillRepeatedly(
+          Return(Network::Utility::parseInternetAddressNoThrow("127.0.0.1", 1111, false)));
   Registry::InjectFactory<Network::Address::Resolver> register_resolver(mock_resolver);
 
   EXPECT_CALL(listener_factory_, createListenSocket(_, _, _, default_bind_type, _, 0));

diff --git a/test/common/network/address_impl_test.cc b/test/common/network/address_impl_test.cc
@@ -223,7 +223,8 @@ TEST(Ipv4InstanceTest, Broadcast) {
   EXPECT_EQ("255.255.255.255", address.ip()->addressAsString());
   EXPECT_EQ(0U, address.ip()->port());
   EXPECT_EQ(IpVersion::v4, address.ip()->version());
-  EXPECT_TRUE(addressesEqual(Network::Utility::parseInternetAddressNoThrow("255.255.255.255"), address));
+  EXPECT_TRUE(
+      addressesEqual(Network::Utility::parseInternetAddressNoThrow("255.255.255.255"), address));
   EXPECT_FALSE(address.ip()->isUnicastAddress());
 }
 
@@ -247,7 +248,8 @@ TEST(Ipv6InstanceTest, SocketAddress) {
   EXPECT_FALSE(address.ip()->isAnyAddress());
   EXPECT_EQ(32000U, address.ip()->port());
   EXPECT_EQ(IpVersion::v6, address.ip()->version());
-  EXPECT_TRUE(addressesEqual(Network::Utility::parseInternetAddressNoThrow("1:0023::0Ef"), address));
+  EXPECT_TRUE(
+      addressesEqual(Network::Utility::parseInternetAddressNoThrow("1:0023::0Ef"), address));
   EXPECT_EQ(nullptr, address.ip()->ipv4());
   EXPECT_TRUE(address.ip()->isUnicastAddress());
   EXPECT_EQ(nullptr, address.pipe());
@@ -275,7 +277,8 @@ TEST(Ipv6InstanceTest, AddressAndPort) {
   EXPECT_EQ("::1", address.ip()->addressAsString());
   EXPECT_EQ(80U, address.ip()->port());
   EXPECT_EQ(IpVersion::v6, address.ip()->version());
-  EXPECT_TRUE(addressesEqual(Network::Utility::parseInternetAddressNoThrow("0:0:0:0:0:0:0:1"), address));
+  EXPECT_TRUE(
+      addressesEqual(Network::Utility::parseInternetAddressNoThrow("0:0:0:0:0:0:0:1"), address));
   EXPECT_TRUE(address.ip()->isUnicastAddress());
 }
 
@@ -320,7 +323,8 @@ TEST(Ipv6InstanceTest, Multicast) {
   EXPECT_EQ(0U, address.ip()->port());
   EXPECT_EQ(IpVersion::v6, address.ip()->version());
   EXPECT_TRUE(addressesEqual(
-      Network::Utility::parseInternetAddressNoThrow("FF00:0000:0000:0000:0000:0000:0000:0000"), address));
+      Network::Utility::parseInternetAddressNoThrow("FF00:0000:0000:0000:0000:0000:0000:0000"),
+      address));
   EXPECT_FALSE(address.ip()->isUnicastAddress());
 }
 
@@ -332,7 +336,8 @@ TEST(Ipv6InstanceTest, Broadcast) {
   EXPECT_EQ(0U, address.ip()->port());
   EXPECT_EQ(IpVersion::v6, address.ip()->version());
   EXPECT_TRUE(addressesEqual(
-      Network::Utility::parseInternetAddressNoThrow("FFFF:FFFF:FFFF:FFFF:FFFF:FFFF:FFFF:FFFF"), address));
+      Network::Utility::parseInternetAddressNoThrow("FFFF:FFFF:FFFF:FFFF:FFFF:FFFF:FFFF:FFFF"),
+      address));
   EXPECT_FALSE(address.ip()->isUnicastAddress());
 }
 

diff --git a/test/common/network/lc_trie_test.cc b/test/common/network/lc_trie_test.cc
@@ -40,7 +40,8 @@ class LcTrieTest : public testing::Test {
     for (const auto& kv : test_output) {
       std::vector<std::string> expected(kv.second);
       std::sort(expected.begin(), expected.end());
-      std::vector<std::string> actual(trie_->getData(Utility::parseInternetAddressNoThrow(kv.first)));
+      std::vector<std::string> actual(
+          trie_->getData(Utility::parseInternetAddressNoThrow(kv.first)));
       std::sort(actual.begin(), actual.end());
       EXPECT_EQ(expected, actual);
     }

diff --git a/test/common/network/listen_socket_impl_test.cc b/test/common/network/listen_socket_impl_test.cc
@@ -207,7 +207,8 @@ TEST_P(ListenSocketImplTestTcp, SetLocalAddress) {
     address_str = "1::2";
   }
 
-  Address::InstanceConstSharedPtr address = Network::Utility::parseInternetAddressNoThrow(address_str);
+  Address::InstanceConstSharedPtr address =
+      Network::Utility::parseInternetAddressNoThrow(address_str);
 
   TestListenSocket socket(Utility::getIpv4AnyAddress());
 

diff --git a/test/common/network/utility_fuzz_test.cc b/test/common/network/utility_fuzz_test.cc
@@ -19,24 +19,20 @@ DEFINE_FUZZER(const uint8_t* buf, size_t len) {
 
   Network::Utility::parseInternetAddressAndPortNoThrow(string_buffer);
 
-  try {
+  {
     envoy::config::core::v3::Address proto_address;
     proto_address.mutable_pipe()->set_path(string_buffer);
-    Network::Utility::protobufAddressToAddress(proto_address);
-  } catch (const EnvoyException& e) {
-    ENVOY_LOG_MISC(debug, "EnvoyException: {}", e.what());
+    Network::Utility::protobufAddressToAddressNoThrow(proto_address);
   }
 
-  try {
+  {
     FuzzedDataProvider provider(buf, len);
     envoy::config::core::v3::Address proto_address;
     const auto port_value = provider.ConsumeIntegral<uint16_t>();
     const std::string address_value = provider.ConsumeRemainingBytesAsString();
     proto_address.mutable_socket_address()->set_address(address_value);
     proto_address.mutable_socket_address()->set_port_value(port_value);
-    Network::Utility::protobufAddressToAddress(proto_address);
-  } catch (const EnvoyException& e) {
-    ENVOY_LOG_MISC(debug, "EnvoyException: {}", e.what());
+    Network::Utility::protobufAddressToAddressNoThrow(proto_address);
   }
 
   try {

diff --git a/test/common/network/utility_test.cc b/test/common/network/utility_test.cc
@@ -141,19 +141,19 @@ TEST(NetworkUtility, socketTypeFromUrl) {
 }
 
 TEST(NetworkUtility, ParseInternetAddress) {
-  EXPECT_THROW(Utility::parseInternetAddressNoThrow(""), EnvoyException);
-  EXPECT_THROW(Utility::parseInternetAddressNoThrow("1.2.3"), EnvoyException);
-  EXPECT_THROW(Utility::parseInternetAddressNoThrow("1.2.3.4.5"), EnvoyException);
-  EXPECT_THROW(Utility::parseInternetAddressNoThrow("1.2.3.256"), EnvoyException);
-  EXPECT_THROW(Utility::parseInternetAddressNoThrow("foo"), EnvoyException);
-  EXPECT_THROW(Utility::parseInternetAddressNoThrow("0:0:0:0"), EnvoyException);
-  EXPECT_THROW(Utility::parseInternetAddressNoThrow("fffff::"), EnvoyException);
-  EXPECT_THROW(Utility::parseInternetAddressNoThrow("/foo"), EnvoyException);
+  EXPECT_EQ(Utility::parseInternetAddressNoThrow(""), nullptr);
+  EXPECT_EQ(Utility::parseInternetAddressNoThrow("1.2.3"), nullptr);
+  EXPECT_EQ(Utility::parseInternetAddressNoThrow("1.2.3.4.5"), nullptr);
+  EXPECT_EQ(Utility::parseInternetAddressNoThrow("1.2.3.256"), nullptr);
+  EXPECT_EQ(Utility::parseInternetAddressNoThrow("foo"), nullptr);
+  EXPECT_EQ(Utility::parseInternetAddressNoThrow("0:0:0:0"), nullptr);
+  EXPECT_EQ(Utility::parseInternetAddressNoThrow("fffff::"), nullptr);
+  EXPECT_EQ(Utility::parseInternetAddressNoThrow("/foo"), nullptr);
 
   // TODO(#24326): make windows getaddrinfo more strict. See below example.
 #ifndef WIN32
-  EXPECT_THROW(Utility::parseInternetAddressNoThrow("[::]"), EnvoyException);
-  EXPECT_THROW(Utility::parseInternetAddressNoThrow("[::1]:1"), EnvoyException);
+  EXPECT_EQ(Utility::parseInternetAddressNoThrow("[::]"), nullptr);
+  EXPECT_EQ(Utility::parseInternetAddressNoThrow("[::1]:1"), nullptr);
 #endif
 
   EXPECT_EQ(nullptr, Utility::parseInternetAddressNoThrow(""));
@@ -543,32 +543,34 @@ TEST(NetworkUtility, ParseProtobufAddress) {
     envoy::config::core::v3::Address proto_address;
     proto_address.mutable_socket_address()->set_address("127.0.0.1");
     proto_address.mutable_socket_address()->set_port_value(1234);
-    EXPECT_EQ("127.0.0.1:1234", Utility::protobufAddressToAddress(proto_address)->asString());
+    EXPECT_EQ("127.0.0.1:1234",
+              Utility::protobufAddressToAddressNoThrow(proto_address)->asString());
   }
   {
     envoy::config::core::v3::Address proto_address;
     proto_address.mutable_socket_address()->set_address("::1");
     proto_address.mutable_socket_address()->set_port_value(1234);
-    EXPECT_EQ("[::1]:1234", Utility::protobufAddressToAddress(proto_address)->asString());
+    EXPECT_EQ("[::1]:1234", Utility::protobufAddressToAddressNoThrow(proto_address)->asString());
   }
   {
     envoy::config::core::v3::Address proto_address;
     proto_address.mutable_pipe()->set_path("/tmp/unix-socket");
-    EXPECT_EQ("/tmp/unix-socket", Utility::protobufAddressToAddress(proto_address)->asString());
+    EXPECT_EQ("/tmp/unix-socket",
+              Utility::protobufAddressToAddressNoThrow(proto_address)->asString());
   }
   {
     envoy::config::core::v3::Address proto_address;
     proto_address.mutable_envoy_internal_address()->set_server_listener_name("internal_listener");
     proto_address.mutable_envoy_internal_address()->set_endpoint_id("12345");
     EXPECT_EQ("envoy://internal_listener/12345",
-              Utility::protobufAddressToAddress(proto_address)->asString());
+              Utility::protobufAddressToAddressNoThrow(proto_address)->asString());
   }
 #if defined(__linux__)
   {
     envoy::config::core::v3::Address proto_address;
     proto_address.mutable_pipe()->set_path("@/tmp/abstract-unix-socket");
     EXPECT_EQ("@/tmp/abstract-unix-socket",
-              Utility::protobufAddressToAddress(proto_address)->asString());
+              Utility::protobufAddressToAddressNoThrow(proto_address)->asString());
   }
 #endif
 }

diff --git a/test/extensions/filters/common/original_src/original_src_socket_option_test.cc b/test/extensions/filters/common/original_src/original_src_socket_option_test.cc
@@ -73,7 +73,8 @@ TEST_F(OriginalSrcSocketOptionTest, TestIpv4HashKeyOther) {
 }
 
 TEST_F(OriginalSrcSocketOptionTest, TestIpv6HashKey) {
-  const auto address = Network::Utility::parseInternetAddressNoThrow("102:304:506:708:90a:b0c:d0e:f00");
+  const auto address =
+      Network::Utility::parseInternetAddressNoThrow("102:304:506:708:90a:b0c:d0e:f00");
   auto option = makeOptionByAddress(address);
   option->hashKey(key_);
 
@@ -83,7 +84,8 @@ TEST_F(OriginalSrcSocketOptionTest, TestIpv6HashKey) {
 }
 
 TEST_F(OriginalSrcSocketOptionTest, TestIpv6HashKeyOther) {
-  const auto address = Network::Utility::parseInternetAddressNoThrow("F02:304:519:708:90a:b0e:FFFF:0000");
+  const auto address =
+      Network::Utility::parseInternetAddressNoThrow("F02:304:519:708:90a:b0e:FFFF:0000");
   auto option = makeOptionByAddress(address);
   option->hashKey(key_);
 

diff --git a/test/extensions/quic/server_preferred_address/fixed_server_preferred_address_test.cc b/test/extensions/quic/server_preferred_address/fixed_server_preferred_address_test.cc
@@ -23,7 +23,7 @@ TEST_F(FixedServerPreferredAddressConfigTest, Validation) {
     cfg.mutable_ipv4_config()->mutable_address()->set_address("not an address");
     cfg.mutable_ipv4_config()->mutable_address()->set_port_value(1);
     EXPECT_THROW_WITH_REGEX(factory_.createServerPreferredAddressConfig(cfg, visitor_, {}),
-                            EnvoyException, ".*malformed IP address: not an address.*");
+                            EnvoyException, ".*Invalid address socket_address.*");
   }
   {
     // Bad address.

diff --git a/test/extensions/tracers/zipkin/span_buffer_test.cc b/test/extensions/tracers/zipkin/span_buffer_test.cc
@@ -34,10 +34,10 @@ enum class IpType { V4, V6 };
 
 Endpoint createEndpoint(const IpType ip_type) {
   Endpoint endpoint;
-  endpoint.setAddress(ip_type == IpType::V6
-                          ? Envoy::Network::Utility::parseInternetAddressNoThrow(
-                                "2001:db8:85a3::8a2e:370:4444", 7334, true)
-                          : Envoy::Network::Utility::parseInternetAddressNoThrow("1.2.3.4", 8080, false));
+  endpoint.setAddress(ip_type == IpType::V6 ? Envoy::Network::Utility::parseInternetAddressNoThrow(
+                                                  "2001:db8:85a3::8a2e:370:4444", 7334, true)
+                                            : Envoy::Network::Utility::parseInternetAddressNoThrow(
+                                                  "1.2.3.4", 8080, false));
   endpoint.setServiceName("service1");
   return endpoint;
 }

diff --git a/test/integration/base_integration_test.cc b/test/integration/base_integration_test.cc
@@ -91,8 +91,8 @@ BaseIntegrationTest::BaseIntegrationTest(const InstanceConstSharedPtrFn& upstrea
 const BaseIntegrationTest::InstanceConstSharedPtrFn
 BaseIntegrationTest::defaultAddressFunction(Network::Address::IpVersion version) {
   return [version](int) {
-    return Network::Utility::parseInternetAddressNoThrow(Network::Test::getLoopbackAddressString(version),
-                                                  0);
+    return Network::Utility::parseInternetAddressNoThrow(
+        Network::Test::getLoopbackAddressString(version), 0);
   };
 }
 

diff --git a/test/integration/fake_upstream.cc b/test/integration/fake_upstream.cc
@@ -616,8 +616,8 @@ makeTcpListenSocket(const Network::Address::InstanceConstSharedPtr& address) {
 
 static Network::Address::InstanceConstSharedPtr makeAddress(uint32_t port,
                                                             Network::Address::IpVersion version) {
-  return Network::Utility::parseInternetAddressNoThrow(Network::Test::getLoopbackAddressString(version),
-                                                port);
+  return Network::Utility::parseInternetAddressNoThrow(
+      Network::Test::getLoopbackAddressString(version), port);
 }
 
 static Network::SocketPtr

diff --git a/test/test_common/utility.cc b/test/test_common/utility.cc
@@ -328,7 +328,8 @@ std::list<Network::DnsResponse>
 TestUtility::makeDnsResponse(const std::list<std::string>& addresses, std::chrono::seconds ttl) {
   std::list<Network::DnsResponse> ret;
   for (const auto& address : addresses) {
-    ret.emplace_back(Network::DnsResponse(Network::Utility::parseInternetAddressNoThrow(address), ttl));
+    ret.emplace_back(
+        Network::DnsResponse(Network::Utility::parseInternetAddressNoThrow(address), ttl));
   }
   return ret;
 }

diff --git a/tools/code_format/config.yaml b/tools/code_format/config.yaml
@@ -105,7 +105,6 @@ paths:
     - source/common/network/listen_socket_impl.cc
     - source/common/network/io_socket_handle_base_impl.cc
     - source/common/network/address_impl.cc
-    - source/common/network/utility.cc
     - source/common/formatter/http_specific_formatter.cc
     - source/common/formatter/stream_info_formatter.h
     - source/common/formatter/stream_info_formatter.cc