diff --git a/envoy/api/io_error.h b/envoy/api/io_error.h index f5de759194d1..049d14053b24 100644 --- a/envoy/api/io_error.h +++ b/envoy/api/io_error.h @@ -35,6 +35,8 @@ class IoError { BadFd, // An existing connection was forcibly closed by the remote host. ConnectionReset, + // Network is unreachable due to network settings. + NetworkUnreachable, // Other error codes cannot be mapped to any one above in getErrorCode(). UnknownError }; diff --git a/envoy/common/platform.h b/envoy/common/platform.h index e610caccb7ee..96ed53f8562a 100644 --- a/envoy/common/platform.h +++ b/envoy/common/platform.h @@ -150,6 +150,7 @@ struct msghdr { #define SOCKET_ERROR_ADDR_IN_USE WSAEADDRINUSE #define SOCKET_ERROR_BADF WSAEBADF #define SOCKET_ERROR_CONNRESET WSAECONNRESET +#define SOCKET_ERROR_NETUNREACH WSAENETUNREACH #define HANDLE_ERROR_PERM ERROR_ACCESS_DENIED #define HANDLE_ERROR_INVALID ERROR_INVALID_HANDLE @@ -259,6 +260,7 @@ typedef int signal_t; // NOLINT(modernize-use-using) #define SOCKET_ERROR_ADDR_IN_USE EADDRINUSE #define SOCKET_ERROR_BADF EBADF #define SOCKET_ERROR_CONNRESET ECONNRESET +#define SOCKET_ERROR_NETUNREACH ENETUNREACH // Mapping POSIX file errors to common error names #define HANDLE_ERROR_PERM EACCES diff --git a/source/common/http/codec_client.cc b/source/common/http/codec_client.cc index 29aa601384a4..e2d242f54326 100644 --- a/source/common/http/codec_client.cc +++ b/source/common/http/codec_client.cc @@ -117,6 +117,9 @@ void CodecClient::onEvent(Network::ConnectionEvent event) { StreamInfo::ResponseFlag::UpstreamProtocolError); } } + } else { + ENVOY_CONN_LOG(warn, "Connection is closed by {} during connecting.", *connection_, + (event == Network::ConnectionEvent::RemoteClose ? "peer" : "self")); } while (!active_requests_.empty()) { // Fake resetting all active streams so that reset() callbacks get invoked. diff --git a/source/common/http/http3/conn_pool.cc b/source/common/http/http3/conn_pool.cc index 49ed2e1a7b3c..9bba3fa0221c 100644 --- a/source/common/http/http3/conn_pool.cc +++ b/source/common/http/http3/conn_pool.cc @@ -65,10 +65,15 @@ allocateConnPool(Event::Dispatcher& dispatcher, Random::RandomGenerator& random_ host, priority, dispatcher, options, transport_socket_options, random_generator, state, [&quic_stat_names, &scope](HttpConnPoolImplBase* pool) -> ::Envoy::ConnectionPool::ActiveClientPtr { + ENVOY_LOG_TO_LOGGER(Envoy::Logger::Registry::getLog(Envoy::Logger::Id::pool), debug, + "Creating Http/3 client"); // If there's no ssl context, the secrets are not loaded. Fast-fail by returning null. auto factory = &pool->host()->transportSocketFactory(); ASSERT(dynamic_cast(factory) != nullptr); if (static_cast(factory)->sslCtx() == nullptr) { + ENVOY_LOG_TO_LOGGER(Envoy::Logger::Registry::getLog(Envoy::Logger::Id::pool), warn, + "Failed to create Http/3 client. Transport socket " + "factory is not configured correctly."); return nullptr; } Http3ConnPoolImpl* h3_pool = reinterpret_cast(pool); @@ -82,7 +87,13 @@ allocateConnPool(Event::Dispatcher& dispatcher, Random::RandomGenerator& random_ data.connection_ = Quic::createQuicNetworkConnection(h3_pool->quicInfo(), pool->dispatcher(), host_address, source_address, quic_stat_names, scope); - return std::make_unique(*pool, data); + // Store a handle to connection as it will be moved during client construction. + Network::Connection& connection = *data.connection_; + auto client = std::make_unique(*pool, data); + if (connection.state() == Network::Connection::State::Closed) { + return nullptr; + } + return client; }, [](Upstream::Host::CreateConnectionData& data, HttpConnPoolImplBase* pool) { CodecClientPtr codec{new CodecClientProd(CodecType::HTTP3, std::move(data.connection_), diff --git a/source/common/network/io_socket_error_impl.cc b/source/common/network/io_socket_error_impl.cc index 8d664be8e1f4..a3e955f4d68a 100644 --- a/source/common/network/io_socket_error_impl.cc +++ b/source/common/network/io_socket_error_impl.cc @@ -58,6 +58,8 @@ Api::IoError::IoErrorCode IoSocketError::errorCodeFromErrno(int sys_errno) { return IoErrorCode::BadFd; case SOCKET_ERROR_CONNRESET: return IoErrorCode::ConnectionReset; + case SOCKET_ERROR_NETUNREACH: + return IoErrorCode::NetworkUnreachable; default: ENVOY_LOG_MISC(debug, "Unknown error code {} details {}", sys_errno, errorDetails(sys_errno)); return IoErrorCode::UnknownError; diff --git a/source/common/quic/envoy_quic_packet_writer.cc b/source/common/quic/envoy_quic_packet_writer.cc index 6a3d358bae01..e2f53bd2df5b 100644 --- a/source/common/quic/envoy_quic_packet_writer.cc +++ b/source/common/quic/envoy_quic_packet_writer.cc @@ -16,7 +16,7 @@ quic::WriteResult convertToQuicWriteResult(Api::IoCallUint64Result& result) { quic::WriteStatus status = result.err_->getErrorCode() == Api::IoError::IoErrorCode::Again ? quic::WRITE_STATUS_BLOCKED : quic::WRITE_STATUS_ERROR; - return {status, static_cast(result.err_->getErrorCode())}; + return {status, static_cast(result.err_->getSystemErrorCode())}; } } // namespace diff --git a/source/common/quic/quic_filter_manager_connection_impl.h b/source/common/quic/quic_filter_manager_connection_impl.h index 6fabaa5ded3c..f2e112297f82 100644 --- a/source/common/quic/quic_filter_manager_connection_impl.h +++ b/source/common/quic/quic_filter_manager_connection_impl.h @@ -82,6 +82,10 @@ class QuicFilterManagerConnectionImpl : public Network::ConnectionImplBase, void setConnectionStats(const Network::Connection::ConnectionStats& stats) override { // TODO(danzh): populate stats. Network::ConnectionImplBase::setConnectionStats(stats); + if (network_connection_ == nullptr) { + ENVOY_CONN_LOG(error, "Quic connection has been detached.", *this); + return; + } network_connection_->setConnectionStats(stats); } Ssl::ConnectionInfoConstSharedPtr ssl() const override; diff --git a/test/common/http/BUILD b/test/common/http/BUILD index edd7e75664ed..93b3cc90f28b 100644 --- a/test/common/http/BUILD +++ b/test/common/http/BUILD @@ -446,6 +446,7 @@ envoy_cc_test( "//test/mocks/runtime:runtime_mocks", "//test/mocks/stats:stats_mocks", "//test/mocks/server:transport_socket_factory_context_mocks", + "//test/test_common:threadsafe_singleton_injector_lib", "//source/common/quic:quic_factory_lib", "//source/common/quic:quic_transport_socket_factory_lib", "//source/common/quic:client_connection_factory_lib", diff --git a/test/common/http/conn_pool_grid_test.cc b/test/common/http/conn_pool_grid_test.cc index 873a813be450..0e03bc9cda65 100644 --- a/test/common/http/conn_pool_grid_test.cc +++ b/test/common/http/conn_pool_grid_test.cc @@ -14,6 +14,7 @@ #include "test/mocks/ssl/mocks.h" #include "test/mocks/upstream/cluster_info.h" #include "test/test_common/simulated_time_system.h" +#include "test/test_common/threadsafe_singleton_injector.h" #include "test/test_common/utility.h" #include "gmock/gmock.h" @@ -680,6 +681,55 @@ TEST_F(ConnectivityGridTest, RealGrid) { auto optional_it3 = ConnectivityGridForTest::forceCreateNextPool(grid); ASSERT_FALSE(optional_it3.has_value()); } + +TEST_F(ConnectivityGridTest, ConnectionCloseDuringCreation) { + EXPECT_CALL(*cluster_, connectTimeout()).WillRepeatedly(Return(std::chrono::seconds(10))); + + testing::InSequence s; + dispatcher_.allow_null_callback_ = true; + // Set the cluster up to have a quic transport socket. + Envoy::Ssl::ClientContextConfigPtr config(new NiceMock()); + NiceMock factory_context; + Ssl::ClientContextSharedPtr ssl_context(new Ssl::MockClientContext()); + EXPECT_CALL(factory_context.context_manager_, createSslClientContext(_, _, _)) + .WillOnce(Return(ssl_context)); + auto factory = + std::make_unique(std::move(config), factory_context); + factory->initialize(); + ASSERT_FALSE(factory->usesProxyProtocolOptions()); + auto& matcher = + static_cast(*cluster_->transport_socket_matcher_); + EXPECT_CALL(matcher, resolve(_)) + .WillRepeatedly( + Return(Upstream::TransportSocketMatcher::MatchData(*factory, matcher.stats_, "test"))); + + ConnectivityGrid grid(dispatcher_, random_, + Upstream::makeTestHost(cluster_, "tcp://127.0.0.1:9000", simTime()), + Upstream::ResourcePriority::Default, socket_options_, + transport_socket_options_, state_, simTime(), alternate_protocols_, + std::chrono::milliseconds(300), options_, quic_stat_names_, store_); + + // Create the HTTP/3 pool. + auto optional_it1 = ConnectivityGridForTest::forceCreateNextPool(grid); + ASSERT_TRUE(optional_it1.has_value()); + EXPECT_EQ("HTTP/3", (**optional_it1)->protocolDescription()); + + Api::MockOsSysCalls os_sys_calls; + TestThreadsafeSingletonInjector os_calls(&os_sys_calls); + EXPECT_CALL(os_sys_calls, socket(_, _, _)).WillOnce(Return(Api::SysCallSocketResult{1, 0})); +#if defined(__APPLE__) || defined(WIN32) + EXPECT_CALL(os_sys_calls, setsocketblocking(1, false)) + .WillOnce(Return(Api::SysCallIntResult{1, 0})); +#endif + EXPECT_CALL(os_sys_calls, bind(_, _, _)).WillOnce(Return(Api::SysCallIntResult{1, 0})); + EXPECT_CALL(os_sys_calls, setsockopt_(_, _, _, _, _)).WillRepeatedly(Return(0)); + EXPECT_CALL(os_sys_calls, sendmsg(_, _, _)).WillOnce(Return(Api::SysCallSizeResult{-1, 101})); + + EXPECT_CALL(os_sys_calls, close(1)).WillOnce(Return(Api::SysCallIntResult{0, 0})); + ConnectionPool::Cancellable* cancel = (**optional_it1)->newStream(decoder_, callbacks_); + EXPECT_EQ(nullptr, cancel); +} + #endif } // namespace diff --git a/test/common/quic/envoy_quic_writer_test.cc b/test/common/quic/envoy_quic_writer_test.cc index 3908fb82ba56..f6a21ffce892 100644 --- a/test/common/quic/envoy_quic_writer_test.cc +++ b/test/common/quic/envoy_quic_writer_test.cc @@ -90,7 +90,7 @@ TEST_F(EnvoyQuicWriterTest, SendBlocked) { quic::WriteResult result = envoy_quic_writer_.WritePacket(str.data(), str.length(), self_address_, peer_address_, nullptr); EXPECT_EQ(quic::WRITE_STATUS_BLOCKED, result.status); - EXPECT_EQ(static_cast(Api::IoError::IoErrorCode::Again), result.error_code); + EXPECT_EQ(SOCKET_ERROR_AGAIN, result.error_code); EXPECT_TRUE(envoy_quic_writer_.IsWriteBlocked()); // Writing while blocked is not allowed. #ifdef NDEBUG @@ -117,7 +117,7 @@ TEST_F(EnvoyQuicWriterTest, SendFailure) { quic::WriteResult result = envoy_quic_writer_.WritePacket(str.data(), str.length(), self_address_, peer_address_, nullptr); EXPECT_EQ(quic::WRITE_STATUS_ERROR, result.status); - EXPECT_EQ(static_cast(Api::IoError::IoErrorCode::NoSupport), result.error_code); + EXPECT_EQ(SOCKET_ERROR_NOT_SUP, result.error_code); EXPECT_FALSE(envoy_quic_writer_.IsWriteBlocked()); } @@ -133,7 +133,7 @@ TEST_F(EnvoyQuicWriterTest, SendFailureMessageTooBig) { // Currently MessageSize should be propagated through error_code. This test // would fail if QUICHE changes to propagate through status in the future. EXPECT_EQ(quic::WRITE_STATUS_ERROR, result.status); - EXPECT_EQ(static_cast(Api::IoError::IoErrorCode::MessageTooBig), result.error_code); + EXPECT_EQ(SOCKET_ERROR_MSG_SIZE, result.error_code); EXPECT_FALSE(envoy_quic_writer_.IsWriteBlocked()); }