Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

socket: removing some exceptions #36991

Merged
merged 1 commit into from
Nov 8, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions contrib/vcl/source/vcl_io_handle.cc
Original file line number Diff line number Diff line change
Expand Up @@ -625,7 +625,7 @@ absl::optional<int> VclIoHandle::domain() {
return {AF_INET};
};

Envoy::Network::Address::InstanceConstSharedPtr VclIoHandle::localAddress() {
absl::StatusOr<Envoy::Network::Address::InstanceConstSharedPtr> VclIoHandle::localAddress() {
vppcom_endpt_t ep;
uint32_t eplen = sizeof(ep);
uint8_t addr_buf[sizeof(struct sockaddr_in6)];
Expand All @@ -636,7 +636,7 @@ Envoy::Network::Address::InstanceConstSharedPtr VclIoHandle::localAddress() {
return vclEndptToAddress(ep, sh_);
}

Envoy::Network::Address::InstanceConstSharedPtr VclIoHandle::peerAddress() {
absl::StatusOr<Envoy::Network::Address::InstanceConstSharedPtr> VclIoHandle::peerAddress() {
VCL_LOG("grabbing peer address sh {:x}", sh_);
vppcom_endpt_t ep;
uint32_t eplen = sizeof(ep);
Expand Down
4 changes: 2 additions & 2 deletions contrib/vcl/source/vcl_io_handle.h
Original file line number Diff line number Diff line change
Expand Up @@ -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<int> domain() override;
Envoy::Network::Address::InstanceConstSharedPtr localAddress() override;
Envoy::Network::Address::InstanceConstSharedPtr peerAddress() override;
absl::StatusOr<Envoy::Network::Address::InstanceConstSharedPtr> localAddress() override;
absl::StatusOr<Envoy::Network::Address::InstanceConstSharedPtr> peerAddress() override;
Api::SysCallIntResult shutdown(int) override { return {0, 0}; }

void initializeFileEvent(Event::Dispatcher& dispatcher, Event::FileReadyCb cb,
Expand Down
8 changes: 4 additions & 4 deletions envoy/network/io_handle.h
Original file line number Diff line number Diff line change
Expand Up @@ -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<Address::InstanceConstSharedPtr> 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<Address::InstanceConstSharedPtr> peerAddress() PURE;

/**
* Duplicates the handle. This is intended to be used only on listener sockets. (see man dup)
Expand Down
22 changes: 12 additions & 10 deletions source/common/network/io_socket_handle_base_impl.cc
Original file line number Diff line number Diff line change
Expand Up @@ -63,30 +63,29 @@ Api::SysCallIntResult IoSocketHandleBaseImpl::setBlocking(bool blocking) {

absl::optional<int> IoSocketHandleBaseImpl::domain() { return domain_; }

Address::InstanceConstSharedPtr IoSocketHandleBaseImpl::localAddress() {
absl::StatusOr<Address::InstanceConstSharedPtr> IoSocketHandleBaseImpl::localAddress() {
sockaddr_storage ss;
socklen_t ss_len = sizeof(ss);
memset(&ss, 0, ss_len);
auto& os_sys_calls = Api::OsSysCallsSingleton::get();
Api::SysCallIntResult result =
os_sys_calls.getsockname(fd_, reinterpret_cast<sockaddr*>(&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<Address::InstanceConstSharedPtr> IoSocketHandleBaseImpl::peerAddress() {
sockaddr_storage ss;
socklen_t ss_len = sizeof(ss);
memset(&ss, 0, ss_len);
auto& os_sys_calls = Api::OsSysCallsSingleton::get();
Api::SysCallIntResult result =
os_sys_calls.getpeername(fd_, reinterpret_cast<sockaddr*>(&ss), &ss_len);
if (result.return_value_ != 0) {
throwEnvoyExceptionOrPanic(
return absl::InvalidArgumentError(
fmt::format("getpeername failed for '{}': {}", fd_, errorDetails(result.errno_)));
}

Expand All @@ -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<std::chrono::milliseconds> IoSocketHandleBaseImpl::lastRoundTripTime() {
Expand All @@ -131,7 +129,11 @@ absl::optional<std::string> 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;
}
Expand Down
4 changes: 2 additions & 2 deletions source/common/network/io_socket_handle_base_impl.h
Original file line number Diff line number Diff line change
Expand Up @@ -29,8 +29,8 @@ class IoSocketHandleBaseImpl : public IoHandle, protected Logger::Loggable<Logge
unsigned long*) override;
Api::SysCallIntResult setBlocking(bool blocking) override;
absl::optional<int> domain() override;
Address::InstanceConstSharedPtr localAddress() override;
Address::InstanceConstSharedPtr peerAddress() override;
absl::StatusOr<Address::InstanceConstSharedPtr> localAddress() override;
absl::StatusOr<Address::InstanceConstSharedPtr> peerAddress() override;
absl::optional<std::chrono::milliseconds> lastRoundTripTime() override;
absl::optional<uint64_t> congestionWindowInBytes() const override;
absl::optional<std::string> interfaceName() override;
Expand Down
12 changes: 10 additions & 2 deletions source/common/network/socket_impl.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
Expand All @@ -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;
}
Expand Down
12 changes: 9 additions & 3 deletions source/common/network/tcp_listener_impl.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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);
Expand Down
5 changes: 4 additions & 1 deletion source/common/network/utility.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
4 changes: 2 additions & 2 deletions source/common/quic/quic_io_handle_wrapper.h
Original file line number Diff line number Diff line change
Expand Up @@ -117,10 +117,10 @@ class QuicIoHandleWrapper : public Network::IoHandle {
return io_handle_.setBlocking(blocking);
}
absl::optional<int> domain() override { return io_handle_.domain(); }
Network::Address::InstanceConstSharedPtr localAddress() override {
absl::StatusOr<Network::Address::InstanceConstSharedPtr> localAddress() override {
return io_handle_.localAddress();
}
Network::Address::InstanceConstSharedPtr peerAddress() override {
absl::StatusOr<Network::Address::InstanceConstSharedPtr> peerAddress() override {
return io_handle_.peerAddress();
}

Expand Down
4 changes: 2 additions & 2 deletions source/extensions/io_socket/user_space/io_handle_impl.cc
Original file line number Diff line number Diff line change
Expand Up @@ -315,11 +315,11 @@ Api::SysCallIntResult IoHandleImpl::setBlocking(bool) { return makeInvalidSyscal

absl::optional<int> IoHandleImpl::domain() { return absl::nullopt; }

Network::Address::InstanceConstSharedPtr IoHandleImpl::localAddress() {
absl::StatusOr<Network::Address::InstanceConstSharedPtr> IoHandleImpl::localAddress() {
return IoHandleImpl::getCommonInternalAddress();
}

Network::Address::InstanceConstSharedPtr IoHandleImpl::peerAddress() {
absl::StatusOr<Network::Address::InstanceConstSharedPtr> IoHandleImpl::peerAddress() {
return IoHandleImpl::getCommonInternalAddress();
}

Expand Down
4 changes: 2 additions & 2 deletions source/extensions/io_socket/user_space/io_handle_impl.h
Original file line number Diff line number Diff line change
Expand Up @@ -80,8 +80,8 @@ class IoHandleImpl final : public Network::IoHandle,
unsigned long*) override;
Api::SysCallIntResult setBlocking(bool blocking) override;
absl::optional<int> domain() override;
Network::Address::InstanceConstSharedPtr localAddress() override;
Network::Address::InstanceConstSharedPtr peerAddress() override;
absl::StatusOr<Network::Address::InstanceConstSharedPtr> localAddress() override;
absl::StatusOr<Network::Address::InstanceConstSharedPtr> peerAddress() override;

void initializeFileEvent(Event::Dispatcher& dispatcher, Event::FileReadyCb cb,
Event::FileTriggerType trigger, uint32_t events) override;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}
Expand Down Expand Up @@ -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);
}
Expand Down
4 changes: 2 additions & 2 deletions test/common/quic/quic_io_handle_wrapper_test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -72,15 +72,15 @@ 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 {
addr->sa_family = AF_INET6;
*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))
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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_;
Expand Down
4 changes: 2 additions & 2 deletions test/extensions/io_socket/user_space/io_handle_impl_test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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());
Expand Down
2 changes: 1 addition & 1 deletion test/integration/filters/test_socket_interface.h
Original file line number Diff line number Diff line change
Expand Up @@ -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<Address::InstanceConstSharedPtr> peerAddress() override {
if (peer_address_override_.has_value()) {
return Network::Utility::getAddressWithPort(
peer_address_override_.value().get(), peer_address_override_.value().get().ip()->port());
Expand Down
10 changes: 5 additions & 5 deletions test/integration/socket_interface_swap.h
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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_;
}
Expand All @@ -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();
}

Expand Down
4 changes: 2 additions & 2 deletions test/mocks/network/io_handle.h
Original file line number Diff line number Diff line change
Expand Up @@ -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<int>, domain, ());
MOCK_METHOD(Address::InstanceConstSharedPtr, localAddress, ());
MOCK_METHOD(Address::InstanceConstSharedPtr, peerAddress, ());
MOCK_METHOD(absl::StatusOr<Address::InstanceConstSharedPtr>, localAddress, ());
MOCK_METHOD(absl::StatusOr<Address::InstanceConstSharedPtr>, peerAddress, ());
MOCK_METHOD(IoHandlePtr, duplicate, ());
MOCK_METHOD(void, createFileEvent_,
(Event::Dispatcher & dispatcher, Event::FileReadyCb cb,
Expand Down
Loading