Skip to content

Commit

Permalink
address: removing the throwing variant of parseInternetAddress
Browse files Browse the repository at this point in the history
Signed-off-by: Alyssa Wilk <alyssar@chromium.org>
  • Loading branch information
alyssawilk committed Jun 10, 2024
1 parent 6b9584f commit fc8b73d
Show file tree
Hide file tree
Showing 51 changed files with 205 additions and 202 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ namespace FilterChain {
// when no IP matcher is configured.
Network::Address::InstanceConstSharedPtr fakeAddress() {
CONSTRUCT_ON_FIRST_USE(Network::Address::InstanceConstSharedPtr,
Network::Utility::parseInternetAddress("255.255.255.255"));
Network::Utility::parseInternetAddressNoThrow("255.255.255.255"));
}

struct FilterChainNameAction
Expand Down
10 changes: 8 additions & 2 deletions source/common/network/resolver_impl.cc
Original file line number Diff line number Diff line change
Expand Up @@ -25,9 +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:
return Network::Utility::parseInternetAddress(
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 addr;
}
case envoy::config::core::v3::SocketAddress::PortSpecifierCase::kNamedPort:
break;
}
Expand Down
18 changes: 4 additions & 14 deletions source/common/network/utility.cc
Original file line number Diff line number Diff line change
Expand Up @@ -160,16 +160,6 @@ Address::InstanceConstSharedPtr Utility::parseInternetAddressNoThrow(const std::
return nullptr;
}

Address::InstanceConstSharedPtr Utility::parseInternetAddress(const std::string& ip_address,
uint16_t port, bool v6only) {
const Address::InstanceConstSharedPtr address =
parseInternetAddressNoThrow(ip_address, port, v6only);
if (address == nullptr) {
throwEnvoyExceptionOrPanic(absl::StrCat("malformed IP address: ", ip_address));
}
return address;
}

Address::InstanceConstSharedPtr
Utility::parseInternetAddressAndPortNoThrow(const std::string& ip_address, bool v6only) {
if (ip_address.empty()) {
Expand Down Expand Up @@ -446,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());
Expand Down
18 changes: 5 additions & 13 deletions source/common/network/utility.h
Original file line number Diff line number Diff line change
Expand Up @@ -240,18 +240,6 @@ class Utility {
static Address::InstanceConstSharedPtr getAddressWithPort(const Address::Instance& address,
uint32_t port);

/**
* Parse an internet host address (IPv4 or IPv6) and create an Instance from it. The address must
* not include a port number. Throws EnvoyException if unable to parse the address.
* @param ip_address string to be parsed as an internet address.
* @param port optional port to include in Instance created from ip_address, 0 by default.
* @param v6only disable IPv4-IPv6 mapping for IPv6 addresses?
* @return pointer to the Instance.
* @throw EnvoyException in case of a malformed IP address.
*/
static Address::InstanceConstSharedPtr
parseInternetAddress(const std::string& ip_address, uint16_t port = 0, bool v6only = true);

/**
* Retrieve the original destination address from an accepted socket.
* The address (IP and port) may be not local and the port may differ from
Expand All @@ -275,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.
Expand Down
2 changes: 1 addition & 1 deletion source/common/upstream/upstream_impl.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand Down
5 changes: 4 additions & 1 deletion source/extensions/filters/udp/dns_filter/dns_filter.cc
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,10 @@ DnsFilterEnvoyConfig::DnsFilterEnvoyConfig(
// Creating the IP address will throw an exception if the address string is malformed
for (auto index = 0; index < address_list.size(); index++) {
const auto address_iter = std::next(address_list.begin(), (i++ % address_list.size()));
auto ipaddr = Network::Utility::parseInternetAddress(*address_iter, 0 /* port */);
auto ipaddr = Network::Utility::parseInternetAddressNoThrow(*address_iter, 0 /* port */);
if (!ipaddr) {
throw EnvoyException(absl::StrCat("malformed IP address: ", *address_iter));
}
addrs.push_back(std::move(ipaddr));
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down
2 changes: 1 addition & 1 deletion source/extensions/tracers/xray/daemon_broker.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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));
}
}

