Skip to content

Commit

Permalink
network: Allow interfaceName() to gracefully handle getifaddrs failur…
Browse files Browse the repository at this point in the history
…es (envoyproxy#32238)

In the mobile world, getifaddrs sometimes returns ERRNO 19 (ENODEV). Given that interfaceName() is designed to return an optional anyway, release assert is not the right thing to do here and instead, we log a warning and return nullopt.

interfaceName() is mainly used for logging purposes, so this change seems low risk.

Risk Level: low
Testing: unit test
Signed-off-by: Ali Beyad <[email protected]>
  • Loading branch information
abeyad authored Feb 8, 2024
1 parent 0c98e29 commit 87fa480
Show file tree
Hide file tree
Showing 2 changed files with 28 additions and 1 deletion.
5 changes: 4 additions & 1 deletion source/common/network/io_socket_handle_base_impl.cc
Original file line number Diff line number Diff line change
Expand Up @@ -134,7 +134,10 @@ absl::optional<std::string> IoSocketHandleBaseImpl::interfaceName() {

Api::InterfaceAddressVector interface_addresses{};
const Api::SysCallIntResult rc = os_syscalls_singleton.getifaddrs(interface_addresses);
RELEASE_ASSERT(!rc.return_value_, fmt::format("getifaddrs error: {}", rc.errno_));
if (rc.return_value_ != 0) {
ENVOY_LOG_EVERY_POW_2(warn, "getifaddrs error: {}", rc.errno_);
return absl::nullopt;
}

absl::optional<std::string> selected_interface_name{};
for (const auto& interface_address : interface_addresses) {
Expand Down
24 changes: 24 additions & 0 deletions test/common/network/io_socket_handle_impl_test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -154,6 +154,30 @@ TEST(IoSocketHandleImpl, NullptrIfaddrs) {
EXPECT_FALSE(maybe_interface_name.has_value());
}

TEST(IoSocketHandleImpl, ErrnoIfaddrs) {
auto& os_syscalls_singleton = Api::OsSysCallsSingleton::get();
auto socket = std::make_shared<Network::Test::TcpListenSocketImmediateListen>(
Network::Test::getCanonicalLoopbackAddress(Address::IpVersion::v4));

NiceMock<Api::MockOsSysCalls> os_sys_calls;
TestThreadsafeSingletonInjector<Api::OsSysCallsImpl> os_calls(&os_sys_calls);

EXPECT_CALL(os_sys_calls, supportsGetifaddrs()).WillRepeatedly(Return(true));
EXPECT_CALL(os_sys_calls, getsockname(_, _, _))
.WillOnce(
Invoke([&](os_fd_t sockfd, sockaddr* addr, socklen_t* addrlen) -> Api::SysCallIntResult {
os_syscalls_singleton.getsockname(sockfd, addr, addrlen);
return {0, 0};
}));
EXPECT_CALL(os_sys_calls, getifaddrs(_))
.WillOnce(Invoke([&](Api::InterfaceAddressVector&) -> Api::SysCallIntResult {
return {/*return_value=*/-1, /*errno=*/19};
}));

const auto maybe_interface_name = socket->ioHandle().interfaceName();
EXPECT_FALSE(maybe_interface_name.has_value());
}

class IoSocketHandleImplTest : public testing::TestWithParam<Network::Address::IpVersion> {};
INSTANTIATE_TEST_SUITE_P(IpVersions, IoSocketHandleImplTest,
testing::ValuesIn(TestEnvironment::getIpVersionsForTest()),
Expand Down

0 comments on commit 87fa480

Please sign in to comment.