From c275f94e58499904efb3b4b9c37f8f328004c581 Mon Sep 17 00:00:00 2001 From: Dan Zhang Date: Thu, 29 Jul 2021 18:14:02 -0400 Subject: [PATCH 1/5] create unidirectional read stream Signed-off-by: Dan Zhang --- .../common/quic/envoy_quic_server_session.cc | 20 +++++++++++--- .../common/quic/envoy_quic_server_session.h | 2 ++ .../quic/envoy_quic_server_session_test.cc | 26 +++++++++++++++++++ 3 files changed, 45 insertions(+), 3 deletions(-) diff --git a/source/common/quic/envoy_quic_server_session.cc b/source/common/quic/envoy_quic_server_session.cc index ca373f22547a1..d2ad24b12c802 100644 --- a/source/common/quic/envoy_quic_server_session.cc +++ b/source/common/quic/envoy_quic_server_session.cc @@ -56,18 +56,32 @@ quic::QuicSpdyStream* EnvoyQuicServerSession::CreateIncomingStream(quic::QuicStr } auto stream = new EnvoyQuicServerStream(id, this, quic::BIDIRECTIONAL, codec_stats_.value(), http3_options_.value(), headers_with_underscores_action_); + initializeStream(stream); + return stream; +} + +void EnvoyQuicServerSession::initializeStream(EnvoyQuicServerStream* stream) { ActivateStream(absl::WrapUnique(stream)); if (aboveHighWatermark()) { stream->runHighWatermarkCallbacks(); } setUpRequestDecoder(*stream); - return stream; } quic::QuicSpdyStream* -EnvoyQuicServerSession::CreateIncomingStream(quic::PendingStream* /*pending*/) { +EnvoyQuicServerSession::CreateIncomingStream(quic::PendingStream* pending) { // Only client side server push stream should trigger this call. - NOT_REACHED_GCOVR_EXCL_LINE; + if (!codec_stats_.has_value() || !http3_options_.has_value()) { + ENVOY_BUG(false, + fmt::format( + "Quic session {} attempts to create stream {} before HCM filter is initialized.", + this->id(), pending->id())); + return nullptr; + } + auto stream = new EnvoyQuicServerStream(pending, this, quic::BIDIRECTIONAL, codec_stats_.value(), + http3_options_.value(), headers_with_underscores_action_); + initializeStream(stream); + return stream; } quic::QuicSpdyStream* EnvoyQuicServerSession::CreateOutgoingBidirectionalStream() { diff --git a/source/common/quic/envoy_quic_server_session.h b/source/common/quic/envoy_quic_server_session.h index 9e6bedb7aac21..98e565bf68145 100644 --- a/source/common/quic/envoy_quic_server_session.h +++ b/source/common/quic/envoy_quic_server_session.h @@ -107,6 +107,8 @@ class EnvoyQuicServerSession : public quic::QuicServerSessionBase, private: void setUpRequestDecoder(EnvoyQuicServerStream& stream); + void initializeStream(EnvoyQuicServerStream* stream); + std::unique_ptr quic_connection_; // These callbacks are owned by network filters and quic session should out live // them. diff --git a/test/common/quic/envoy_quic_server_session_test.cc b/test/common/quic/envoy_quic_server_session_test.cc index 349afd7ff9737..64c8a13f1c2e5 100644 --- a/test/common/quic/envoy_quic_server_session_test.cc +++ b/test/common/quic/envoy_quic_server_session_test.cc @@ -1296,5 +1296,31 @@ TEST_P(EnvoyQuicServerSessionTest, OnCanWriteUpdateWatermarkGquic) { EXPECT_TRUE(stream1->IsFlowControlBlocked()); } +TEST_P(EnvoyQuicServerSessionTest, IncomingUnidirectionalReadStream) { + installReadFilter(); + Http::MockRequestDecoder request_decoder; + Http::MockStreamCallbacks stream_callbacks; + // IETF stream 5 and G-Quic stream 2 are server initiated. + quic::QuicStreamId stream_id = + quic::VersionUsesHttp3(quic_version_[0].transport_version) ? 2u : 3u; + auto payload = std::make_unique(8); + quic::QuicDataWriter payload_writer(8, payload.get()); + EXPECT_TRUE(payload_writer.WriteVarInt62(1ul)); + EXPECT_CALL(http_connection_callbacks_, newStream(_, false)) + .WillOnce(Invoke([&request_decoder, &stream_callbacks](Http::ResponseEncoder& encoder, + bool) -> Http::RequestDecoder& { + encoder.getStream().addCallbacks(stream_callbacks); + return request_decoder; + })); + + quic::QuicStreamFrame stream_frame(stream_id, false, 0, absl::string_view(payload.get(), 1)); + envoy_quic_session_.OnStreamFrame(stream_frame); + quic::QuicRstStreamFrame rst(/*control_frame_id=*/1u, stream_id, + quic::QUIC_REFUSED_STREAM, /*bytes_written=*/0u); +EXPECT_CALL(stream_callbacks, onResetStream(Http::StreamResetReason::RemoteRefusedStreamReset, _)); + envoy_quic_session_.OnRstStream(rst); +} + + } // namespace Quic } // namespace Envoy From 38774bbe981c49f20f2800bc33ca6484eb9ec76a Mon Sep 17 00:00:00 2001 From: Dan Zhang Date: Thu, 29 Jul 2021 20:04:05 -0400 Subject: [PATCH 2/5] format Signed-off-by: Dan Zhang --- source/common/quic/envoy_quic_server_session.cc | 3 +-- source/common/quic/envoy_quic_server_session.h | 2 +- test/common/quic/envoy_quic_server_session_test.cc | 7 ++++--- 3 files changed, 6 insertions(+), 6 deletions(-) diff --git a/source/common/quic/envoy_quic_server_session.cc b/source/common/quic/envoy_quic_server_session.cc index b42363f241565..41a6e6e58dd89 100644 --- a/source/common/quic/envoy_quic_server_session.cc +++ b/source/common/quic/envoy_quic_server_session.cc @@ -68,8 +68,7 @@ void EnvoyQuicServerSession::initializeStream(EnvoyQuicServerStream* stream) { setUpRequestDecoder(*stream); } -quic::QuicSpdyStream* -EnvoyQuicServerSession::CreateIncomingStream(quic::PendingStream* pending) { +quic::QuicSpdyStream* EnvoyQuicServerSession::CreateIncomingStream(quic::PendingStream* pending) { // Only client side server push stream should trigger this call. if (!codec_stats_.has_value() || !http3_options_.has_value()) { ENVOY_BUG(false, diff --git a/source/common/quic/envoy_quic_server_session.h b/source/common/quic/envoy_quic_server_session.h index 2af83609d3dc6..fcab1ee4a9d36 100644 --- a/source/common/quic/envoy_quic_server_session.h +++ b/source/common/quic/envoy_quic_server_session.h @@ -102,7 +102,7 @@ class EnvoyQuicServerSession : public quic::QuicServerSessionBase, void setUpRequestDecoder(EnvoyQuicServerStream& stream); void initializeStream(EnvoyQuicServerStream* stream); - + std::unique_ptr quic_connection_; // These callbacks are owned by network filters and quic session should out live // them. diff --git a/test/common/quic/envoy_quic_server_session_test.cc b/test/common/quic/envoy_quic_server_session_test.cc index fbb3e06916be5..d1408319ef3cc 100644 --- a/test/common/quic/envoy_quic_server_session_test.cc +++ b/test/common/quic/envoy_quic_server_session_test.cc @@ -1042,9 +1042,10 @@ TEST_F(EnvoyQuicServerSessionTest, IncomingUnidirectionalReadStream) { quic::QuicStreamFrame stream_frame(stream_id, false, 0, absl::string_view(payload.get(), 1)); envoy_quic_session_.OnStreamFrame(stream_frame); - quic::QuicRstStreamFrame rst(/*control_frame_id=*/1u, stream_id, - quic::QUIC_REFUSED_STREAM, /*bytes_written=*/0u); -EXPECT_CALL(stream_callbacks, onResetStream(Http::StreamResetReason::RemoteRefusedStreamReset, _)); + quic::QuicRstStreamFrame rst(/*control_frame_id=*/1u, stream_id, quic::QUIC_REFUSED_STREAM, + /*bytes_written=*/0u); + EXPECT_CALL(stream_callbacks, + onResetStream(Http::StreamResetReason::RemoteRefusedStreamReset, _)); envoy_quic_session_.OnRstStream(rst); } From 439243251c6d809ff5d04e9bc8735e67fb39dd53 Mon Sep 17 00:00:00 2001 From: Dan Zhang Date: Fri, 30 Jul 2021 16:43:19 -0400 Subject: [PATCH 3/5] not create server initiated stream Signed-off-by: Dan Zhang --- source/common/quic/envoy_quic_client_session.cc | 7 +++---- source/common/quic/envoy_quic_client_stream.cc | 10 ---------- source/common/quic/envoy_quic_client_stream.h | 3 --- test/common/quic/BUILD | 1 + .../quic/envoy_quic_client_session_test.cc | 17 +++++++++++++++++ .../quic/envoy_quic_server_session_test.cc | 5 ++--- 6 files changed, 23 insertions(+), 20 deletions(-) diff --git a/source/common/quic/envoy_quic_client_session.cc b/source/common/quic/envoy_quic_client_session.cc index 805afe0c1dceb..22e416781f8a5 100644 --- a/source/common/quic/envoy_quic_client_session.cc +++ b/source/common/quic/envoy_quic_client_session.cc @@ -91,14 +91,13 @@ std::unique_ptr EnvoyQuicClientSession::CreateClient } quic::QuicSpdyStream* EnvoyQuicClientSession::CreateIncomingStream(quic::QuicStreamId /*id*/) { - // Disallow server initiated stream. - NOT_REACHED_GCOVR_EXCL_LINE; + // Envoy doesn't support server initiated stream. + return nullptr; } quic::QuicSpdyStream* EnvoyQuicClientSession::CreateIncomingStream(quic::PendingStream* /*pending*/) { - // Disallow server initiated stream. - NOT_REACHED_GCOVR_EXCL_LINE; + return nullptr; } bool EnvoyQuicClientSession::hasDataToWrite() { return HasDataToWrite(); } diff --git a/source/common/quic/envoy_quic_client_stream.cc b/source/common/quic/envoy_quic_client_stream.cc index 804a39934fd85..9cd929b645af1 100644 --- a/source/common/quic/envoy_quic_client_stream.cc +++ b/source/common/quic/envoy_quic_client_stream.cc @@ -44,16 +44,6 @@ EnvoyQuicClientStream::EnvoyQuicClientStream( "Send buffer limit should be larger than 8KB."); } -EnvoyQuicClientStream::EnvoyQuicClientStream( - quic::PendingStream* pending, quic::QuicSpdyClientSession* client_session, - quic::StreamType type, Http::Http3::CodecStats& stats, - const envoy::config::core::v3::Http3ProtocolOptions& http3_options) - : quic::QuicSpdyClientStream(pending, client_session, type), - EnvoyQuicStream( - static_cast(GetReceiveWindow().value()), *filterManagerConnection(), - [this]() { runLowWatermarkCallbacks(); }, [this]() { runHighWatermarkCallbacks(); }, - stats, http3_options) {} - Http::Status EnvoyQuicClientStream::encodeHeaders(const Http::RequestHeaderMap& headers, bool end_stream) { // Required headers must be present. This can only happen by some erroneous processing after the diff --git a/source/common/quic/envoy_quic_client_stream.h b/source/common/quic/envoy_quic_client_stream.h index 0e82d8622319e..2292b6958e820 100644 --- a/source/common/quic/envoy_quic_client_stream.h +++ b/source/common/quic/envoy_quic_client_stream.h @@ -27,9 +27,6 @@ class EnvoyQuicClientStream : public quic::QuicSpdyClientStream, EnvoyQuicClientStream(quic::QuicStreamId id, quic::QuicSpdyClientSession* client_session, quic::StreamType type, Http::Http3::CodecStats& stats, const envoy::config::core::v3::Http3ProtocolOptions& http3_options); - EnvoyQuicClientStream(quic::PendingStream* pending, quic::QuicSpdyClientSession* client_session, - quic::StreamType type, Http::Http3::CodecStats& stats, - const envoy::config::core::v3::Http3ProtocolOptions& http3_options); void setResponseDecoder(Http::ResponseDecoder& decoder) { response_decoder_ = &decoder; } diff --git a/test/common/quic/BUILD b/test/common/quic/BUILD index 943fd4b241470..3c78046aa7060 100644 --- a/test/common/quic/BUILD +++ b/test/common/quic/BUILD @@ -182,6 +182,7 @@ envoy_cc_test( "//test/mocks/stats:stats_mocks", "//test/test_common:logging_lib", "//test/test_common:simulated_time_system_lib", + "@com_googlesource_quiche//:quic_test_tools_session_peer_lib", ], ) diff --git a/test/common/quic/envoy_quic_client_session_test.cc b/test/common/quic/envoy_quic_client_session_test.cc index 84554a41bd9da..49463c964d223 100644 --- a/test/common/quic/envoy_quic_client_session_test.cc +++ b/test/common/quic/envoy_quic_client_session_test.cc @@ -6,6 +6,7 @@ #include "quiche/quic/core/crypto/null_encrypter.h" #include "quiche/quic/test_tools/crypto_test_utils.h" +#include "quiche/quic/test_tools/quic_session_peer.h" #include "quiche/quic/test_tools/quic_test_utils.h" #if defined(__GNUC__) @@ -324,5 +325,21 @@ TEST_F(EnvoyQuicClientSessionTest, ConnectionClosePopulatesQuicVersionStats) { EXPECT_EQ(1U, TestUtility::findCounter(store_, "http3.quic_version_rfc_v1")->value()); } +TEST_F(EnvoyQuicClientSessionTest, IncomingUnidirectionalReadStream) { + // IETF stream 3 is server initiated uni-directional stream. + quic::QuicStreamId stream_id = 3u; + auto payload = std::make_unique(8); + quic::QuicDataWriter payload_writer(8, payload.get()); + EXPECT_TRUE(payload_writer.WriteVarInt62(1ul)); + quic::QuicStreamFrame stream_frame(stream_id, false, 0, absl::string_view(payload.get(), 1)); + envoy_quic_session_.OnStreamFrame(stream_frame); + EXPECT_FALSE(quic::test::QuicSessionPeer::IsStreamCreated(&envoy_quic_session_, stream_id)); + + stream_id = 1u; + quic::QuicStreamFrame stream_frame2(stream_id, false, 0, "aaa"); + envoy_quic_session_.OnStreamFrame(stream_frame2); + EXPECT_FALSE(quic::test::QuicSessionPeer::IsStreamCreated(&envoy_quic_session_, stream_id)); +} + } // namespace Quic } // namespace Envoy diff --git a/test/common/quic/envoy_quic_server_session_test.cc b/test/common/quic/envoy_quic_server_session_test.cc index d1408319ef3cc..313c45b51a2db 100644 --- a/test/common/quic/envoy_quic_server_session_test.cc +++ b/test/common/quic/envoy_quic_server_session_test.cc @@ -1027,9 +1027,8 @@ TEST_F(EnvoyQuicServerSessionTest, IncomingUnidirectionalReadStream) { installReadFilter(); Http::MockRequestDecoder request_decoder; Http::MockStreamCallbacks stream_callbacks; - // IETF stream 5 and G-Quic stream 2 are server initiated. - quic::QuicStreamId stream_id = - quic::VersionUsesHttp3(quic_version_[0].transport_version) ? 2u : 3u; + // IETF stream 2 is client initiated uni-directional stream. + quic::QuicStreamId stream_id = 2u; auto payload = std::make_unique(8); quic::QuicDataWriter payload_writer(8, payload.get()); EXPECT_TRUE(payload_writer.WriteVarInt62(1ul)); From fa24b94087047b1cff8e4f4614906da0e0be77a9 Mon Sep 17 00:00:00 2001 From: Dan Zhang Date: Thu, 5 Aug 2021 15:20:48 -0400 Subject: [PATCH 4/5] return nullptr for pending stream Signed-off-by: Dan Zhang --- .../common/quic/envoy_quic_client_session.cc | 1 + .../common/quic/envoy_quic_server_session.cc | 21 ++++--------------- .../common/quic/envoy_quic_server_session.h | 2 -- .../common/quic/envoy_quic_server_stream.cc | 16 -------------- source/common/quic/envoy_quic_server_stream.h | 6 ------ .../quic/envoy_quic_server_session_test.cc | 13 +----------- 6 files changed, 6 insertions(+), 53 deletions(-) diff --git a/source/common/quic/envoy_quic_client_session.cc b/source/common/quic/envoy_quic_client_session.cc index 22e416781f8a5..6b70ce5048db5 100644 --- a/source/common/quic/envoy_quic_client_session.cc +++ b/source/common/quic/envoy_quic_client_session.cc @@ -97,6 +97,7 @@ quic::QuicSpdyStream* EnvoyQuicClientSession::CreateIncomingStream(quic::QuicStr quic::QuicSpdyStream* EnvoyQuicClientSession::CreateIncomingStream(quic::PendingStream* /*pending*/) { + // Envoy doesn't support server push. return nullptr; } diff --git a/source/common/quic/envoy_quic_server_session.cc b/source/common/quic/envoy_quic_server_session.cc index 41a6e6e58dd89..122685c49dc76 100644 --- a/source/common/quic/envoy_quic_server_session.cc +++ b/source/common/quic/envoy_quic_server_session.cc @@ -56,31 +56,18 @@ quic::QuicSpdyStream* EnvoyQuicServerSession::CreateIncomingStream(quic::QuicStr } auto stream = new EnvoyQuicServerStream(id, this, quic::BIDIRECTIONAL, codec_stats_.value(), http3_options_.value(), headers_with_underscores_action_); - initializeStream(stream); - return stream; -} - -void EnvoyQuicServerSession::initializeStream(EnvoyQuicServerStream* stream) { ActivateStream(absl::WrapUnique(stream)); if (aboveHighWatermark()) { stream->runHighWatermarkCallbacks(); } setUpRequestDecoder(*stream); + return stream; } -quic::QuicSpdyStream* EnvoyQuicServerSession::CreateIncomingStream(quic::PendingStream* pending) { +quic::QuicSpdyStream* +EnvoyQuicServerSession::CreateIncomingStream(quic::PendingStream* /*pending*/) { // Only client side server push stream should trigger this call. - if (!codec_stats_.has_value() || !http3_options_.has_value()) { - ENVOY_BUG(false, - fmt::format( - "Quic session {} attempts to create stream {} before HCM filter is initialized.", - this->id(), pending->id())); - return nullptr; - } - auto stream = new EnvoyQuicServerStream(pending, this, quic::BIDIRECTIONAL, codec_stats_.value(), - http3_options_.value(), headers_with_underscores_action_); - initializeStream(stream); - return stream; + return nullptr; } quic::QuicSpdyStream* EnvoyQuicServerSession::CreateOutgoingBidirectionalStream() { diff --git a/source/common/quic/envoy_quic_server_session.h b/source/common/quic/envoy_quic_server_session.h index fcab1ee4a9d36..a3a804023aef9 100644 --- a/source/common/quic/envoy_quic_server_session.h +++ b/source/common/quic/envoy_quic_server_session.h @@ -101,8 +101,6 @@ class EnvoyQuicServerSession : public quic::QuicServerSessionBase, private: void setUpRequestDecoder(EnvoyQuicServerStream& stream); - void initializeStream(EnvoyQuicServerStream* stream); - std::unique_ptr quic_connection_; // These callbacks are owned by network filters and quic session should out live // them. diff --git a/source/common/quic/envoy_quic_server_stream.cc b/source/common/quic/envoy_quic_server_stream.cc index 86ac524b5e660..a1f9c02bd1626 100644 --- a/source/common/quic/envoy_quic_server_stream.cc +++ b/source/common/quic/envoy_quic_server_stream.cc @@ -49,22 +49,6 @@ EnvoyQuicServerStream::EnvoyQuicServerStream( "Send buffer limit should be larger than 8KB."); } -EnvoyQuicServerStream::EnvoyQuicServerStream( - quic::PendingStream* pending, quic::QuicSpdySession* session, quic::StreamType type, - Http::Http3::CodecStats& stats, - const envoy::config::core::v3::Http3ProtocolOptions& http3_options, - envoy::config::core::v3::HttpProtocolOptions::HeadersWithUnderscoresAction - headers_with_underscores_action) - : quic::QuicSpdyServerStreamBase(pending, session, type), - EnvoyQuicStream( - // This should be larger than 8k to fully utilize congestion control - // window. And no larger than the max stream flow control window for - // the stream to buffer all the data. - static_cast(GetReceiveWindow().value()), *filterManagerConnection(), - [this]() { runLowWatermarkCallbacks(); }, [this]() { runHighWatermarkCallbacks(); }, - stats, http3_options), - headers_with_underscores_action_(headers_with_underscores_action) {} - void EnvoyQuicServerStream::encode100ContinueHeaders(const Http::ResponseHeaderMap& headers) { ASSERT(headers.Status()->value() == "100"); encodeHeaders(headers, false); diff --git a/source/common/quic/envoy_quic_server_stream.h b/source/common/quic/envoy_quic_server_stream.h index e0b98e835fdad..3ec59d9ff2aaa 100644 --- a/source/common/quic/envoy_quic_server_stream.h +++ b/source/common/quic/envoy_quic_server_stream.h @@ -28,12 +28,6 @@ class EnvoyQuicServerStream : public quic::QuicSpdyServerStreamBase, envoy::config::core::v3::HttpProtocolOptions::HeadersWithUnderscoresAction headers_with_underscores_action); - EnvoyQuicServerStream(quic::PendingStream* pending, quic::QuicSpdySession* session, - quic::StreamType type, Http::Http3::CodecStats& stats, - const envoy::config::core::v3::Http3ProtocolOptions& http3_options, - envoy::config::core::v3::HttpProtocolOptions::HeadersWithUnderscoresAction - headers_with_underscores_action); - void setRequestDecoder(Http::RequestDecoder& decoder) { request_decoder_ = &decoder; } // Http::StreamEncoder diff --git a/test/common/quic/envoy_quic_server_session_test.cc b/test/common/quic/envoy_quic_server_session_test.cc index 313c45b51a2db..1cd7eef3f4e5d 100644 --- a/test/common/quic/envoy_quic_server_session_test.cc +++ b/test/common/quic/envoy_quic_server_session_test.cc @@ -1032,20 +1032,9 @@ TEST_F(EnvoyQuicServerSessionTest, IncomingUnidirectionalReadStream) { auto payload = std::make_unique(8); quic::QuicDataWriter payload_writer(8, payload.get()); EXPECT_TRUE(payload_writer.WriteVarInt62(1ul)); - EXPECT_CALL(http_connection_callbacks_, newStream(_, false)) - .WillOnce(Invoke([&request_decoder, &stream_callbacks](Http::ResponseEncoder& encoder, - bool) -> Http::RequestDecoder& { - encoder.getStream().addCallbacks(stream_callbacks); - return request_decoder; - })); - + EXPECT_CALL(http_connection_callbacks_, newStream(_, false)).Times(0u); quic::QuicStreamFrame stream_frame(stream_id, false, 0, absl::string_view(payload.get(), 1)); envoy_quic_session_.OnStreamFrame(stream_frame); - quic::QuicRstStreamFrame rst(/*control_frame_id=*/1u, stream_id, quic::QUIC_REFUSED_STREAM, - /*bytes_written=*/0u); - EXPECT_CALL(stream_callbacks, - onResetStream(Http::StreamResetReason::RemoteRefusedStreamReset, _)); - envoy_quic_session_.OnRstStream(rst); } } // namespace Quic From 4b84ac35cecf5b1b1025036aafe014167bb15ccb Mon Sep 17 00:00:00 2001 From: Dan Zhang Date: Mon, 9 Aug 2021 19:17:41 -0400 Subject: [PATCH 5/5] close connection for server push stream Signed-off-by: Dan Zhang --- .../common/quic/envoy_quic_client_session.cc | 2 +- .../common/quic/envoy_quic_server_session.cc | 2 +- .../common/quic/platform/quiche_flags_impl.cc | 1 + .../quic/envoy_quic_client_session_test.cc | 23 +++++++++++-------- .../quic/envoy_quic_server_session_test.cc | 9 ++++++-- 5 files changed, 23 insertions(+), 14 deletions(-) diff --git a/source/common/quic/envoy_quic_client_session.cc b/source/common/quic/envoy_quic_client_session.cc index 6b70ce5048db5..df9fd5f7a6755 100644 --- a/source/common/quic/envoy_quic_client_session.cc +++ b/source/common/quic/envoy_quic_client_session.cc @@ -98,7 +98,7 @@ quic::QuicSpdyStream* EnvoyQuicClientSession::CreateIncomingStream(quic::QuicStr quic::QuicSpdyStream* EnvoyQuicClientSession::CreateIncomingStream(quic::PendingStream* /*pending*/) { // Envoy doesn't support server push. - return nullptr; + NOT_REACHED_GCOVR_EXCL_LINE; } bool EnvoyQuicClientSession::hasDataToWrite() { return HasDataToWrite(); } diff --git a/source/common/quic/envoy_quic_server_session.cc b/source/common/quic/envoy_quic_server_session.cc index 122685c49dc76..15d1d28745dd3 100644 --- a/source/common/quic/envoy_quic_server_session.cc +++ b/source/common/quic/envoy_quic_server_session.cc @@ -67,7 +67,7 @@ quic::QuicSpdyStream* EnvoyQuicServerSession::CreateIncomingStream(quic::QuicStr quic::QuicSpdyStream* EnvoyQuicServerSession::CreateIncomingStream(quic::PendingStream* /*pending*/) { // Only client side server push stream should trigger this call. - return nullptr; + NOT_REACHED_GCOVR_EXCL_LINE; } quic::QuicSpdyStream* EnvoyQuicServerSession::CreateOutgoingBidirectionalStream() { diff --git a/source/common/quic/platform/quiche_flags_impl.cc b/source/common/quic/platform/quiche_flags_impl.cc index af607a83adc49..6dcd07e3f0815 100644 --- a/source/common/quic/platform/quiche_flags_impl.cc +++ b/source/common/quic/platform/quiche_flags_impl.cc @@ -34,6 +34,7 @@ absl::flat_hash_map makeFlagMap() { #undef QUIC_FLAG // Disable IETF draft 29 implementation. Envoy only supports RFC-v1. FLAGS_quic_reloadable_flag_quic_disable_version_draft_29->setValue(true); + FLAGS_quic_reloadable_flag_quic_decline_server_push_stream->setValue(true); #define QUIC_PROTOCOL_FLAG(type, flag, ...) flags.emplace(FLAGS_##flag->name(), FLAGS_##flag); #include "quiche/quic/core/quic_protocol_flags_list.h" diff --git a/test/common/quic/envoy_quic_client_session_test.cc b/test/common/quic/envoy_quic_client_session_test.cc index 49463c964d223..92f54a87f948c 100644 --- a/test/common/quic/envoy_quic_client_session_test.cc +++ b/test/common/quic/envoy_quic_client_session_test.cc @@ -70,8 +70,10 @@ class EnvoyQuicClientSessionTest : public testing::Test { EnvoyQuicClientSessionTest() : api_(Api::createApiForTest(time_system_)), dispatcher_(api_->allocateDispatcher("test_thread")), connection_helper_(*dispatcher_), - alarm_factory_(*dispatcher_, *connection_helper_.GetClock()), - quic_version_({quic::CurrentSupportedHttp3Versions()[0]}), + alarm_factory_(*dispatcher_, *connection_helper_.GetClock()), quic_version_([]() { + SetQuicReloadableFlag(quic_decline_server_push_stream, true); + return quic::CurrentSupportedHttp3Versions(); + }()), peer_addr_(Network::Utility::getAddressWithPort(*Network::Utility::getIpv6LoopbackAddress(), 12345)), self_addr_(Network::Utility::getAddressWithPort(*Network::Utility::getIpv6LoopbackAddress(), @@ -326,19 +328,20 @@ TEST_F(EnvoyQuicClientSessionTest, ConnectionClosePopulatesQuicVersionStats) { } TEST_F(EnvoyQuicClientSessionTest, IncomingUnidirectionalReadStream) { + quic::QuicStreamId stream_id = 1u; + quic::QuicStreamFrame stream_frame(stream_id, false, 0, "aaa"); + envoy_quic_session_.OnStreamFrame(stream_frame); + EXPECT_FALSE(quic::test::QuicSessionPeer::IsStreamCreated(&envoy_quic_session_, stream_id)); // IETF stream 3 is server initiated uni-directional stream. - quic::QuicStreamId stream_id = 3u; + stream_id = 3u; auto payload = std::make_unique(8); quic::QuicDataWriter payload_writer(8, payload.get()); EXPECT_TRUE(payload_writer.WriteVarInt62(1ul)); - quic::QuicStreamFrame stream_frame(stream_id, false, 0, absl::string_view(payload.get(), 1)); - envoy_quic_session_.OnStreamFrame(stream_frame); - EXPECT_FALSE(quic::test::QuicSessionPeer::IsStreamCreated(&envoy_quic_session_, stream_id)); - - stream_id = 1u; - quic::QuicStreamFrame stream_frame2(stream_id, false, 0, "aaa"); + EXPECT_CALL(network_connection_callbacks_, onEvent(Network::ConnectionEvent::LocalClose)); + EXPECT_CALL(*quic_connection_, SendConnectionClosePacket(quic::QUIC_HTTP_RECEIVE_SERVER_PUSH, _, + "Received server push stream")); + quic::QuicStreamFrame stream_frame2(stream_id, false, 0, absl::string_view(payload.get(), 1)); envoy_quic_session_.OnStreamFrame(stream_frame2); - EXPECT_FALSE(quic::test::QuicSessionPeer::IsStreamCreated(&envoy_quic_session_, stream_id)); } } // namespace Quic diff --git a/test/common/quic/envoy_quic_server_session_test.cc b/test/common/quic/envoy_quic_server_session_test.cc index 1cd7eef3f4e5d..ce493e33da91c 100644 --- a/test/common/quic/envoy_quic_server_session_test.cc +++ b/test/common/quic/envoy_quic_server_session_test.cc @@ -151,8 +151,10 @@ class EnvoyQuicServerSessionTest : public testing::Test { EnvoyQuicServerSessionTest() : api_(Api::createApiForTest(time_system_)), dispatcher_(api_->allocateDispatcher("test_thread")), connection_helper_(*dispatcher_), - alarm_factory_(*dispatcher_, *connection_helper_.GetClock()), - quic_version_({quic::CurrentSupportedHttp3Versions()[0]}), + alarm_factory_(*dispatcher_, *connection_helper_.GetClock()), quic_version_({[]() { + SetQuicReloadableFlag(quic_decline_server_push_stream, true); + return quic::CurrentSupportedHttp3Versions()[0]; + }()}), quic_stat_names_(listener_config_.listenerScope().symbolTable()), quic_connection_(new MockEnvoyQuicServerConnection( connection_helper_, alarm_factory_, writer_, quic_version_, *listener_config_.socket_)), @@ -1033,6 +1035,9 @@ TEST_F(EnvoyQuicServerSessionTest, IncomingUnidirectionalReadStream) { quic::QuicDataWriter payload_writer(8, payload.get()); EXPECT_TRUE(payload_writer.WriteVarInt62(1ul)); EXPECT_CALL(http_connection_callbacks_, newStream(_, false)).Times(0u); + EXPECT_CALL(network_connection_callbacks_, onEvent(Network::ConnectionEvent::LocalClose)); + EXPECT_CALL(*quic_connection_, SendConnectionClosePacket(quic::QUIC_HTTP_RECEIVE_SERVER_PUSH, _, + "Received server push stream")); quic::QuicStreamFrame stream_frame(stream_id, false, 0, absl::string_view(payload.get(), 1)); envoy_quic_session_.OnStreamFrame(stream_frame); }