diff --git a/source/common/network/connection_impl.cc b/source/common/network/connection_impl.cc index f7869f639617..2326daeb054f 100644 --- a/source/common/network/connection_impl.cc +++ b/source/common/network/connection_impl.cc @@ -542,28 +542,32 @@ ClientConnectionImpl::ClientConnectionImpl( const Network::ConnectionSocket::OptionsSharedPtr& options) : ConnectionImpl(dispatcher, std::make_unique(remote_address), std::move(transport_socket), false) { - if (!Network::Socket::applyOptions(options, *socket_, - envoy::api::v2::core::SocketOption::STATE_PREBIND)) { - // Set a special error state to ensure asynchronous close to give the owner of the - // ConnectionImpl a chance to add callbacks and detect the "disconnect". - immediate_error_event_ = ConnectionEvent::LocalClose; - // Trigger a write event to close this connection out-of-band. - file_event_->activate(Event::FileReadyType::Write); - return; - } - - if (source_address != nullptr) { - const Api::SysCallIntResult result = source_address->bind(fd()); - if (result.rc_ < 0) { - ENVOY_LOG_MISC(debug, "Bind failure. Failed to bind to {}: {}", source_address->asString(), - strerror(result.errno_)); - bind_error_ = true; + // There are no meaningful socket options or source address semantics for + // non-IP sockets, so skip. + if (remote_address->ip() != nullptr) { + if (!Network::Socket::applyOptions(options, *socket_, + envoy::api::v2::core::SocketOption::STATE_PREBIND)) { // Set a special error state to ensure asynchronous close to give the owner of the // ConnectionImpl a chance to add callbacks and detect the "disconnect". immediate_error_event_ = ConnectionEvent::LocalClose; - // Trigger a write event to close this connection out-of-band. file_event_->activate(Event::FileReadyType::Write); + return; + } + + if (source_address != nullptr) { + const Api::SysCallIntResult result = source_address->bind(fd()); + if (result.rc_ < 0) { + ENVOY_LOG_MISC(debug, "Bind failure. Failed to bind to {}: {}", source_address->asString(), + strerror(result.errno_)); + bind_error_ = true; + // Set a special error state to ensure asynchronous close to give the owner of the + // ConnectionImpl a chance to add callbacks and detect the "disconnect". + immediate_error_event_ = ConnectionEvent::LocalClose; + + // Trigger a write event to close this connection out-of-band. + file_event_->activate(Event::FileReadyType::Write); + } } } } diff --git a/test/common/network/connection_impl_test.cc b/test/common/network/connection_impl_test.cc index 8ae698340f0d..49755f70a0b7 100644 --- a/test/common/network/connection_impl_test.cc +++ b/test/common/network/connection_impl_test.cc @@ -77,8 +77,8 @@ INSTANTIATE_TEST_CASE_P(IpVersions, ConnectionImplDeathTest, TestUtility::ipTestParamsToString); TEST_P(ConnectionImplDeathTest, BadFd) { - DangerousDeprecatedTestTime test_time; - Event::DispatcherImpl dispatcher(test_time.timeSource()); + MockTimeSource time_source; + Event::DispatcherImpl dispatcher(time_source); EXPECT_DEATH_LOG_TO_STDERR( ConnectionImpl(dispatcher, std::make_unique(-1, nullptr, nullptr), Network::Test::createRawBufferSocket(), false), @@ -89,7 +89,7 @@ class ConnectionImplTest : public testing::TestWithParam { public: void setUpBasicConnection() { if (dispatcher_.get() == nullptr) { - dispatcher_.reset(new Event::DispatcherImpl(test_time_.timeSource())); + dispatcher_.reset(new Event::DispatcherImpl(time_source_)); } listener_ = dispatcher_->createListener(socket_, listener_callbacks_, true, false); @@ -152,7 +152,7 @@ class ConnectionImplTest : public testing::TestWithParam { MockBufferFactory* factory = new StrictMock; dispatcher_.reset( - new Event::DispatcherImpl(test_time_.timeSource(), Buffer::WatermarkFactoryPtr{factory})); + new Event::DispatcherImpl(time_source_, Buffer::WatermarkFactoryPtr{factory})); // The first call to create a client session will get a MockBuffer. // Other calls for server sessions will by default get a normal OwnedImpl. EXPECT_CALL(*factory, create_(_, _)) @@ -169,7 +169,7 @@ class ConnectionImplTest : public testing::TestWithParam { } protected: - DangerousDeprecatedTestTime test_time_; + MockTimeSource time_source_; Event::DispatcherPtr dispatcher_; Stats::IsolatedStoreImpl stats_store_; Network::TcpListenSocket socket_{Network::Test::getAnyAddress(GetParam()), nullptr, true}; @@ -233,7 +233,7 @@ TEST_P(ConnectionImplTest, CloseDuringConnectCallback) { } TEST_P(ConnectionImplTest, ImmediateConnectError) { - dispatcher_.reset(new Event::DispatcherImpl(test_time_.timeSource())); + dispatcher_.reset(new Event::DispatcherImpl(time_source_)); // Using a broadcast/multicast address as the connection destiantion address causes an // immediate error return from connect(). @@ -805,7 +805,7 @@ TEST_P(ConnectionImplTest, BindFailureTest) { source_address_ = Network::Address::InstanceConstSharedPtr{ new Network::Address::Ipv6Instance(address_string, 0)}; } - dispatcher_.reset(new Event::DispatcherImpl(test_time_.timeSource())); + dispatcher_.reset(new Event::DispatcherImpl(time_source_)); listener_ = dispatcher_->createListener(socket_, listener_callbacks_, true, false); client_connection_ = dispatcher_->createClientConnection( @@ -1199,7 +1199,7 @@ class ReadBufferLimitTest : public ConnectionImplTest { public: void readBufferLimitTest(uint32_t read_buffer_limit, uint32_t expected_chunk_size) { const uint32_t buffer_size = 256 * 1024; - dispatcher_.reset(new Event::DispatcherImpl(test_time_.timeSource())); + dispatcher_.reset(new Event::DispatcherImpl(time_source_)); listener_ = dispatcher_->createListener(socket_, listener_callbacks_, true, false); client_connection_ = dispatcher_->createClientConnection( @@ -1270,8 +1270,8 @@ TEST_P(ReadBufferLimitTest, SomeLimit) { class TcpClientConnectionImplTest : public testing::TestWithParam { protected: - TcpClientConnectionImplTest() : dispatcher_(test_time_.timeSource()) {} - DangerousDeprecatedTestTime test_time_; + TcpClientConnectionImplTest() : dispatcher_(time_source_) {} + MockTimeSource time_source_; Event::DispatcherImpl dispatcher_; }; INSTANTIATE_TEST_CASE_P(IpVersions, TcpClientConnectionImplTest, @@ -1309,5 +1309,33 @@ TEST_P(TcpClientConnectionImplTest, BadConnectConnRefused) { dispatcher_.run(Event::Dispatcher::RunType::Block); } +class PipeClientConnectionImplTest : public testing::Test { +protected: + PipeClientConnectionImplTest() : dispatcher_(time_source_) {} + MockTimeSource time_source_; + Event::DispatcherImpl dispatcher_; + const std::string path_{TestEnvironment::unixDomainSocketPath("foo")}; +}; + +// Validate we skip setting socket options on UDS. +TEST_F(PipeClientConnectionImplTest, SkipSocketOptions) { + auto option = std::make_shared(); + EXPECT_CALL(*option, setOption(_, _)).Times(0); + auto options = std::make_shared(); + options->emplace_back(option); + ClientConnectionPtr connection = dispatcher_.createClientConnection( + Utility::resolveUrl("unix://" + path_), Network::Address::InstanceConstSharedPtr(), + Network::Test::createRawBufferSocket(), options); + connection->close(ConnectionCloseType::NoFlush); +} + +// Validate we skip setting source address. +TEST_F(PipeClientConnectionImplTest, SkipSourceAddress) { + ClientConnectionPtr connection = dispatcher_.createClientConnection( + Utility::resolveUrl("unix://" + path_), Utility::resolveUrl("tcp://1.2.3.4:5"), + Network::Test::createRawBufferSocket(), nullptr); + connection->close(ConnectionCloseType::NoFlush); +} + } // namespace Network } // namespace Envoy diff --git a/test/mocks/common.cc b/test/mocks/common.cc index afc95170c38b..5126b0b1f2b7 100644 --- a/test/mocks/common.cc +++ b/test/mocks/common.cc @@ -10,6 +10,9 @@ MockSystemTimeSource::~MockSystemTimeSource() {} MockMonotonicTimeSource::MockMonotonicTimeSource() {} MockMonotonicTimeSource::~MockMonotonicTimeSource() {} +MockTimeSource::MockTimeSource() : TimeSource(system_, monotonic_) {} +MockTimeSource::~MockTimeSource() {} + MockTokenBucket::MockTokenBucket() {} MockTokenBucket::~MockTokenBucket() {} diff --git a/test/mocks/common.h b/test/mocks/common.h index 70f6bb6f569d..adf25120f065 100644 --- a/test/mocks/common.h +++ b/test/mocks/common.h @@ -51,6 +51,15 @@ class MockMonotonicTimeSource : public MonotonicTimeSource { MOCK_METHOD0(currentTime, MonotonicTime()); }; +class MockTimeSource : public TimeSource { +public: + MockTimeSource(); + ~MockTimeSource(); + + MockSystemTimeSource system_; + MockMonotonicTimeSource monotonic_; +}; + class MockTokenBucket : public TokenBucket { public: MockTokenBucket(); diff --git a/test/server/server_corpus/clusterfuzz-testcase-minimized-server_fuzz_test-5632902623657984 b/test/server/server_corpus/clusterfuzz-testcase-minimized-server_fuzz_test-5632902623657984 new file mode 100644 index 000000000000..6c7837d4677e --- /dev/null +++ b/test/server/server_corpus/clusterfuzz-testcase-minimized-server_fuzz_test-5632902623657984 @@ -0,0 +1 @@ +static_resources { clusters { name: " " connect_timeout { nanos: 6 } hosts { pipe { path: " " } } health_checks { timeout { nanos: 6 } interval { nanos: 6 } unhealthy_threshold { } healthy_threshold { } tcp_health_check { } } } } cluster_manager { upstream_bind_config { source_address { address: "0.0.0.0" port_value: 0 } freebind { value: true } } } admin { address { pipe { path: " " } } }