network: centralize IP parsing and unify on getaddrinfo() to avoid circular deps

source/common/network/BUILD

@@ -500,6 +500,7 @@ envoy_cc_library(
     deps = [
         ":address_lib",
         ":default_socket_interface_lib",
+        ":ip_address_parsing_lib",
         ":socket_lib",
         ":socket_option_lib",
         "//envoy/api:os_sys_calls_interface",
@@ -517,6 +518,16 @@ envoy_cc_library(
     ],
 )
 
+envoy_cc_library(
+    name = "ip_address_parsing_lib",
+    srcs = ["ip_address_parsing.cc"],
+    hdrs = ["ip_address_parsing.h"],
+    deps = [
+        "//source/common/api:os_sys_calls_lib",
+        "//source/common/common:statusor_lib",
+    ],
+)
+
 envoy_cc_library(
     name = "transport_socket_options_lib",
     srcs = ["transport_socket_options_impl.cc"],

source/common/network/ip_address_parsing.cc

@@ -0,0 +1,67 @@
+#include "source/common/network/ip_address_parsing.h"
+
+#include <cstring>
+
+#include "source/common/api/os_sys_calls_impl.h"
+
+namespace Envoy {
+namespace Network {
+namespace IpAddressParsing {
+
+StatusOr<sockaddr_in> parseIPv4(const std::string& ip_address, uint16_t port) {
+  // Use inet_pton() for IPv4 as it's simpler, faster, and already enforces
+  // strict dotted-quad format while rejecting non-standard notations.
+  sockaddr_in sa4;
+  memset(&sa4, 0, sizeof(sa4));
+  if (inet_pton(AF_INET, ip_address.c_str(), &sa4.sin_addr) != 1) {
+    return absl::FailedPreconditionError("failed parsing ipv4");
+  }
+  sa4.sin_family = AF_INET;
+  sa4.sin_port = htons(port);
+  return sa4;
+}
+
+StatusOr<sockaddr_in6> parseIPv6(const std::string& ip_address, uint16_t port) {
+  // Parse IPv6 with optional scope using getaddrinfo().
+  // While inet_pton() would be faster and simpler, it does not support IPv6
+  // addresses that specify a scope, e.g. `::%eth0` to listen on only one interface.
+  struct addrinfo hints;
+  memset(&hints, 0, sizeof(hints));
+  struct addrinfo* res = nullptr;
+  // Suppresses any potentially lengthy network host address lookups and inhibit the
+  // invocation of a name resolution service.
+  hints.ai_flags = AI_NUMERICHOST | AI_NUMERICSERV;
+  hints.ai_family = AF_INET6;
+  // Given that we do not specify a service but we use getaddrinfo() to only parse the node
+  // address, specifying the socket type allows to hint the getaddrinfo() to return only an
+  // element with the below socket type. The behavior though remains platform dependent and
+  // anyway we consume only the first element if the call succeeds.
+  hints.ai_socktype = SOCK_DGRAM;
+  hints.ai_protocol = IPPROTO_UDP;
+
+  // We want to use the interface of OsSysCalls for this for the platform-independence, but
+  // we do not want to use the common OsSysCallsSingleton.
+  //
+  // The problem with using OsSysCallsSingleton is that we likely want to override getaddrinfo()
+  // for DNS lookups in tests, but typically that override would resolve a name to e.g. the
+  // address from resolveUrl("tcp://[::1]:80"). But resolveUrl calls ``parseIPv6``, which calls
+  // getaddrinfo(), so if we use the mock here then mocking DNS causes infinite recursion.
+  //
+  // We do not ever need to mock this getaddrinfo() call, because it is only used to parse
+  // numeric IP addresses, per ``ai_flags``, so it should be deterministic resolution. There
+  // is no need to mock it to test failure cases.
+  static Api::OsSysCallsImpl os_sys_calls;
+  const Api::SysCallIntResult rc =
+      os_sys_calls.getaddrinfo(ip_address.c_str(), /*service=*/nullptr, &hints, &res);
+  if (rc.return_value_ != 0) {
+    return absl::FailedPreconditionError(absl::StrCat("getaddrinfo error: ", rc.return_value_));
+  }
+  sockaddr_in6 sa6 = *reinterpret_cast<sockaddr_in6*>(res->ai_addr);
+  os_sys_calls.freeaddrinfo(res);
+  sa6.sin6_port = htons(port);
+  return sa6;
+}
+
+} // namespace IpAddressParsing
+} // namespace Network
+} // namespace Envoy

source/common/network/ip_address_parsing.h

@@ -0,0 +1,34 @@
+#pragma once
+
+#include <cstdint>
+#include <string>
+
+#include "envoy/common/platform.h"
+
+#include "source/common/common/statusor.h"
+
+namespace Envoy {
+namespace Network {
+
+// Utilities for parsing numeric IP addresses into sockaddr structures.
+// These helper methods avoid higher-level dependencies and are suitable for
+// use by multiple components that need low-level parsing without constructing
+// `Address::Instance` objects.
+namespace IpAddressParsing {
+
+// Parse an IPv4 address string into a sockaddr_in with the provided port.
+// Returns a failure status if the address is not a valid numeric IPv4 string.
+StatusOr<sockaddr_in> parseIPv4(const std::string& ip_address, uint16_t port);
+
+// Parse an IPv6 address string (optionally with a scope id, e.g. ``fe80::1%2``
+// or ``fe80::1%eth0``) into a sockaddr_in6 with the provided port.
+//
+// Uses getaddrinfo() with ``AI_NUMERICHOST|AI_NUMERICSERV`` to avoid DNS lookups
+// and to support scoped addresses consistently across all platforms.
+//
+// Returns a failure status if the address is not a valid numeric IPv6 string.
+StatusOr<sockaddr_in6> parseIPv6(const std::string& ip_address, uint16_t port);
+
+} // namespace IpAddressParsing
+} // namespace Network
+} // namespace Envoy

source/common/network/utility.cc

@@ -22,6 +22,7 @@
 #include "source/common/common/utility.h"
 #include "source/common/network/address_impl.h"
 #include "source/common/network/io_socket_error_impl.h"
+#include "source/common/network/ip_address_parsing.h"
 #include "source/common/network/socket_option_impl.h"
 #include "source/common/protobuf/protobuf.h"
 #include "source/common/protobuf/utility.h"
@@ -106,75 +107,18 @@ Api::IoCallUint64Result receiveMessage(uint64_t max_rx_datagram_size, Buffer::In
   return result;
 }
 
-StatusOr<sockaddr_in> parseV4Address(const std::string& ip_address, uint16_t port) {
-  sockaddr_in sa4;
-  memset(&sa4, 0, sizeof(sa4));
-  if (inet_pton(AF_INET, ip_address.c_str(), &sa4.sin_addr) != 1) {
-    return absl::FailedPreconditionError("failed parsing ipv4");
-  }
-  sa4.sin_family = AF_INET;
-  sa4.sin_port = htons(port);
-  return sa4;
-}
-
-StatusOr<sockaddr_in6> parseV6Address(const std::string& ip_address, uint16_t port) {
-  // Parse IPv6 with optional scope using getaddrinfo().
-  // While inet_pton() would be faster and simpler, it does not support IPv6
-  // addresses that specify a scope, e.g. `::%eth0` to listen on only one interface.
-  struct addrinfo hints;
-  memset(&hints, 0, sizeof(hints));
-  struct addrinfo* res = nullptr;
-  // Suppresses any potentially lengthy network host address lookups and inhibit the invocation of
-  // a name resolution service.
-  hints.ai_flags = AI_NUMERICHOST | AI_NUMERICSERV;
-  hints.ai_family = AF_INET6;
-  // Given that we don't specify a service but we use getaddrinfo() to only parse the node
-  // address, specifying the socket type allows to hint the getaddrinfo() to return only an
-  // element with the below socket type. The behavior though remains platform dependent and anyway
-  // we consume only the first element (if the call succeeds).
-  hints.ai_socktype = SOCK_DGRAM;
-  hints.ai_protocol = IPPROTO_UDP;
-
-  // We want to use the interface of OsSysCalls for this for the
-  // platform-independence, but we don't want to use the common
-  // OsSysCallsSingleton.
-  //
-  // The problem with using OsSysCallsSingleton is that we likely
-  // want to override getaddrinfo() for DNS lookups in tests, but
-  // typically that override would resolve a name to e.g.
-  // the address from resolveUrl("tcp://[::1]:80") - but resolveUrl
-  // calls parseV6Address, which calls getaddrinfo(), so if we use
-  // the mock *here* then mocking DNS causes infinite recursion.
-  //
-  // We don't ever need to mock *this* getaddrinfo() call, because
-  // it's only used to parse numeric IP addresses, per `ai_flags`,
-  // so it should be deterministic resolution; there's no need to
-  // mock it to test failure cases.
-  static Api::OsSysCallsImpl os_sys_calls;
-
-  const Api::SysCallIntResult rc =
-      os_sys_calls.getaddrinfo(ip_address.c_str(), /*service=*/nullptr, &hints, &res);
-  if (rc.return_value_ != 0) {
-    return absl::FailedPreconditionError(fmt::format("getaddrinfo error: {}", rc.return_value_));
-  }
-  sockaddr_in6 sa6 = *reinterpret_cast<sockaddr_in6*>(res->ai_addr);
-  os_sys_calls.freeaddrinfo(res);
-  sa6.sin6_port = htons(port);
-  return sa6;
-}
-
 } // namespace
 
 Address::InstanceConstSharedPtr
 Utility::parseInternetAddressNoThrow(const std::string& ip_address, uint16_t port, bool v6only,
                                      absl::optional<std::string> network_namespace) {
-  StatusOr<sockaddr_in> sa4 = parseV4Address(ip_address, port);
+  StatusOr<sockaddr_in> sa4 = IpAddressParsing::parseIPv4(ip_address, port);
   if (sa4.ok()) {
     return instanceOrNull(Address::InstanceFactory::createInstancePtr<Address::Ipv4Instance>(
         &sa4.value(), nullptr, network_namespace));
   }
 
-  StatusOr<sockaddr_in6> sa6 = parseV6Address(ip_address, port);
+  StatusOr<sockaddr_in6> sa6 = IpAddressParsing::parseIPv6(ip_address, port);
   if (sa6.ok()) {
     return instanceOrNull(Address::InstanceFactory::createInstancePtr<Address::Ipv6Instance>(
         *sa6, v6only, nullptr, network_namespace));
@@ -200,7 +144,7 @@ Utility::parseInternetAddressAndPortNoThrow(const std::string& ip_address, bool
     if (port_str.empty() || !absl::SimpleAtoi(port_str, &port64) || port64 > 65535) {
       return nullptr;
     }
-    StatusOr<sockaddr_in6> sa6 = parseV6Address(ip_str, port64);
+    StatusOr<sockaddr_in6> sa6 = IpAddressParsing::parseIPv6(ip_str, static_cast<uint16_t>(port64));
     if (sa6.ok()) {
       return instanceOrNull(Address::InstanceFactory::createInstancePtr<Address::Ipv6Instance>(
           *sa6, v6only, nullptr, network_namespace));
@@ -218,7 +162,7 @@ Utility::parseInternetAddressAndPortNoThrow(const std::string& ip_address, bool
   if (port_str.empty() || !absl::SimpleAtoi(port_str, &port64) || port64 > 65535) {
     return nullptr;
   }
-  StatusOr<sockaddr_in> sa4 = parseV4Address(ip_str, port64);
+  StatusOr<sockaddr_in> sa4 = IpAddressParsing::parseIPv4(ip_str, static_cast<uint16_t>(port64));
   if (sa4.ok()) {
     return instanceOrNull(Address::InstanceFactory::createInstancePtr<Address::Ipv4Instance>(
         &sa4.value(), nullptr, network_namespace));

test/common/network/BUILD

@@ -405,6 +405,15 @@ envoy_cc_test(
     ],
 )
 
+envoy_cc_test(
+    name = "ip_address_parsing_test",
+    srcs = ["ip_address_parsing_test.cc"],
+    rbe_pool = "6gig",
+    deps = [
+        "//source/common/network:ip_address_parsing_lib",
+    ],
+)
+
 envoy_cc_fuzz_test(
     name = "udp_fuzz_test",
     srcs = ["udp_fuzz.cc"],

test/common/network/ip_address_parsing_test.cc

@@ -0,0 +1,138 @@
+#include <arpa/inet.h>
+
+#include "source/common/network/ip_address_parsing.h"
+
+#include "gtest/gtest.h"
+
+namespace Envoy {
+namespace Network {
+
+TEST(IpAddressParsingTest, ParseIPv4Valid) {
+  // Test standard dotted-quad notation.
+  auto sa4_or = IpAddressParsing::parseIPv4("127.0.0.1", /*port=*/8080);
+  ASSERT_TRUE(sa4_or.ok());
+  const sockaddr_in sa4 = sa4_or.value();
+  EXPECT_EQ(AF_INET, sa4.sin_family);
+  EXPECT_EQ(htons(8080), sa4.sin_port);
+  EXPECT_EQ(htonl(INADDR_LOOPBACK), sa4.sin_addr.s_addr);
+
+  // Test another valid IPv4.
+  auto sa4_or2 = IpAddressParsing::parseIPv4("192.168.1.1", /*port=*/443);
+  ASSERT_TRUE(sa4_or2.ok());
+  const sockaddr_in sa4_2 = sa4_or2.value();
+  EXPECT_EQ(AF_INET, sa4_2.sin_family);
+  EXPECT_EQ(htons(443), sa4_2.sin_port);
+  char buf[INET_ADDRSTRLEN];
+  inet_ntop(AF_INET, &sa4_2.sin_addr, buf, INET_ADDRSTRLEN);
+  EXPECT_EQ("192.168.1.1", std::string(buf));
+
+  // Test edge case IPs.
+  EXPECT_TRUE(IpAddressParsing::parseIPv4("0.0.0.0", 0).ok());
+  EXPECT_TRUE(IpAddressParsing::parseIPv4("255.255.255.255", 65535).ok());
+}
+
+TEST(IpAddressParsingTest, ParseIPv4Invalid) {
+  // Test incomplete addresses.
+  EXPECT_FALSE(IpAddressParsing::parseIPv4("1.2.3", 0).ok());
+  EXPECT_FALSE(IpAddressParsing::parseIPv4("1.2", 0).ok());
+  EXPECT_FALSE(IpAddressParsing::parseIPv4("1", 0).ok());
+
+  // Test out-of-range octets.
+  EXPECT_FALSE(IpAddressParsing::parseIPv4("1.2.3.256", 0).ok());
+  EXPECT_FALSE(IpAddressParsing::parseIPv4("256.0.0.1", 0).ok());
+  EXPECT_FALSE(IpAddressParsing::parseIPv4("1.2.3.999", 0).ok());
+
+  // Test non-numeric input.
+  EXPECT_FALSE(IpAddressParsing::parseIPv4("not_an_ip", 0).ok());
+  EXPECT_FALSE(IpAddressParsing::parseIPv4("abc.def.ghi.jkl", 0).ok());
+  EXPECT_FALSE(IpAddressParsing::parseIPv4("", 0).ok());
+
+  // Test IPv6 addresses (should fail for IPv4 parser).
+  EXPECT_FALSE(IpAddressParsing::parseIPv4("::1", 0).ok());
+  EXPECT_FALSE(IpAddressParsing::parseIPv4("fe80::1", 0).ok());
+
+  // Test that inet_pton() correctly rejects non-standard formats.
+  // These formats might be accepted by some liberal parsers but should be rejected
+  // by inet_pton() which enforces strict dotted-quad notation.
+  EXPECT_FALSE(IpAddressParsing::parseIPv4("127.1", 0).ok());      // Short form
+  EXPECT_FALSE(IpAddressParsing::parseIPv4("0x7f.0.0.1", 0).ok()); // Hex notation
+  // Note: Some platforms (e.g., macOS) may accept octal notation in inet_pton(),
+  // so we skip this test to maintain platform compatibility.
+
+  // Test extra dots.
+  EXPECT_FALSE(IpAddressParsing::parseIPv4("1.2.3.4.", 0).ok());
+  EXPECT_FALSE(IpAddressParsing::parseIPv4(".1.2.3.4", 0).ok());
+  EXPECT_FALSE(IpAddressParsing::parseIPv4("1..2.3.4", 0).ok());
+
+  // Test with spaces.
+  EXPECT_FALSE(IpAddressParsing::parseIPv4(" 1.2.3.4", 0).ok());
+  EXPECT_FALSE(IpAddressParsing::parseIPv4("1.2.3.4 ", 0).ok());
+  EXPECT_FALSE(IpAddressParsing::parseIPv4("1.2. 3.4", 0).ok());
+}
+
+TEST(IpAddressParsingTest, ParseIPv6Valid) {
+  // Test loopback.
+  auto sa6_or = IpAddressParsing::parseIPv6("::1", /*port=*/443);
+  ASSERT_TRUE(sa6_or.ok());
+  const sockaddr_in6 sa6 = sa6_or.value();
+  EXPECT_EQ(AF_INET6, sa6.sin6_family);
+  EXPECT_EQ(htons(443), sa6.sin6_port);
+  in6_addr loopback = IN6ADDR_LOOPBACK_INIT;
+  EXPECT_EQ(0, memcmp(&loopback, &sa6.sin6_addr, sizeof(in6_addr)));
+
+  // Test other valid IPv6 addresses.
+  EXPECT_TRUE(IpAddressParsing::parseIPv6("::", 0).ok());
+  EXPECT_TRUE(IpAddressParsing::parseIPv6("::ffff:127.0.0.1", 0).ok()); // IPv4-mapped
+  EXPECT_TRUE(IpAddressParsing::parseIPv6("2001:db8::1", 0).ok());
+  EXPECT_TRUE(IpAddressParsing::parseIPv6("fe80::1", 0).ok());
+  EXPECT_TRUE(IpAddressParsing::parseIPv6("2001:0db8:0000:0000:0000:0000:0000:0001", 0).ok());
+
+  // Test IPv6 with scope (this is why we need getaddrinfo() for IPv6).
+  // Note: Actual scope parsing depends on platform support.
+#ifdef __linux__
+  // Numeric scope ID.
+  auto sa6_scope = IpAddressParsing::parseIPv6("fe80::1%2", 80);
+  ASSERT_TRUE(sa6_scope.ok());
+  EXPECT_EQ(2, sa6_scope.value().sin6_scope_id);
+#endif
+}
+
+TEST(IpAddressParsingTest, ParseIPv6Invalid) {
+  // Test invalid characters.
+  EXPECT_FALSE(IpAddressParsing::parseIPv6("::g", 0).ok());
+  EXPECT_FALSE(IpAddressParsing::parseIPv6("gggg::1", 0).ok());
+
+  // Test invalid format.
+  EXPECT_FALSE(IpAddressParsing::parseIPv6("1:::1", 0).ok());
+  EXPECT_FALSE(IpAddressParsing::parseIPv6(":::1", 0).ok());
+  EXPECT_FALSE(IpAddressParsing::parseIPv6("1::2::3", 0).ok()); // Multiple ::
+
+  // Test non-IP strings.
+  EXPECT_FALSE(IpAddressParsing::parseIPv6("not_an_ip", 0).ok());
+  EXPECT_FALSE(IpAddressParsing::parseIPv6("", 0).ok());
+
+  // Test IPv4 addresses (should fail for IPv6 parser).
+  EXPECT_FALSE(IpAddressParsing::parseIPv6("127.0.0.1", 0).ok());
+  EXPECT_FALSE(IpAddressParsing::parseIPv6("192.168.1.1", 0).ok());
+
+  // Test too many groups.
+  EXPECT_FALSE(IpAddressParsing::parseIPv6("1:2:3:4:5:6:7:8:9", 0).ok());
+
+  // Test with spaces.
+  EXPECT_FALSE(IpAddressParsing::parseIPv6(" ::1", 0).ok());
+  EXPECT_FALSE(IpAddressParsing::parseIPv6("::1 ", 0).ok());
+  EXPECT_FALSE(IpAddressParsing::parseIPv6(":: 1", 0).ok());
+}
+
+TEST(IpAddressParsingTest, PortBoundaries) {
+  // Test port boundaries for IPv4.
+  EXPECT_TRUE(IpAddressParsing::parseIPv4("127.0.0.1", 0).ok());
+  EXPECT_TRUE(IpAddressParsing::parseIPv4("127.0.0.1", 65535).ok());
+
+  // Test port boundaries for IPv6.
+  EXPECT_TRUE(IpAddressParsing::parseIPv6("::1", 0).ok());
+  EXPECT_TRUE(IpAddressParsing::parseIPv6("::1", 65535).ok());
+}
+
+} // namespace Network
+} // namespace Envoy