diff --git a/contrib/vcl/source/vcl_io_handle.cc b/contrib/vcl/source/vcl_io_handle.cc index 8078d35e65a9..667d329f0dda 100644 --- a/contrib/vcl/source/vcl_io_handle.cc +++ b/contrib/vcl/source/vcl_io_handle.cc @@ -625,7 +625,7 @@ absl::optional VclIoHandle::domain() { return {AF_INET}; }; -Envoy::Network::Address::InstanceConstSharedPtr VclIoHandle::localAddress() { +absl::StatusOr VclIoHandle::localAddress() { vppcom_endpt_t ep; uint32_t eplen = sizeof(ep); uint8_t addr_buf[sizeof(struct sockaddr_in6)]; @@ -636,7 +636,7 @@ Envoy::Network::Address::InstanceConstSharedPtr VclIoHandle::localAddress() { return vclEndptToAddress(ep, sh_); } -Envoy::Network::Address::InstanceConstSharedPtr VclIoHandle::peerAddress() { +absl::StatusOr VclIoHandle::peerAddress() { VCL_LOG("grabbing peer address sh {:x}", sh_); vppcom_endpt_t ep; uint32_t eplen = sizeof(ep); diff --git a/contrib/vcl/source/vcl_io_handle.h b/contrib/vcl/source/vcl_io_handle.h index 167f9c2370c6..94d502cc0fec 100644 --- a/contrib/vcl/source/vcl_io_handle.h +++ b/contrib/vcl/source/vcl_io_handle.h @@ -66,8 +66,8 @@ class VclIoHandle : public Envoy::Network::IoHandle, unsigned long out_buffer_len, unsigned long* bytes_returned) override; Api::SysCallIntResult setBlocking(bool blocking) override; absl::optional domain() override; - Envoy::Network::Address::InstanceConstSharedPtr localAddress() override; - Envoy::Network::Address::InstanceConstSharedPtr peerAddress() override; + absl::StatusOr localAddress() override; + absl::StatusOr peerAddress() override; Api::SysCallIntResult shutdown(int) override { return {0, 0}; } void initializeFileEvent(Event::Dispatcher& dispatcher, Event::FileReadyCb cb, diff --git a/envoy/network/io_handle.h b/envoy/network/io_handle.h index 474c81f7959d..466738275ec7 100644 --- a/envoy/network/io_handle.h +++ b/envoy/network/io_handle.h @@ -293,15 +293,15 @@ class IoHandle { /** * Get local address (ip:port pair) - * @return local address as @ref Address::InstanceConstSharedPtr + * @return local address as @ref Address::InstanceConstSharedPtr or error status. */ - virtual Address::InstanceConstSharedPtr localAddress() PURE; + virtual absl::StatusOr localAddress() PURE; /** * Get peer's address (ip:port pair) - * @return peer's address as @ref Address::InstanceConstSharedPtr + * @return peer's address as @ref Address::InstanceConstSharedPtr or error status. */ - virtual Address::InstanceConstSharedPtr peerAddress() PURE; + virtual absl::StatusOr peerAddress() PURE; /** * Duplicates the handle. This is intended to be used only on listener sockets. (see man dup) diff --git a/source/common/network/io_socket_handle_base_impl.cc b/source/common/network/io_socket_handle_base_impl.cc index 8cee42c39ae3..71635facb6a0 100644 --- a/source/common/network/io_socket_handle_base_impl.cc +++ b/source/common/network/io_socket_handle_base_impl.cc @@ -63,7 +63,7 @@ Api::SysCallIntResult IoSocketHandleBaseImpl::setBlocking(bool blocking) { absl::optional IoSocketHandleBaseImpl::domain() { return domain_; } -Address::InstanceConstSharedPtr IoSocketHandleBaseImpl::localAddress() { +absl::StatusOr IoSocketHandleBaseImpl::localAddress() { sockaddr_storage ss; socklen_t ss_len = sizeof(ss); memset(&ss, 0, ss_len); @@ -71,14 +71,13 @@ Address::InstanceConstSharedPtr IoSocketHandleBaseImpl::localAddress() { Api::SysCallIntResult result = os_sys_calls.getsockname(fd_, reinterpret_cast(&ss), &ss_len); if (result.return_value_ != 0) { - throwEnvoyExceptionOrPanic(fmt::format("getsockname failed for '{}': ({}) {}", fd_, - result.errno_, errorDetails(result.errno_))); + return absl::InvalidArgumentError(fmt::format("getsockname failed for '{}': ({}) {}", fd_, + result.errno_, errorDetails(result.errno_))); } - return THROW_OR_RETURN_VALUE(Address::addressFromSockAddr(ss, ss_len, socket_v6only_), - Address::InstanceConstSharedPtr); + return Address::addressFromSockAddr(ss, ss_len, socket_v6only_); } -Address::InstanceConstSharedPtr IoSocketHandleBaseImpl::peerAddress() { +absl::StatusOr IoSocketHandleBaseImpl::peerAddress() { sockaddr_storage ss; socklen_t ss_len = sizeof(ss); memset(&ss, 0, ss_len); @@ -86,7 +85,7 @@ Address::InstanceConstSharedPtr IoSocketHandleBaseImpl::peerAddress() { Api::SysCallIntResult result = os_sys_calls.getpeername(fd_, reinterpret_cast(&ss), &ss_len); if (result.return_value_ != 0) { - throwEnvoyExceptionOrPanic( + return absl::InvalidArgumentError( fmt::format("getpeername failed for '{}': {}", fd_, errorDetails(result.errno_))); } @@ -103,8 +102,7 @@ Address::InstanceConstSharedPtr IoSocketHandleBaseImpl::peerAddress() { fmt::format("getsockname failed for '{}': {}", fd_, errorDetails(result.errno_))); } } - return THROW_OR_RETURN_VALUE(Address::addressFromSockAddr(ss, ss_len, socket_v6only_), - Address::InstanceConstSharedPtr); + return Address::addressFromSockAddr(ss, ss_len, socket_v6only_); } absl::optional IoSocketHandleBaseImpl::lastRoundTripTime() { @@ -131,7 +129,11 @@ absl::optional IoSocketHandleBaseImpl::interfaceName() { return absl::nullopt; } - Address::InstanceConstSharedPtr socket_address = localAddress(); + auto address_or_error = localAddress(); + if (!address_or_error.status().ok()) { + return absl::nullopt; + } + Address::InstanceConstSharedPtr& socket_address = *address_or_error; if (!socket_address || socket_address->type() != Address::Type::Ip) { return absl::nullopt; } diff --git a/source/common/network/io_socket_handle_base_impl.h b/source/common/network/io_socket_handle_base_impl.h index be02fb00b1f0..c6c02b0f7f6b 100644 --- a/source/common/network/io_socket_handle_base_impl.h +++ b/source/common/network/io_socket_handle_base_impl.h @@ -29,8 +29,8 @@ class IoSocketHandleBaseImpl : public IoHandle, protected Logger::Loggable domain() override; - Address::InstanceConstSharedPtr localAddress() override; - Address::InstanceConstSharedPtr peerAddress() override; + absl::StatusOr localAddress() override; + absl::StatusOr peerAddress() override; absl::optional lastRoundTripTime() override; absl::optional congestionWindowInBytes() const override; absl::optional interfaceName() override; diff --git a/source/common/network/socket_impl.cc b/source/common/network/socket_impl.cc index 4c1ede5de5e7..50c8a7d21af4 100644 --- a/source/common/network/socket_impl.cc +++ b/source/common/network/socket_impl.cc @@ -82,7 +82,11 @@ Api::SysCallIntResult SocketImpl::bind(Network::Address::InstanceConstSharedPtr bind_result = io_handle_->bind(address); if (bind_result.return_value_ == 0 && address->ip()->port() == 0) { - connection_info_provider_->setLocalAddress(io_handle_->localAddress()); + auto address_or_error = io_handle_->localAddress(); + if (!address_or_error.status().ok()) { + return Api::SysCallIntResult{-1, HANDLE_ERROR_INVALID}; + } + connection_info_provider_->setLocalAddress(*address_or_error); } return bind_result; } @@ -92,7 +96,11 @@ Api::SysCallIntResult SocketImpl::listen(int backlog) { return io_handle_->liste Api::SysCallIntResult SocketImpl::connect(const Network::Address::InstanceConstSharedPtr address) { auto result = io_handle_->connect(address); if (address->type() == Address::Type::Ip) { - connection_info_provider_->setLocalAddress(io_handle_->localAddress()); + auto address_or_error = io_handle_->localAddress(); + if (!address_or_error.status().ok()) { + return Api::SysCallIntResult{-1, HANDLE_ERROR_INVALID}; + } + connection_info_provider_->setLocalAddress(*address_or_error); } return result; } diff --git a/source/common/network/tcp_listener_impl.cc b/source/common/network/tcp_listener_impl.cc index 3cb9356ba69b..ee7eeb97d812 100644 --- a/source/common/network/tcp_listener_impl.cc +++ b/source/common/network/tcp_listener_impl.cc @@ -87,8 +87,12 @@ absl::Status TcpListenerImpl::onSocketEvent(short flags) { // Get the local address from the new socket if the listener is listening on IP ANY // (e.g., 0.0.0.0 for IPv4) (local_address_ is nullptr in this case). - const Address::InstanceConstSharedPtr& local_address = - local_address_ ? local_address_ : io_handle->localAddress(); + Address::InstanceConstSharedPtr local_address = local_address_; + if (!local_address) { + auto address_or_error = io_handle->localAddress(); + RETURN_IF_NOT_OK_REF(address_or_error.status()); + local_address = *address_or_error; + } // The accept() call that filled in remote_addr doesn't fill in more than the sa_family field // for Unix domain sockets; apparently there isn't a mechanism in the kernel to get the @@ -100,7 +104,9 @@ absl::Status TcpListenerImpl::onSocketEvent(short flags) { Address::InstanceConstSharedPtr remote_address; if (remote_addr.ss_family == AF_UNIX) { - remote_address = io_handle->peerAddress(); + auto address_or_error = io_handle->peerAddress(); + RETURN_IF_NOT_OK_REF(address_or_error.status()); + remote_address = *address_or_error; } else { auto address_or_error = Address::addressFromSockAddr( remote_addr, remote_addr_len, local_address->ip()->version() == Address::IpVersion::v6); diff --git a/source/common/network/utility.cc b/source/common/network/utility.cc index f177756b5f86..9409d67de353 100644 --- a/source/common/network/utility.cc +++ b/source/common/network/utility.cc @@ -394,7 +394,10 @@ Address::InstanceConstSharedPtr Utility::getOriginalDst(Socket& sock) { status = sock.getSocketOption(opt_tp.level(), opt_tp.option(), &is_tp, &flag_len).return_value_; if (status == 0 && is_tp) { - return sock.ioHandle().localAddress(); + auto address_or_error = sock.ioHandle().localAddress(); + if (address_or_error.status().ok()) { + return *address_or_error; + } } } return nullptr; diff --git a/source/common/quic/quic_io_handle_wrapper.h b/source/common/quic/quic_io_handle_wrapper.h index 73f752d87777..78439af0d22c 100644 --- a/source/common/quic/quic_io_handle_wrapper.h +++ b/source/common/quic/quic_io_handle_wrapper.h @@ -117,10 +117,10 @@ class QuicIoHandleWrapper : public Network::IoHandle { return io_handle_.setBlocking(blocking); } absl::optional domain() override { return io_handle_.domain(); } - Network::Address::InstanceConstSharedPtr localAddress() override { + absl::StatusOr localAddress() override { return io_handle_.localAddress(); } - Network::Address::InstanceConstSharedPtr peerAddress() override { + absl::StatusOr peerAddress() override { return io_handle_.peerAddress(); } diff --git a/source/extensions/io_socket/user_space/io_handle_impl.cc b/source/extensions/io_socket/user_space/io_handle_impl.cc index cf86c0700dc7..8ca2088cb4d3 100644 --- a/source/extensions/io_socket/user_space/io_handle_impl.cc +++ b/source/extensions/io_socket/user_space/io_handle_impl.cc @@ -315,11 +315,11 @@ Api::SysCallIntResult IoHandleImpl::setBlocking(bool) { return makeInvalidSyscal absl::optional IoHandleImpl::domain() { return absl::nullopt; } -Network::Address::InstanceConstSharedPtr IoHandleImpl::localAddress() { +absl::StatusOr IoHandleImpl::localAddress() { return IoHandleImpl::getCommonInternalAddress(); } -Network::Address::InstanceConstSharedPtr IoHandleImpl::peerAddress() { +absl::StatusOr IoHandleImpl::peerAddress() { return IoHandleImpl::getCommonInternalAddress(); } diff --git a/source/extensions/io_socket/user_space/io_handle_impl.h b/source/extensions/io_socket/user_space/io_handle_impl.h index e2f1b8587f99..0c75dfda4d8f 100644 --- a/source/extensions/io_socket/user_space/io_handle_impl.h +++ b/source/extensions/io_socket/user_space/io_handle_impl.h @@ -80,8 +80,8 @@ class IoHandleImpl final : public Network::IoHandle, unsigned long*) override; Api::SysCallIntResult setBlocking(bool blocking) override; absl::optional domain() override; - Network::Address::InstanceConstSharedPtr localAddress() override; - Network::Address::InstanceConstSharedPtr peerAddress() override; + absl::StatusOr localAddress() override; + absl::StatusOr peerAddress() override; void initializeFileEvent(Event::Dispatcher& dispatcher, Event::FileReadyCb cb, Event::FileTriggerType trigger, uint32_t events) override; diff --git a/test/common/network/io_uring_socket_handle_impl_integration_test.cc b/test/common/network/io_uring_socket_handle_impl_integration_test.cc index c25fbc7b9844..46878065d611 100644 --- a/test/common/network/io_uring_socket_handle_impl_integration_test.cc +++ b/test/common/network/io_uring_socket_handle_impl_integration_test.cc @@ -125,7 +125,7 @@ class IoUringSocketHandleImplIntegrationTest : public testing::Test { Event::PlatformDefaultTriggerType, Event::FileReadyType::Read); // Connect from io_uring handle. - io_uring_socket_handle_->connect(listener->localAddress()); + io_uring_socket_handle_->connect(*listener->localAddress()); while (error == -1) { dispatcher_->run(Event::Dispatcher::RunType::NonBlock); } @@ -192,7 +192,7 @@ TEST_F(IoUringSocketHandleImplIntegrationTest, Accept) { Event::PlatformDefaultTriggerType, Event::FileReadyType::Read); // Connect from the socket handle. - io_socket_handle_->connect(io_uring_socket_handle_->localAddress()); + io_socket_handle_->connect(*io_uring_socket_handle_->localAddress()); while (!accepted) { dispatcher_->run(Event::Dispatcher::RunType::NonBlock); } diff --git a/test/common/quic/quic_io_handle_wrapper_test.cc b/test/common/quic/quic_io_handle_wrapper_test.cc index b512ceff2a38..30622d2e7b13 100644 --- a/test/common/quic/quic_io_handle_wrapper_test.cc +++ b/test/common/quic/quic_io_handle_wrapper_test.cc @@ -72,7 +72,7 @@ TEST_F(QuicIoHandleWrapperTest, DelegateIoHandleCalls) { *addrlen = sizeof(sockaddr_in6); return Api::SysCallIntResult{0, 0}; })); - addr = wrapper_->localAddress(); + addr = *wrapper_->localAddress(); EXPECT_CALL(os_sys_calls_, getpeername(_, _, _)) .WillOnce(Invoke([](os_fd_t, sockaddr* addr, socklen_t* addrlen) -> Api::SysCallIntResult { @@ -80,7 +80,7 @@ TEST_F(QuicIoHandleWrapperTest, DelegateIoHandleCalls) { *addrlen = sizeof(sockaddr_in6); return Api::SysCallIntResult{0, 0}; })); - addr = wrapper_->peerAddress(); + addr = *wrapper_->peerAddress(); Network::IoHandle::RecvMsgOutput output(1, nullptr); EXPECT_CALL(os_sys_calls_, recvmsg(fd, _, MSG_TRUNC)) diff --git a/test/extensions/io_socket/user_space/connection_compatbility_test.cc b/test/extensions/io_socket/user_space/connection_compatbility_test.cc index c5044fae25f8..e3018d9c4dc9 100644 --- a/test/extensions/io_socket/user_space/connection_compatbility_test.cc +++ b/test/extensions/io_socket/user_space/connection_compatbility_test.cc @@ -31,8 +31,8 @@ class InternalClientConnectionImplTest : public testing::Test { void SetUp() override { std::tie(io_handle_, io_handle_peer_) = IoHandleFactory::createIoHandlePair(); - local_addr_ = io_handle_->localAddress(); - remote_addr_ = io_handle_->peerAddress(); + local_addr_ = *io_handle_->localAddress(); + remote_addr_ = *io_handle_->peerAddress(); } Api::ApiPtr api_; Event::DispatcherPtr dispatcher_; diff --git a/test/extensions/io_socket/user_space/io_handle_impl_test.cc b/test/extensions/io_socket/user_space/io_handle_impl_test.cc index 7a39edd4ccde..bea47e6bdd96 100644 --- a/test/extensions/io_socket/user_space/io_handle_impl_test.cc +++ b/test/extensions/io_socket/user_space/io_handle_impl_test.cc @@ -1059,12 +1059,12 @@ TEST_F(IoHandleImplTest, NotifyWritableAfterShutdownWrite) { } TEST_F(IoHandleImplTest, ReturnValidInternalAddress) { - const auto& local_address = io_handle_->localAddress(); + const auto local_address = *io_handle_->localAddress(); ASSERT_NE(nullptr, local_address); ASSERT_EQ(nullptr, local_address->ip()); ASSERT_EQ(nullptr, local_address->pipe()); ASSERT_NE(nullptr, local_address->envoyInternalAddress()); - const auto& remote_address = io_handle_->peerAddress(); + const auto remote_address = *io_handle_->peerAddress(); ASSERT_NE(nullptr, remote_address); ASSERT_EQ(nullptr, remote_address->ip()); ASSERT_EQ(nullptr, remote_address->pipe()); diff --git a/test/integration/filters/test_socket_interface.h b/test/integration/filters/test_socket_interface.h index 8f4488d7b728..3c96e8a11c82 100644 --- a/test/integration/filters/test_socket_interface.h +++ b/test/integration/filters/test_socket_interface.h @@ -62,7 +62,7 @@ class TestIoSocketHandle : public Test::IoSocketHandlePlatformImpl { // HTTP/3 sockets won't have a bound peer address, but instead get peer // address from the argument in sendmsg. TestIoSocketHandle::sendmsg will // stash that in peer_address_override_. - Address::InstanceConstSharedPtr peerAddress() override { + absl::StatusOr peerAddress() override { if (peer_address_override_.has_value()) { return Network::Utility::getAddressWithPort( peer_address_override_.value().get(), peer_address_override_.value().get().ip()->port()); diff --git a/test/integration/socket_interface_swap.h b/test/integration/socket_interface_swap.h index a35981a799e7..03f5b7676dbd 100644 --- a/test/integration/socket_interface_swap.h +++ b/test/integration/socket_interface_swap.h @@ -21,8 +21,8 @@ class SocketInterfaceSwap { Network::Address::InstanceConstSharedPtr& peer_address_override_out) { absl::MutexLock lock(&mutex_); if (socket_type_ == io_handle->getSocketType() && error_ && - (io_handle->localAddress()->ip()->port() == src_port_ || - (dst_port_ && io_handle->peerAddress()->ip()->port() == dst_port_))) { + ((*io_handle->localAddress())->ip()->port() == src_port_ || + (dst_port_ && (*io_handle->peerAddress())->ip()->port() == dst_port_))) { ASSERT(matched_iohandle_ == nullptr || matched_iohandle_ == io_handle, "Matched multiple io_handles, expected at most one to match."); matched_iohandle_ = io_handle; @@ -31,7 +31,7 @@ class SocketInterfaceSwap { : Envoy::Network::IoSocketError::create(error_->getSystemErrorCode()); } - if (orig_dnat_address_ != nullptr && *orig_dnat_address_ == *io_handle->peerAddress()) { + if (orig_dnat_address_ != nullptr && *orig_dnat_address_ == **io_handle->peerAddress()) { ASSERT(translated_dnat_address_ != nullptr); peer_address_override_out = translated_dnat_address_; } @@ -44,8 +44,8 @@ class SocketInterfaceSwap { Network::Address::InstanceConstSharedPtr& peer_address_override_out) { absl::MutexLock lock(&mutex_); if (block_connect_ && socket_type_ == io_handle->getSocketType() && - (io_handle->localAddress()->ip()->port() == src_port_ || - (dst_port_ && io_handle->peerAddress()->ip()->port() == dst_port_))) { + ((*io_handle->localAddress())->ip()->port() == src_port_ || + (dst_port_ && (*io_handle->peerAddress())->ip()->port() == dst_port_))) { return Network::IoSocketError::getIoSocketEagainError(); } diff --git a/test/mocks/network/io_handle.h b/test/mocks/network/io_handle.h index 8830e75584e4..4aec8614d3de 100644 --- a/test/mocks/network/io_handle.h +++ b/test/mocks/network/io_handle.h @@ -52,8 +52,8 @@ class MockIoHandle : public IoHandle { (int level, int optname, void* optval, socklen_t* optlen)); MOCK_METHOD(Api::SysCallIntResult, setBlocking, (bool blocking)); MOCK_METHOD(absl::optional, domain, ()); - MOCK_METHOD(Address::InstanceConstSharedPtr, localAddress, ()); - MOCK_METHOD(Address::InstanceConstSharedPtr, peerAddress, ()); + MOCK_METHOD(absl::StatusOr, localAddress, ()); + MOCK_METHOD(absl::StatusOr, peerAddress, ()); MOCK_METHOD(IoHandlePtr, duplicate, ()); MOCK_METHOD(void, createFileEvent_, (Event::Dispatcher & dispatcher, Event::FileReadyCb cb,