diff --git a/source/common/network/address_impl.cc b/source/common/network/address_impl.cc index 9592c194bbbdc..74fc2d6e8d8cb 100644 --- a/source/common/network/address_impl.cc +++ b/source/common/network/address_impl.cc @@ -8,6 +8,32 @@ namespace Network { namespace Address { +InstanceConstSharedPtr addressFromFd(int fd) { + sockaddr_storage ss; + socklen_t ss_len = sizeof ss; + const int rc = ::getsockname(fd, reinterpret_cast(&ss), &ss_len); + if (rc != 0) { + throw EnvoyException(fmt::format("getsockname failed for '{}': {}", fd, strerror(errno))); + } + switch (ss.ss_family) { + case AF_INET: { + ASSERT(ss_len == sizeof(sockaddr_in)); + const struct sockaddr_in* sin = reinterpret_cast(&ss); + ASSERT(AF_INET == sin->sin_family); + return InstanceConstSharedPtr(new Address::Ipv4Instance(sin)); + } + case AF_INET6: { + ASSERT(ss_len == sizeof(sockaddr_in6)); + const struct sockaddr_in6* sin6 = reinterpret_cast(&ss); + ASSERT(AF_INET6 == sin6->sin6_family); + return InstanceConstSharedPtr(new Address::Ipv6Instance(*sin6)); + } + default: + throw EnvoyException(fmt::format("Unexpected family in getsockname result: {}", ss.ss_family)); + } + NOT_REACHED; +} + int InstanceBase::flagsFromSocketType(SocketType type) const { int flags = SOCK_NONBLOCK; if (type == SocketType::Stream) { diff --git a/source/common/network/address_impl.h b/source/common/network/address_impl.h index 1aaf12426ad0c..ac459d23516c6 100644 --- a/source/common/network/address_impl.h +++ b/source/common/network/address_impl.h @@ -7,6 +7,13 @@ namespace Network { namespace Address { +/** + * Obtain an address from a bound file descriptor. Raises an EnvoyException on failure. + * @param fd file descriptor. + * @return InstanceConstSharedPtr for bound address. + */ +InstanceConstSharedPtr addressFromFd(int fd); + /** * Base class for all address types. */ diff --git a/source/common/network/listen_socket_impl.cc b/source/common/network/listen_socket_impl.cc index 32dd351349ad9..8055c1e4a0e7c 100644 --- a/source/common/network/listen_socket_impl.cc +++ b/source/common/network/listen_socket_impl.cc @@ -15,6 +15,11 @@ void ListenSocketImpl::doBind() { throw EnvoyException( fmt::format("cannot bind '{}': {}", local_address_->asString(), strerror(errno))); } + if (local_address_->type() == Address::Type::Ip && local_address_->ip()->port() == 0) { + // If the port we bind is zero, then the OS will pick a free port for us (assuming there are + // any), and we need to find out the port number that the OS picked. + local_address_ = Address::addressFromFd(fd_); + } } // TODO(wattli): remove this once the admin port is updated with address. diff --git a/test/common/network/listen_socket_impl_test.cc b/test/common/network/listen_socket_impl_test.cc index b053406facfa7..9d0d601b22d4f 100644 --- a/test/common/network/listen_socket_impl_test.cc +++ b/test/common/network/listen_socket_impl_test.cc @@ -32,4 +32,13 @@ TEST(ListenSocket, All) { EXPECT_EQ("127.0.0.1:15004", socket6.localAddress()->asString()); } +// Validate we get port allocation when binding to port zero. +TEST(ListenSocket, BindPortZero) { + TcpListenSocket socket(Utility::resolveUrl("tcp://127.0.0.1:0"), true); + EXPECT_EQ(Address::Type::Ip, socket.localAddress()->type()); + EXPECT_EQ("127.0.0.1", socket.localAddress()->ip()->addressAsString()); + EXPECT_GT(socket.localAddress()->ip()->port(), 0U); + EXPECT_EQ(Address::IpVersion::v4, socket.localAddress()->ip()->version()); +} + } // Network diff --git a/test/test_common/network_utility.cc b/test/test_common/network_utility.cc index 839a5691d80c8..db8e2c73722f3 100644 --- a/test/test_common/network_utility.cc +++ b/test/test_common/network_utility.cc @@ -50,37 +50,10 @@ Address::InstanceConstSharedPtr findOrCheckFreePort(Address::InstanceConstShared << "' with error: " << strerror(err) << " (" << err << ")"; return nullptr; } + // If the port we bind is zero, then the OS will pick a free port for us (assuming there are + // any), and we need to find out the port number that the OS picked so we can return it. if (addr_port->ip()->port() == 0) { - // If the port we bind is zero, then the OS will pick a free port for us (assuming there are - // any), and we need to find out the port number that the OS picked so we can return it. - sockaddr_storage ss; - socklen_t ss_len = sizeof ss; - rc = ::getsockname(fd, reinterpret_cast(&ss), &ss_len); - if (rc != 0) { - const int err = errno; - ADD_FAILURE() << "getsockname failed for '" << addr_port->asString() - << "' with error: " << strerror(err) << " (" << err << ")"; - return nullptr; - } - switch (ss.ss_family) { - case AF_INET: { - EXPECT_EQ(ss_len, sizeof(sockaddr_in)); - const struct sockaddr_in* sin = reinterpret_cast(&ss); - EXPECT_EQ(AF_INET, sin->sin_family); - addr_port.reset(new Address::Ipv4Instance(sin)); - break; - } - case AF_INET6: { - EXPECT_EQ(ss_len, sizeof(sockaddr_in6)); - const struct sockaddr_in6* sin6 = reinterpret_cast(&ss); - EXPECT_EQ(AF_INET6, sin6->sin6_family); - addr_port.reset(new Address::Ipv6Instance(*sin6)); - break; - } - default: - ADD_FAILURE() << "Unexpected family in getsockname result: " << ss.ss_family; - return nullptr; - } + return Address::addressFromFd(fd); } return addr_port; }