Skip to content

Commit

Permalink
address: removing the throwing variant of parseInternetAddress (#34654)
Browse files Browse the repository at this point in the history
Risk Level: low
Testing: updated tests
Docs Changes: n/a
Release Notes: n/a
envoyproxy/envoy-mobile#176

Signed-off-by: Alyssa Wilk <alyssar@chromium.org>
  • Loading branch information
alyssawilk committed Jun 12, 2024
1 parent 82dc695 commit ceb7d07
Show file tree
Hide file tree
Showing 53 changed files with 217 additions and 212 deletions.
2 changes: 1 addition & 1 deletion mobile/test/common/integration/client_integration_test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -714,7 +714,7 @@ TEST_P(ClientIntegrationTest, ReresolveAndDrain) {
return; // This test relies on ipv4 loopback.
}

auto next_address = Network::Utility::parseInternetAddress(
auto next_address = Network::Utility::parseInternetAddressNoThrow(
"127.0.0.3", fake_upstreams_[0]->localAddress()->ip()->port());
// This will hopefully be miniminally flaky because of low use of 127.0.0.3
// but may need to be disabled.
Expand Down
20 changes: 11 additions & 9 deletions mobile/test/common/network/src_addr_socket_option_impl_test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -30,36 +30,36 @@ class SrcAddrSocketOptionImplTest : public testing::Test {
};

TEST_F(SrcAddrSocketOptionImplTest, TestSetOptionPreBindSetsAddress) {
const auto address = Network::Utility::parseInternetAddress("127.0.0.2");
const auto address = Network::Utility::parseInternetAddressNoThrow("127.0.0.2");
auto option = makeOptionByAddress(address);
EXPECT_TRUE(option->setOption(socket_, envoy::config::core::v3::SocketOption::STATE_PREBIND));
EXPECT_EQ(*socket_.connection_info_provider_->localAddress(), *address);
}

TEST_F(SrcAddrSocketOptionImplTest, TestSetOptionPreBindSetsAddressSecond) {
const auto address = Network::Utility::parseInternetAddress("1.2.3.4");
const auto address = Network::Utility::parseInternetAddressNoThrow("1.2.3.4");
auto option = makeOptionByAddress(address);
EXPECT_TRUE(option->setOption(socket_, envoy::config::core::v3::SocketOption::STATE_PREBIND));
EXPECT_EQ(*socket_.connection_info_provider_->localAddress(), *address);
}

TEST_F(SrcAddrSocketOptionImplTest, TestSetOptionNotPrebindDoesNotSetAddress) {
const auto address = Network::Utility::parseInternetAddress("1.2.3.4");
const auto address = Network::Utility::parseInternetAddressNoThrow("1.2.3.4");
auto option = makeOptionByAddress(address);
EXPECT_TRUE(option->setOption(socket_, envoy::config::core::v3::SocketOption::STATE_LISTENING));
EXPECT_NE(*socket_.connection_info_provider_->localAddress(), *address);
}

TEST_F(SrcAddrSocketOptionImplTest, TestSetOptionSafeWithNullAddress) {
const auto address = Network::Utility::parseInternetAddress("4.3.2.1");
const auto address = Network::Utility::parseInternetAddressNoThrow("4.3.2.1");
socket_.connection_info_provider_->setLocalAddress(address);
auto option = std::make_unique<SrcAddrSocketOptionImpl>(nullptr);
EXPECT_TRUE(option->setOption(socket_, envoy::config::core::v3::SocketOption::STATE_PREBIND));
EXPECT_EQ(*socket_.connection_info_provider_->localAddress(), *address);
}

TEST_F(SrcAddrSocketOptionImplTest, TestIpv4HashKey) {
const auto address = Network::Utility::parseInternetAddress("1.2.3.4");
const auto address = Network::Utility::parseInternetAddressNoThrow("1.2.3.4");
auto option = makeOptionByAddress(address);
option->hashKey(key_);

Expand All @@ -69,7 +69,7 @@ TEST_F(SrcAddrSocketOptionImplTest, TestIpv4HashKey) {
}

TEST_F(SrcAddrSocketOptionImplTest, TestIpv4HashKeyOther) {
const auto address = Network::Utility::parseInternetAddress("255.254.253.0");
const auto address = Network::Utility::parseInternetAddressNoThrow("255.254.253.0");
auto option = makeOptionByAddress(address);
option->hashKey(key_);

Expand All @@ -79,7 +79,8 @@ TEST_F(SrcAddrSocketOptionImplTest, TestIpv4HashKeyOther) {
}

TEST_F(SrcAddrSocketOptionImplTest, TestIpv6HashKey) {
const auto address = Network::Utility::parseInternetAddress("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_);

Expand All @@ -89,7 +90,8 @@ TEST_F(SrcAddrSocketOptionImplTest, TestIpv6HashKey) {
}

TEST_F(SrcAddrSocketOptionImplTest, TestIpv6HashKeyOther) {
const auto address = Network::Utility::parseInternetAddress("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_);

Expand All @@ -99,7 +101,7 @@ TEST_F(SrcAddrSocketOptionImplTest, TestIpv6HashKeyOther) {
}

TEST_F(SrcAddrSocketOptionImplTest, TestOptionDetailsNotSupported) {
const auto address = Network::Utility::parseInternetAddress("255.254.253.0");
const auto address = Network::Utility::parseInternetAddressNoThrow("255.254.253.0");
auto option = makeOptionByAddress(address);

auto details =
Expand Down
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
Loading

0 comments on commit ceb7d07

Please sign in to comment.