diff --git a/source/common/network/connection_impl.cc b/source/common/network/connection_impl.cc index 1a8d74bab9290..004f3f2c2b1d6 100644 --- a/source/common/network/connection_impl.cc +++ b/source/common/network/connection_impl.cc @@ -275,6 +275,15 @@ void ConnectionImpl::raiseEvent(ConnectionEvent event) { // connected events. callback->onEvent(event); } + // We may have pending data in the write buffer on transport handshake + // completion, which may also have completed in the context of onReadReady(), + // where no check of the write buffer is made. Provide an opportunity to flush + // here. If connection write is not ready, this is harmless. We should only do + // this if we're still open (the above callbacks may have closed). + if (state() == State::Open && event == ConnectionEvent::Connected && + write_buffer_->length() > 0) { + onWriteReady(); + } } bool ConnectionImpl::readEnabled() const { return read_enabled_; } diff --git a/test/common/network/connection_impl_test.cc b/test/common/network/connection_impl_test.cc index 9dd61ae496808..1fdea7b15c795 100644 --- a/test/common/network/connection_impl_test.cc +++ b/test/common/network/connection_impl_test.cc @@ -842,6 +842,10 @@ class MockTransportConnectionImplTest : public testing::Test { EXPECT_CALL(dispatcher_, createFileEvent_(0, _, _, _)) .WillOnce(DoAll(SaveArg<1>(&file_ready_cb_), Return(file_event_))); transport_socket_ = new NiceMock; + EXPECT_CALL(*transport_socket_, setTransportSocketCallbacks(_)) + .WillOnce(Invoke([this](TransportSocketCallbacks& callbacks) { + transport_socket_callbacks_ = &callbacks; + })); connection_.reset( new ConnectionImpl(dispatcher_, std::make_unique(0, nullptr, nullptr), TransportSocketPtr(transport_socket_), true)); @@ -861,9 +865,10 @@ class MockTransportConnectionImplTest : public testing::Test { std::unique_ptr connection_; Event::MockDispatcher dispatcher_; NiceMock callbacks_; - NiceMock* transport_socket_; + MockTransportSocket* transport_socket_; Event::MockFileEvent* file_event_; Event::FileReadyCb file_ready_cb_; + TransportSocketCallbacks* transport_socket_callbacks_; }; // Test that BytesSentCb is invoked at the correct times @@ -1122,6 +1127,33 @@ TEST_F(MockTransportConnectionImplTest, WriteEndStreamStopIteration) { connection_->write(buffer, true); } +// Validate that when the transport signals ConnectionEvent::Connected, that we +// check for pending write buffer content. +TEST_F(MockTransportConnectionImplTest, WriteReadyOnConnected) { + InSequence s; + + // Queue up some data in write buffer, simulating what happens in SSL handshake. + const std::string val("some data"); + Buffer::OwnedImpl buffer(val); + EXPECT_CALL(*file_event_, activate(Event::FileReadyType::Write)).WillOnce(Invoke(file_ready_cb_)); + EXPECT_CALL(*transport_socket_, doWrite(BufferStringEqual(val), false)) + .WillOnce(Return(IoResult{PostIoAction::KeepOpen, 0, false})); + connection_->write(buffer, false); + + // A read event happens, resulting in handshake completion and + // raiseEvent(Network::ConnectionEvent::Connected). Since we have data queued + // in the write buffer, we should see a doWrite with this data. + EXPECT_CALL(*transport_socket_, doRead(_)).WillOnce(InvokeWithoutArgs([this] { + transport_socket_callbacks_->raiseEvent(Network::ConnectionEvent::Connected); + return IoResult{PostIoAction::KeepOpen, 0, false}; + })); + EXPECT_CALL(*transport_socket_, doWrite(BufferStringEqual(val), false)) + .WillOnce(Return(IoResult{PostIoAction::KeepOpen, 0, false})); + file_ready_cb_(Event::FileReadyType::Read); + EXPECT_CALL(*transport_socket_, doWrite(_, true)) + .WillOnce(Return(IoResult{PostIoAction::KeepOpen, 0, true})); +} + class ReadBufferLimitTest : public ConnectionImplTest { public: void readBufferLimitTest(uint32_t read_buffer_limit, uint32_t expected_chunk_size) {