Expand Down
6 changes: 3 additions & 3 deletions test/common/common/dns_utils_test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -20,11 +20,11 @@ TEST(DnsUtils, MultipleGenerateTest) {

TEST(DnsUtils, ListChanged) {
Network::Address::InstanceConstSharedPtr address1 =
Network::Utility::parseInternetAddress("10.0.0.1");
Network::Utility::parseInternetAddressNoThrow("10.0.0.1");
Network::Address::InstanceConstSharedPtr address1_dup =
Network::Utility::parseInternetAddress("10.0.0.1");
Network::Utility::parseInternetAddressNoThrow("10.0.0.1");
Network::Address::InstanceConstSharedPtr address2 =
Network::Utility::parseInternetAddress("10.0.0.2");
Network::Utility::parseInternetAddressNoThrow("10.0.0.2");

std::vector<Network::Address::InstanceConstSharedPtr> addresses1 = {address1, address2};
std::vector<Network::Address::InstanceConstSharedPtr> addresses2 = {address1_dup, address2};
Expand Down
6 changes: 3 additions & 3 deletions test/common/listener_manager/filter_chain_benchmark_test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -56,16 +56,16 @@ class MockConnectionSocket : public Network::ConnectionSocket {
std::make_shared<Network::Address::PipeInstance>(destination_address));
} else {
res->connection_info_provider_->setLocalAddress(
Network::Utility::parseInternetAddress(destination_address, destination_port));
Network::Utility::parseInternetAddressNoThrow(destination_address, destination_port));
}
if (absl::StartsWith(source_address, "/")) {
res->connection_info_provider_->setRemoteAddress(
std::make_shared<Network::Address::PipeInstance>(source_address));
} else {
res->connection_info_provider_->setRemoteAddress(
Network::Utility::parseInternetAddress(source_address, source_port));
Network::Utility::parseInternetAddressNoThrow(source_address, source_port));
res->connection_info_provider_->setDirectRemoteAddressForTest(
Network::Utility::parseInternetAddress(source_address, source_port));
Network::Utility::parseInternetAddressNoThrow(source_address, source_port));
}
res->server_name_ = server_name;
res->ja3_hash_ = ja3_hash;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -82,7 +82,7 @@ class FilterChainManagerImplTest : public testing::TestWithParam<bool> {
local_address_ = std::make_shared<Network::Address::PipeInstance>(destination_address);
} else {
local_address_ =
Network::Utility::parseInternetAddress(destination_address, destination_port);
Network::Utility::parseInternetAddressNoThrow(destination_address, destination_port);
}
mock_socket->connection_info_provider_->setLocalAddress(local_address_);

