diff --git a/source/common/network/address_impl.cc b/source/common/network/address_impl.cc index 69dfc1969bd2..82217aa8ed63 100644 --- a/source/common/network/address_impl.cc +++ b/source/common/network/address_impl.cc @@ -32,19 +32,6 @@ const SocketInterface* sockInterfaceOrDefault(const SocketInterface* sock_interf return sock_interface == nullptr ? &SocketInterfaceSingleton::get() : sock_interface; } -void throwOnError(absl::Status status) { - if (!status.ok()) { - throwEnvoyExceptionOrPanic(status.ToString()); - } -} - -InstanceConstSharedPtr throwOnError(StatusOr address) { - if (!address.ok()) { - throwOnError(address.status()); - } - return *address; -} - } // namespace bool forceV6() { @@ -108,16 +95,6 @@ StatusOr addressFromSockAddr(const sockaddr_sto } } -Address::InstanceConstSharedPtr addressFromSockAddrOrThrow(const sockaddr_storage& ss, - socklen_t ss_len, bool v6only) { - // Though we don't have any test coverage where address validation in addressFromSockAddr() fails, - // this code is called in worker thread and can throw in theory. In that case, the program will - // crash due to uncaught exception. In practice, we don't expect any address validation in - // addressFromSockAddr() to fail in worker thread. - StatusOr address = addressFromSockAddr(ss, ss_len, v6only); - return throwOnError(address); -} - Address::InstanceConstSharedPtr addressFromSockAddrOrDie(const sockaddr_storage& ss, socklen_t ss_len, os_fd_t fd, bool v6only) { // Set v6only to false so that mapped-v6 address can be normalize to v4 @@ -138,7 +115,7 @@ addressFromSockAddrOrDie(const sockaddr_storage& ss, socklen_t ss_len, os_fd_t f Ipv4Instance::Ipv4Instance(const sockaddr_in* address, const SocketInterface* sock_interface) : InstanceBase(Type::Ip, sockInterfaceOrDefault(sock_interface)) { - throwOnError(validateProtocolSupported()); + THROW_IF_NOT_OK(validateProtocolSupported()); initHelper(address); } @@ -148,7 +125,7 @@ Ipv4Instance::Ipv4Instance(const std::string& address, const SocketInterface* so Ipv4Instance::Ipv4Instance(const std::string& address, uint32_t port, const SocketInterface* sock_interface) : InstanceBase(Type::Ip, sockInterfaceOrDefault(sock_interface)) { - throwOnError(validateProtocolSupported()); + THROW_IF_NOT_OK(validateProtocolSupported()); memset(&ip_.ipv4_.address_, 0, sizeof(ip_.ipv4_.address_)); ip_.ipv4_.address_.sin_family = AF_INET; ip_.ipv4_.address_.sin_port = htons(port); @@ -163,7 +140,7 @@ Ipv4Instance::Ipv4Instance(const std::string& address, uint32_t port, Ipv4Instance::Ipv4Instance(uint32_t port, const SocketInterface* sock_interface) : InstanceBase(Type::Ip, sockInterfaceOrDefault(sock_interface)) { - throwOnError(validateProtocolSupported()); + THROW_IF_NOT_OK(validateProtocolSupported()); memset(&ip_.ipv4_.address_, 0, sizeof(ip_.ipv4_.address_)); ip_.ipv4_.address_.sin_family = AF_INET; ip_.ipv4_.address_.sin_port = htons(port); @@ -296,7 +273,7 @@ InstanceConstSharedPtr Ipv6Instance::Ipv6Helper::addressWithoutScopeId() const { Ipv6Instance::Ipv6Instance(const sockaddr_in6& address, bool v6only, const SocketInterface* sock_interface) : InstanceBase(Type::Ip, sockInterfaceOrDefault(sock_interface)) { - throwOnError(validateProtocolSupported()); + THROW_IF_NOT_OK(validateProtocolSupported()); initHelper(address, v6only); } @@ -306,7 +283,7 @@ Ipv6Instance::Ipv6Instance(const std::string& address, const SocketInterface* so Ipv6Instance::Ipv6Instance(const std::string& address, uint32_t port, const SocketInterface* sock_interface, bool v6only) : InstanceBase(Type::Ip, sockInterfaceOrDefault(sock_interface)) { - throwOnError(validateProtocolSupported()); + THROW_IF_NOT_OK(validateProtocolSupported()); sockaddr_in6 addr_in; memset(&addr_in, 0, sizeof(addr_in)); addr_in.sin6_family = AF_INET6; diff --git a/source/common/network/address_impl.h b/source/common/network/address_impl.h index eb65305adaeb..5a52d910e169 100644 --- a/source/common/network/address_impl.h +++ b/source/common/network/address_impl.h @@ -36,8 +36,6 @@ bool forceV6(); */ StatusOr addressFromSockAddr(const sockaddr_storage& ss, socklen_t len, bool v6only = true); -InstanceConstSharedPtr addressFromSockAddrOrThrow(const sockaddr_storage& ss, socklen_t len, - bool v6only = true); /** * Convert an address in the form of the socket address struct defined by Posix, Linux, etc. into diff --git a/source/common/network/io_socket_handle_base_impl.cc b/source/common/network/io_socket_handle_base_impl.cc index 3a2e8f3e79dc..8cee42c39ae3 100644 --- a/source/common/network/io_socket_handle_base_impl.cc +++ b/source/common/network/io_socket_handle_base_impl.cc @@ -74,7 +74,8 @@ Address::InstanceConstSharedPtr IoSocketHandleBaseImpl::localAddress() { throwEnvoyExceptionOrPanic(fmt::format("getsockname failed for '{}': ({}) {}", fd_, result.errno_, errorDetails(result.errno_))); } - return Address::addressFromSockAddrOrThrow(ss, ss_len, socket_v6only_); + return THROW_OR_RETURN_VALUE(Address::addressFromSockAddr(ss, ss_len, socket_v6only_), + Address::InstanceConstSharedPtr); } Address::InstanceConstSharedPtr IoSocketHandleBaseImpl::peerAddress() { @@ -102,7 +103,8 @@ Address::InstanceConstSharedPtr IoSocketHandleBaseImpl::peerAddress() { fmt::format("getsockname failed for '{}': {}", fd_, errorDetails(result.errno_))); } } - return Address::addressFromSockAddrOrThrow(ss, ss_len, socket_v6only_); + return THROW_OR_RETURN_VALUE(Address::addressFromSockAddr(ss, ss_len, socket_v6only_), + Address::InstanceConstSharedPtr); } absl::optional IoSocketHandleBaseImpl::lastRoundTripTime() { diff --git a/source/common/network/tcp_listener_impl.cc b/source/common/network/tcp_listener_impl.cc index 9e0b91ed5acd..3cb9356ba69b 100644 --- a/source/common/network/tcp_listener_impl.cc +++ b/source/common/network/tcp_listener_impl.cc @@ -53,7 +53,7 @@ bool TcpListenerImpl::rejectCxOverGlobalLimit() const { } } -void TcpListenerImpl::onSocketEvent(short flags) { +absl::Status TcpListenerImpl::onSocketEvent(short flags) { ASSERT(bind_to_port_); ASSERT(flags & (Event::FileReadyType::Read)); @@ -98,12 +98,15 @@ void TcpListenerImpl::onSocketEvent(short flags) { // effect if the socket is a v4 socket, but for v6 sockets this will create an IPv4 remote // address if an IPv4 local_address was created from an IPv6 mapped IPv4 address. - const Address::InstanceConstSharedPtr remote_address = - (remote_addr.ss_family == AF_UNIX) - ? io_handle->peerAddress() - : Address::addressFromSockAddrOrThrow(remote_addr, remote_addr_len, - local_address->ip()->version() == - Address::IpVersion::v6); + Address::InstanceConstSharedPtr remote_address; + if (remote_addr.ss_family == AF_UNIX) { + remote_address = io_handle->peerAddress(); + } else { + auto address_or_error = Address::addressFromSockAddr( + remote_addr, remote_addr_len, local_address->ip()->version() == Address::IpVersion::v6); + RETURN_IF_NOT_OK_REF(address_or_error.status()); + remote_address = *address_or_error; + } cb_.onAccept(std::make_unique(std::move(io_handle), local_address, remote_address, overload_state_, @@ -113,6 +116,7 @@ void TcpListenerImpl::onSocketEvent(short flags) { ENVOY_LOG_MISC(trace, "TcpListener accepted {} new connections.", connections_accepted_from_kernel_count); cb_.recordConnectionsAcceptedOnSocketEvent(connections_accepted_from_kernel_count); + return absl::OkStatus(); } TcpListenerImpl::TcpListenerImpl(Event::Dispatcher& dispatcher, Random::RandomGenerator& random, @@ -137,11 +141,7 @@ TcpListenerImpl::TcpListenerImpl(Event::Dispatcher& dispatcher, Random::RandomGe // transient accept errors or early termination due to accepting // max_connections_to_accept_per_socket_event connections. socket_->ioHandle().initializeFileEvent( - dispatcher, - [this](uint32_t events) { - onSocketEvent(events); - return absl::OkStatus(); - }, + dispatcher, [this](uint32_t events) { return onSocketEvent(events); }, Event::FileTriggerType::Level, Event::FileReadyType::Read); } } diff --git a/source/common/network/tcp_listener_impl.h b/source/common/network/tcp_listener_impl.h index 0c2b32bf93b9..88c1c35dac01 100644 --- a/source/common/network/tcp_listener_impl.h +++ b/source/common/network/tcp_listener_impl.h @@ -36,7 +36,7 @@ class TcpListenerImpl : public BaseListenerImpl { TcpListenerCallbacks& cb_; private: - void onSocketEvent(short flags); + absl::Status onSocketEvent(short flags); // Returns true if global connection limit has been reached and the accepted socket should be // rejected/closed. If the accepted socket is to be admitted, false is returned.