Skip to content

Commit 637ad27

Browse files
agrawrohagrawroh
committed
network: centralize IP parsing and unify on getaddrinfo() to avoid circular deps
Signed-off-by: Rohit Agrawal <[email protected]>
1 parent 45c3745 commit 637ad27

File tree

6 files changed

+180
-61
lines changed

6 files changed

+180
-61
lines changed

‎source/common/network/BUILD

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -500,6 +500,7 @@ envoy_cc_library(
500500
deps = [
501501
":address_lib",
502502
":default_socket_interface_lib",
503+
":ip_address_parsing_lib",
503504
":socket_lib",
504505
":socket_option_lib",
505506
"//envoy/api:os_sys_calls_interface",
@@ -517,6 +518,16 @@ envoy_cc_library(
517518
],
518519
)
519520

521+
envoy_cc_library(
522+
name = "ip_address_parsing_lib",
523+
srcs = ["ip_address_parsing.cc"],
524+
hdrs = ["ip_address_parsing.h"],
525+
deps = [
526+
"//source/common/api:os_sys_calls_lib",
527+
"//source/common/common:statusor_lib",
528+
],
529+
)
530+
520531
envoy_cc_library(
521532
name = "transport_socket_options_lib",
522533
srcs = ["transport_socket_options_impl.cc"],

‎source/common/network/ip_address_parsing.cc

Lines changed: 83 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,83 @@
1+
#include "source/common/network/ip_address_parsing.h"
2+
3+
#include <cstring>
4+
5+
#include "source/common/api/os_sys_calls_impl.h"
6+
7+
namespace Envoy {
8+
namespace Network {
9+
namespace IpAddressParsing {
10+
11+
StatusOr<sockaddr_in> parseIPv4(const std::string& ip_address, uint16_t port) {
12+
// Prefer getaddrinfo() with AI_NUMERICHOST|AI_NUMERICSERV for consistency with IPv6 parsing
13+
// and to keep a single parsing method. This also ensures no DNS lookups occur.
14+
struct addrinfo hints;
15+
memset(&hints, 0, sizeof(hints));
16+
struct addrinfo* res = nullptr;
17+
hints.ai_flags = AI_NUMERICHOST | AI_NUMERICSERV;
18+
hints.ai_family = AF_INET;
19+
hints.ai_socktype = SOCK_DGRAM;
20+
hints.ai_protocol = IPPROTO_UDP;
21+
22+
static Api::OsSysCallsImpl os_sys_calls;
23+
const Api::SysCallIntResult rc =
24+
os_sys_calls.getaddrinfo(ip_address.c_str(), /*service=*/nullptr, &hints, &res);
25+
if (rc.return_value_ != 0) {
26+
return absl::FailedPreconditionError(absl::StrCat("getaddrinfo error: ", rc.return_value_));
27+
}
28+
sockaddr_in sa4 = *reinterpret_cast<sockaddr_in*>(res->ai_addr);
29+
os_sys_calls.freeaddrinfo(res);
30+
sa4.sin_port = htons(port);
31+
// Enforce strict dotted-quad by round-tripping via inet_ntop and requiring equality.
32+
char buf[INET_ADDRSTRLEN];
33+
const char* printed = inet_ntop(AF_INET, &sa4.sin_addr, buf, INET_ADDRSTRLEN);
34+
if (printed == nullptr || ip_address != printed) {
35+
return absl::FailedPreconditionError("failed parsing ipv4");
36+
}
37+
return sa4;
38+
}
39+
40+
StatusOr<sockaddr_in6> parseIPv6(const std::string& ip_address, uint16_t port) {
41+
// Parse IPv6 with optional scope using getaddrinfo().
42+
// While inet_pton() would be faster and simpler, it does not support IPv6
43+
// addresses that specify a scope, e.g. `::%eth0` to listen on only one interface.
44+
struct addrinfo hints;
45+
memset(&hints, 0, sizeof(hints));
46+
struct addrinfo* res = nullptr;
47+
// Suppresses any potentially lengthy network host address lookups and inhibit the
48+
// invocation of a name resolution service.
49+
hints.ai_flags = AI_NUMERICHOST | AI_NUMERICSERV;
50+
hints.ai_family = AF_INET6;
51+
// Given that we do not specify a service but we use getaddrinfo() to only parse the node
52+
// address, specifying the socket type allows to hint the getaddrinfo() to return only an
53+
// element with the below socket type. The behavior though remains platform dependent and
54+
// anyway we consume only the first element if the call succeeds.
55+
hints.ai_socktype = SOCK_DGRAM;
56+
hints.ai_protocol = IPPROTO_UDP;
57+
58+
// We want to use the interface of OsSysCalls for this for the platform-independence, but
59+
// we do not want to use the common OsSysCallsSingleton.
60+
//
61+
// The problem with using OsSysCallsSingleton is that we likely want to override getaddrinfo()
62+
// for DNS lookups in tests, but typically that override would resolve a name to e.g. the
63+
// address from resolveUrl("tcp://[::1]:80") - but resolveUrl calls parseIPv6, which calls
64+
// getaddrinfo(), so if we use the mock here then mocking DNS causes infinite recursion.
65+
//
66+
// We do not ever need to mock this getaddrinfo() call, because it is only used to parse
67+
// numeric IP addresses, per ai_flags, so it should be deterministic resolution; there is no
68+
// need to mock it to test failure cases.
69+
static Api::OsSysCallsImpl os_sys_calls;
70+
const Api::SysCallIntResult rc =
71+
os_sys_calls.getaddrinfo(ip_address.c_str(), /*service=*/nullptr, &hints, &res);
72+
if (rc.return_value_ != 0) {
73+
return absl::FailedPreconditionError(absl::StrCat("getaddrinfo error: ", rc.return_value_));
74+
}
75+
sockaddr_in6 sa6 = *reinterpret_cast<sockaddr_in6*>(res->ai_addr);
76+
os_sys_calls.freeaddrinfo(res);
77+
sa6.sin6_port = htons(port);
78+
return sa6;
79+
}
80+
81+
} // namespace IpAddressParsing
82+
} // namespace Network
83+
} // namespace Envoy

‎source/common/network/ip_address_parsing.h

Lines changed: 34 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,34 @@
1+
#pragma once
2+
3+
#include <cstdint>
4+
#include <string>
5+
6+
#include "envoy/common/platform.h"
7+
8+
#include "source/common/common/statusor.h"
9+
10+
namespace Envoy {
11+
namespace Network {
12+
13+
// Utilities for parsing numeric IP addresses into sockaddr structures.
14+
// These helpers methods avoid higher-level dependencies and are suitable for
15+
// use by multiple components that need low-level parsing without constructing
16+
// `Address::Instance` objects.
17+
namespace IpAddressParsing {
18+
19+
// Parse an IPv4 address string into a sockaddr_in with the provided port.
20+
// Returns a failure status if the address is not a valid numeric IPv4 string.
21+
StatusOr<sockaddr_in> parseIPv4(const std::string& ip_address, uint16_t port);
22+
23+
// Parse an IPv6 address string (optionally with a scope id, e.g. "fe80::1%2"
24+
// or "fe80::1%eth0") into a sockaddr_in6 with the provided port.
25+
//
26+
// Uses getaddrinfo() with AI_NUMERICHOST|AI_NUMERICSERV to avoid DNS lookups
27+
// and to support scoped addresses consistently across all platforms.
28+
//
29+
// Returns a failure status if the address is not a valid numeric IPv6 string.
30+
StatusOr<sockaddr_in6> parseIPv6(const std::string& ip_address, uint16_t port);
31+
32+
} // namespace IpAddressParsing
33+
} // namespace Network
34+
} // namespace Envoy

‎source/common/network/utility.cc

Lines changed: 5 additions & 61 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,7 @@
2222
#include "source/common/common/utility.h"
2323
#include "source/common/network/address_impl.h"
2424
#include "source/common/network/io_socket_error_impl.h"
25+
#include "source/common/network/ip_address_parsing.h"
2526
#include "source/common/network/socket_option_impl.h"
2627
#include "source/common/protobuf/protobuf.h"
2728
#include "source/common/protobuf/utility.h"
@@ -106,75 +107,18 @@ Api::IoCallUint64Result receiveMessage(uint64_t max_rx_datagram_size, Buffer::In
106107
return result;
107108
}
108109

109-
StatusOr<sockaddr_in> parseV4Address(const std::string& ip_address, uint16_t port) {
110-
sockaddr_in sa4;
111-
memset(&sa4, 0, sizeof(sa4));
112-
if (inet_pton(AF_INET, ip_address.c_str(), &sa4.sin_addr) != 1) {
113-
return absl::FailedPreconditionError("failed parsing ipv4");
114-
}
115-
sa4.sin_family = AF_INET;
116-
sa4.sin_port = htons(port);
117-
return sa4;
118-
}
119-
120-
StatusOr<sockaddr_in6> parseV6Address(const std::string& ip_address, uint16_t port) {
121-
// Parse IPv6 with optional scope using getaddrinfo().
122-
// While inet_pton() would be faster and simpler, it does not support IPv6
123-
// addresses that specify a scope, e.g. `::%eth0` to listen on only one interface.
124-
struct addrinfo hints;
125-
memset(&hints, 0, sizeof(hints));
126-
struct addrinfo* res = nullptr;
127-
// Suppresses any potentially lengthy network host address lookups and inhibit the invocation of
128-
// a name resolution service.
129-
hints.ai_flags = AI_NUMERICHOST | AI_NUMERICSERV;
130-
hints.ai_family = AF_INET6;
131-
// Given that we don't specify a service but we use getaddrinfo() to only parse the node
132-
// address, specifying the socket type allows to hint the getaddrinfo() to return only an
133-
// element with the below socket type. The behavior though remains platform dependent and anyway
134-
// we consume only the first element (if the call succeeds).
135-
hints.ai_socktype = SOCK_DGRAM;
136-
hints.ai_protocol = IPPROTO_UDP;
137-
138-
// We want to use the interface of OsSysCalls for this for the
139-
// platform-independence, but we don't want to use the common
140-
// OsSysCallsSingleton.
141-
//
142-
// The problem with using OsSysCallsSingleton is that we likely
143-
// want to override getaddrinfo() for DNS lookups in tests, but
144-
// typically that override would resolve a name to e.g.
145-
// the address from resolveUrl("tcp://[::1]:80") - but resolveUrl
146-
// calls parseV6Address, which calls getaddrinfo(), so if we use
147-
// the mock *here* then mocking DNS causes infinite recursion.
148-
//
149-
// We don't ever need to mock *this* getaddrinfo() call, because
150-
// it's only used to parse numeric IP addresses, per `ai_flags`,
151-
// so it should be deterministic resolution; there's no need to
152-
// mock it to test failure cases.
153-
static Api::OsSysCallsImpl os_sys_calls;
154-
155-
const Api::SysCallIntResult rc =
156-
os_sys_calls.getaddrinfo(ip_address.c_str(), /*service=*/nullptr, &hints, &res);
157-
if (rc.return_value_ != 0) {
158-
return absl::FailedPreconditionError(fmt::format("getaddrinfo error: {}", rc.return_value_));
159-
}
160-
sockaddr_in6 sa6 = *reinterpret_cast<sockaddr_in6*>(res->ai_addr);
161-
os_sys_calls.freeaddrinfo(res);
162-
sa6.sin6_port = htons(port);
163-
return sa6;
164-
}
165-
166110
} // namespace
167111

168112
Address::InstanceConstSharedPtr
169113
Utility::parseInternetAddressNoThrow(const std::string& ip_address, uint16_t port, bool v6only,
170114
absl::optional<std::string> network_namespace) {
171-
StatusOr<sockaddr_in> sa4 = parseV4Address(ip_address, port);
115+
StatusOr<sockaddr_in> sa4 = IpAddressParsing::parseIPv4(ip_address, port);
172116
if (sa4.ok()) {
173117
return instanceOrNull(Address::InstanceFactory::createInstancePtr<Address::Ipv4Instance>(
174118
&sa4.value(), nullptr, network_namespace));
175119
}
176120

177-
StatusOr<sockaddr_in6> sa6 = parseV6Address(ip_address, port);
121+
StatusOr<sockaddr_in6> sa6 = IpAddressParsing::parseIPv6(ip_address, port);
178122
if (sa6.ok()) {
179123
return instanceOrNull(Address::InstanceFactory::createInstancePtr<Address::Ipv6Instance>(
180124
*sa6, v6only, nullptr, network_namespace));
@@ -200,7 +144,7 @@ Utility::parseInternetAddressAndPortNoThrow(const std::string& ip_address, bool
200144
if (port_str.empty() || !absl::SimpleAtoi(port_str, &port64) || port64 > 65535) {
201145
return nullptr;
202146
}
203-
StatusOr<sockaddr_in6> sa6 = parseV6Address(ip_str, port64);
147+
StatusOr<sockaddr_in6> sa6 = IpAddressParsing::parseIPv6(ip_str, static_cast<uint16_t>(port64));
204148
if (sa6.ok()) {
205149
return instanceOrNull(Address::InstanceFactory::createInstancePtr<Address::Ipv6Instance>(
206150
*sa6, v6only, nullptr, network_namespace));
@@ -218,7 +162,7 @@ Utility::parseInternetAddressAndPortNoThrow(const std::string& ip_address, bool
218162
if (port_str.empty() || !absl::SimpleAtoi(port_str, &port64) || port64 > 65535) {
219163
return nullptr;
220164
}
221-
StatusOr<sockaddr_in> sa4 = parseV4Address(ip_str, port64);
165+
StatusOr<sockaddr_in> sa4 = IpAddressParsing::parseIPv4(ip_str, static_cast<uint16_t>(port64));
222166
if (sa4.ok()) {
223167
return instanceOrNull(Address::InstanceFactory::createInstancePtr<Address::Ipv4Instance>(
224168
&sa4.value(), nullptr, network_namespace));

‎test/common/network/BUILD

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -405,6 +405,15 @@ envoy_cc_test(
405405
],
406406
)
407407

408+
envoy_cc_test(
409+
name = "ip_address_parsing_test",
410+
srcs = ["ip_address_parsing_test.cc"],
411+
rbe_pool = "6gig",
412+
deps = [
413+
"//source/common/network:ip_address_parsing_lib",
414+
],
415+
)
416+
408417
envoy_cc_fuzz_test(
409418
name = "udp_fuzz_test",
410419
srcs = ["udp_fuzz.cc"],

‎test/common/network/ip_address_parsing_test.cc

Lines changed: 38 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,38 @@
1+
#include <arpa/inet.h>
2+
3+
#include "source/common/network/ip_address_parsing.h"
4+
5+
#include "gtest/gtest.h"
6+
7+
namespace Envoy {
8+
namespace Network {
9+
10+
TEST(IpAddressParsingTest, ParseIPv4Basic) {
11+
auto sa4_or = IpAddressParsing::parseIPv4("127.0.0.1", /*port=*/8080);
12+
ASSERT_TRUE(sa4_or.ok());
13+
const sockaddr_in sa4 = sa4_or.value();
14+
EXPECT_EQ(AF_INET, sa4.sin_family);
15+
EXPECT_EQ(htons(8080), sa4.sin_port);
16+
EXPECT_EQ(htonl(INADDR_LOOPBACK), sa4.sin_addr.s_addr);
17+
18+
EXPECT_FALSE(IpAddressParsing::parseIPv4("1.2.3", 0).ok());
19+
EXPECT_FALSE(IpAddressParsing::parseIPv4("1.2.3.256", 0).ok());
20+
EXPECT_FALSE(IpAddressParsing::parseIPv4("not_an_ip", 0).ok());
21+
}
22+
23+
TEST(IpAddressParsingTest, ParseIPv6Basic) {
24+
auto sa6_or = IpAddressParsing::parseIPv6("::1", /*port=*/443);
25+
ASSERT_TRUE(sa6_or.ok());
26+
const sockaddr_in6 sa6 = sa6_or.value();
27+
EXPECT_EQ(AF_INET6, sa6.sin6_family);
28+
EXPECT_EQ(htons(443), sa6.sin6_port);
29+
in6_addr loopback = IN6ADDR_LOOPBACK_INIT;
30+
EXPECT_EQ(0, memcmp(&loopback, &sa6.sin6_addr, sizeof(in6_addr)));
31+
32+
EXPECT_FALSE(IpAddressParsing::parseIPv6("::g", 0).ok());
33+
EXPECT_FALSE(IpAddressParsing::parseIPv6("1:::1", 0).ok());
34+
EXPECT_FALSE(IpAddressParsing::parseIPv6("not_an_ip", 0).ok());
35+
}
36+
37+
} // namespace Network
38+
} // namespace Envoy

0 commit comments

Comments
 (0)