Expand All @@ -96,7 +96,7 @@ class FilterChainManagerImplTest : public testing::TestWithParam<bool> {
if (absl::StartsWith(source_address, "/")) {
remote_address_ = std::make_shared<Network::Address::PipeInstance>(source_address);
} else {
remote_address_ = Network::Utility::parseInternetAddress(source_address, source_port);
remote_address_ = Network::Utility::parseInternetAddressNoThrow(source_address, source_port);
}
mock_socket->connection_info_provider_->setRemoteAddress(remote_address_);
NiceMock<StreamInfo::MockStreamInfo> stream_info;
Expand Down
3 changes: 2 additions & 1 deletion test/common/listener_manager/listener_manager_impl_test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -6519,7 +6519,8 @@ TEST_P(ListenerManagerImplWithRealFiltersTest, AddressResolver) {
NiceMock<Network::MockAddressResolver> mock_resolver;
EXPECT_CALL(mock_resolver, resolve(_))
.Times(2)
.WillRepeatedly(Return(Network::Utility::parseInternetAddress("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));
Expand Down
6 changes: 3 additions & 3 deletions test/common/listener_manager/listener_manager_impl_test.h
Original file line number Diff line number Diff line change
Expand Up @@ -228,7 +228,7 @@ class ListenerManagerImplTest : public testing::TestWithParam<bool> {
local_address_ = std::make_shared<Network::Address::PipeInstance>(destination_address);
} else {
local_address_ =
Network::Utility::parseInternetAddress(destination_address, destination_port);
Network::Utility::parseInternetAddressNoThrow(destination_address, destination_port);
}
socket_->connection_info_provider_->setLocalAddress(local_address_);

Expand All @@ -242,7 +242,7 @@ class ListenerManagerImplTest : public testing::TestWithParam<bool> {
if (absl::StartsWith(source_address, "/")) {
remote_address_ = std::make_shared<Network::Address::PipeInstance>(source_address);
} else {
remote_address_ = Network::Utility::parseInternetAddress(source_address, source_port);
remote_address_ = Network::Utility::parseInternetAddressNoThrow(source_address, source_port);
}
socket_->connection_info_provider_->setRemoteAddress(remote_address_);

Expand All @@ -254,7 +254,7 @@ class ListenerManagerImplTest : public testing::TestWithParam<bool> {
std::make_shared<Network::Address::PipeInstance>(direct_source_address);
} else {
direct_remote_address_ =
Network::Utility::parseInternetAddress(direct_source_address, source_port);
Network::Utility::parseInternetAddressNoThrow(direct_source_address, source_port);
}
socket_->connection_info_provider_->setDirectRemoteAddressForTest(direct_remote_address_);

Expand Down
31 changes: 18 additions & 13 deletions test/common/network/address_impl_test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -100,7 +100,7 @@ void testSocketBindAndConnect(Network::Address::IpVersion ip_version, bool v6onl

if (!v6only) {
ASSERT_EQ(IpVersion::v6, addr_port->ip()->version());
auto v4_addr_port = Network::Utility::parseInternetAddress(
auto v4_addr_port = Network::Utility::parseInternetAddressNoThrow(
Network::Test::getLoopbackAddressUrlString(Network::Address::IpVersion::v4),
addr_port->ip()->port(), true);
ASSERT_NE(v4_addr_port, nullptr);
Expand Down Expand Up @@ -157,7 +157,7 @@ TEST(Ipv4InstanceTest, SocketAddress) {
EXPECT_EQ("1.2.3.4", address.ip()->addressAsString());
EXPECT_EQ(6502U, address.ip()->port());
EXPECT_EQ(IpVersion::v4, address.ip()->version());
EXPECT_TRUE(addressesEqual(Network::Utility::parseInternetAddress("1.2.3.4"), address));
EXPECT_TRUE(addressesEqual(Network::Utility::parseInternetAddressNoThrow("1.2.3.4"), address));
EXPECT_EQ(nullptr, address.ip()->ipv6());
EXPECT_TRUE(address.ip()->isUnicastAddress());
EXPECT_EQ(nullptr, address.pipe());
Expand All @@ -172,7 +172,7 @@ TEST(Ipv4InstanceTest, AddressOnly) {
EXPECT_EQ("3.4.5.6", address.ip()->addressAsString());
EXPECT_EQ(0U, address.ip()->port());
EXPECT_EQ(IpVersion::v4, address.ip()->version());
EXPECT_TRUE(addressesEqual(Network::Utility::parseInternetAddress("3.4.5.6"), address));
EXPECT_TRUE(addressesEqual(Network::Utility::parseInternetAddressNoThrow("3.4.5.6"), address));
EXPECT_TRUE(address.ip()->isUnicastAddress());
}

Expand All @@ -185,7 +185,7 @@ TEST(Ipv4InstanceTest, AddressAndPort) {
EXPECT_FALSE(address.ip()->isAnyAddress());
EXPECT_EQ(80U, address.ip()->port());
EXPECT_EQ(IpVersion::v4, address.ip()->version());
EXPECT_TRUE(addressesEqual(Network::Utility::parseInternetAddress("127.0.0.1"), address));
EXPECT_TRUE(addressesEqual(Network::Utility::parseInternetAddressNoThrow("127.0.0.1"), address));
EXPECT_TRUE(address.ip()->isUnicastAddress());
}

Expand All @@ -198,7 +198,7 @@ TEST(Ipv4InstanceTest, PortOnly) {
EXPECT_TRUE(address.ip()->isAnyAddress());
EXPECT_EQ(443U, address.ip()->port());
EXPECT_EQ(IpVersion::v4, address.ip()->version());
EXPECT_TRUE(addressesEqual(Network::Utility::parseInternetAddress("0.0.0.0"), address));
EXPECT_TRUE(addressesEqual(Network::Utility::parseInternetAddressNoThrow("0.0.0.0"), address));
EXPECT_FALSE(address.ip()->isUnicastAddress());
}

Expand All @@ -211,7 +211,7 @@ TEST(Ipv4InstanceTest, Multicast) {
EXPECT_FALSE(address.ip()->isAnyAddress());
EXPECT_EQ(0U, address.ip()->port());
EXPECT_EQ(IpVersion::v4, address.ip()->version());
EXPECT_TRUE(addressesEqual(Network::Utility::parseInternetAddress("230.0.0.1"), address));
EXPECT_TRUE(addressesEqual(Network::Utility::parseInternetAddressNoThrow("230.0.0.1"), address));
EXPECT_FALSE(address.ip()->isUnicastAddress());
}

Expand All @@ -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::parseInternetAddress("255.255.255.255"), address));
EXPECT_TRUE(
addressesEqual(Network::Utility::parseInternetAddressNoThrow("255.255.255.255"), address));
EXPECT_FALSE(address.ip()->isUnicastAddress());
}

Expand All @@ -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::parseInternetAddress("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());
Expand All @@ -263,7 +265,7 @@ TEST(Ipv6InstanceTest, AddressOnly) {
EXPECT_EQ(0U, address.ip()->port());
EXPECT_EQ(IpVersion::v6, address.ip()->version());
EXPECT_TRUE(addressesEqual(
Network::Utility::parseInternetAddress("2001:db8:85a3::8a2e:0370:7334"), address));
Network::Utility::parseInternetAddressNoThrow("2001:db8:85a3::8a2e:0370:7334"), address));
EXPECT_TRUE(address.ip()->isUnicastAddress());
}

Expand All @@ -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::parseInternetAddress("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());
}

Expand Down Expand Up @@ -306,7 +309,7 @@ TEST(Ipv6InstanceTest, PortOnly) {
EXPECT_TRUE(address.ip()->isAnyAddress());
EXPECT_EQ(443U, address.ip()->port());
EXPECT_EQ(IpVersion::v6, address.ip()->version());
EXPECT_TRUE(addressesEqual(Network::Utility::parseInternetAddress("::0000"), address));
EXPECT_TRUE(addressesEqual(Network::Utility::parseInternetAddressNoThrow("::0000"), address));
EXPECT_FALSE(address.ip()->isUnicastAddress());
}

Expand All @@ -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::parseInternetAddress("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());
}

Expand All @@ -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::parseInternetAddress("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());
}

Expand Down
6 changes: 3 additions & 3 deletions test/common/network/cidr_range_test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -69,7 +69,7 @@ TEST(TruncateIpAddressAndLength, Various) {
{{"ffff::ffff", 999}, {"ffff::ffff", 128}},
};
for (const auto& kv : test_cases) {
InstanceConstSharedPtr inPtr = Utility::parseInternetAddress(kv.first.first);
InstanceConstSharedPtr inPtr = Utility::parseInternetAddressNoThrow(kv.first.first);
EXPECT_NE(inPtr, nullptr) << kv.first.first;
int length_io = kv.first.second;
InstanceConstSharedPtr outPtr = CidrRange::truncateIpAddressAndLength(inPtr, &length_io);
Expand Down Expand Up @@ -202,7 +202,7 @@ TEST(CidrRangeTest, OperatorIsEqual) {
TEST(CidrRangeTest, InvalidCidrRange) { EXPECT_FALSE(CidrRange::create("foo").status().ok()); }

TEST(Ipv4CidrRangeTest, InstanceConstSharedPtrAndLengthCtor) {
InstanceConstSharedPtr ptr = Utility::parseInternetAddress("1.2.3.5");
InstanceConstSharedPtr ptr = Utility::parseInternetAddressNoThrow("1.2.3.5");
CidrRange rng(*CidrRange::create(ptr, 31)); // Copy ctor.
EXPECT_EQ(rng.length(), 31);
EXPECT_EQ(rng.ip()->version(), IpVersion::v4);
Expand Down Expand Up @@ -267,7 +267,7 @@ TEST(Ipv4CidrRangeTest, BigRange) {
}

TEST(Ipv6CidrRange, InstanceConstSharedPtrAndLengthCtor) {
InstanceConstSharedPtr ptr = Utility::parseInternetAddress("abcd::0345");
InstanceConstSharedPtr ptr = Utility::parseInternetAddressNoThrow("abcd::0345");
CidrRange rng(*CidrRange::create(ptr, 127)); // Copy ctor.
EXPECT_EQ(rng.length(), 127);
EXPECT_EQ(rng.ip()->version(), IpVersion::v6);
Expand Down
Loading

0 comments on commit fc8b73d

Please sign in to comment.