onStreamError

Signed-off-by: Dan Zhang <[email protected]>
envoyproxy · Apr 5, 2021 · 5577f48 · 5577f48
1 parent 7f01805
commit 5577f48
Show file tree

Hide file tree

Showing 5 changed files with 37 additions and 8 deletions.
diff --git a/source/common/quic/envoy_quic_client_stream.cc b/source/common/quic/envoy_quic_client_stream.cc
@@ -307,11 +307,17 @@ QuicFilterManagerConnectionImpl* EnvoyQuicClientStream::filterManagerConnection(
   return dynamic_cast<QuicFilterManagerConnectionImpl*>(session());
 }
 
-void EnvoyQuicClientStream::onStreamError(bool close_connection_upon_invalid_header) {
+void EnvoyQuicClientStream::onStreamError(absl::optional<bool> should_close_connection) {
   if (details_.empty()) {
     details_ = Http3ResponseCodeDetailValues::invalid_http_header;
   }
-
+  bool close_connection_upon_invalid_header;
+  if (should_close_connection != absl::nullopt) {
+    close_connection_upon_invalid_header = should_close_connection.value();
+  } else {
+    close_connection_upon_invalid_header =
+        !http3_options_.override_stream_error_on_invalid_http_message().value();
+  }
   if (close_connection_upon_invalid_header) {
     stream_delegate()->OnStreamError(quic::QUIC_HTTP_FRAME_ERROR, "Invalid headers");
   } else {

diff --git a/source/common/quic/envoy_quic_client_stream.h b/source/common/quic/envoy_quic_client_stream.h
@@ -78,7 +78,7 @@ class EnvoyQuicClientStream : public quic::QuicSpdyClientStream,
 
   // Either reset the stream or close the connection according to
   // close_connection_upon_invalid_header.
-  void onStreamError(bool close_connection_upon_invalid_header);
+  void onStreamError(absl::optional<bool> should_close_connection);
 
   Http::ResponseDecoder* response_decoder_{nullptr};
 

diff --git a/source/common/quic/envoy_quic_server_stream.cc b/source/common/quic/envoy_quic_server_stream.cc
@@ -148,7 +148,7 @@ void EnvoyQuicServerStream::OnInitialHeadersComplete(bool fin, size_t frame_len,
   }
   quic::QuicSpdyServerStreamBase::OnInitialHeadersComplete(fin, frame_len, header_list);
   if (!headers_decompressed() || header_list.empty()) {
-    onStreamError(!http3_options_.override_stream_error_on_invalid_http_message().value());
+    onStreamError(absl::nullopt);
     return;
   }
 
@@ -164,7 +164,7 @@ void EnvoyQuicServerStream::OnInitialHeadersComplete(bool fin, size_t frame_len,
   }
   if (Http::HeaderUtility::requestHeadersValid(*headers) != absl::nullopt) {
     details_ = Http3ResponseCodeDetailValues::invalid_http_header;
-    onStreamError(!http3_options_.override_stream_error_on_invalid_http_message().value());
+    onStreamError(absl::nullopt);
     return;
   }
   request_decoder_->decodeHeaders(std::move(headers),
@@ -336,11 +336,18 @@ EnvoyQuicServerStream::validateHeader(const std::string& header_name,
   return result;
 }
 
-void EnvoyQuicServerStream::onStreamError(bool close_connection_upon_invalid_header) {
+void EnvoyQuicServerStream::onStreamError(absl::optional<bool> should_close_connection) {
   if (details_.empty()) {
     details_ = Http3ResponseCodeDetailValues::invalid_http_header;
   }
 
+  bool close_connection_upon_invalid_header;
+  if (should_close_connection != absl::nullopt) {
+    close_connection_upon_invalid_header = should_close_connection.value();
+  } else {
+    close_connection_upon_invalid_header =
+        !http3_options_.override_stream_error_on_invalid_http_message().value();
+  }
   if (close_connection_upon_invalid_header) {
     stream_delegate()->OnStreamError(quic::QUIC_HTTP_FRAME_ERROR, "Invalid headers");
   } else {

diff --git a/source/common/quic/envoy_quic_server_stream.h b/source/common/quic/envoy_quic_server_stream.h
@@ -91,7 +91,7 @@ class EnvoyQuicServerStream : public quic::QuicSpdyServerStreamBase,
 
   // Either reset the stream or close the connection according to
   // close_connection_upon_invalid_header.
-  void onStreamError(bool close_connection_upon_invalid_header);
+  void onStreamError(absl::optional<bool> should_close_connection);
 
   Http::RequestDecoder* request_decoder_{nullptr};
   envoy::config::core::v3::HttpProtocolOptions::HeadersWithUnderscoresAction

diff --git a/test/common/quic/envoy_quic_server_session_test.cc b/test/common/quic/envoy_quic_server_session_test.cc
@@ -1,3 +1,5 @@
+#include <memory>
+
 #if defined(__GNUC__)
 #pragma GCC diagnostic push
 #pragma GCC diagnostic ignored "-Wunused-parameter"
@@ -162,7 +164,6 @@ class EnvoyQuicServerSessionTest : public testing::TestWithParam<bool> {
                             &compressed_certs_cache_, *dispatcher_,
                             /*send_buffer_limit*/ quic::kDefaultFlowControlSendWindow * 1.5,
                             listener_config_),
-        read_filter_(new Network::MockReadFilter()),
         stats_({ALL_HTTP3_CODEC_STATS(
             POOL_COUNTER_PREFIX(listener_config_.listenerScope(), "http3."),
             POOL_GAUGE_PREFIX(listener_config_.listenerScope(), "http3."))}) {
@@ -213,6 +214,7 @@ class EnvoyQuicServerSessionTest : public testing::TestWithParam<bool> {
 
   bool installReadFilter() {
     // Setup read filter.
+    read_filter_ = std::make_shared<Network::MockReadFilter>(),
     envoy_quic_session_.addReadFilter(read_filter_);
     EXPECT_EQ(Http::Protocol::Http3,
               read_filter_->callbacks_->connection().streamInfo().protocol().value());
@@ -290,6 +292,19 @@ class EnvoyQuicServerSessionTest : public testing::TestWithParam<bool> {
 INSTANTIATE_TEST_SUITE_P(EnvoyQuicServerSessionTests, EnvoyQuicServerSessionTest,
                          testing::ValuesIn({true, false}));
 
+TEST_P(EnvoyQuicServerSessionTest, NewStreamBeforeInitializingFilter) {
+  quic::QuicStreamId stream_id =
+      quic::VersionUsesHttp3(quic_version_[0].transport_version) ? 4u : 5u;
+  EXPECT_ENVOY_BUG(envoy_quic_session_.GetOrCreateStream(stream_id),
+                   fmt::format("attempts to create stream", envoy_quic_session_.id(), stream_id));
+  EXPECT_CALL(*quic_connection_,
+              SendConnectionClosePacket(quic::QUIC_NO_ERROR, _, "Closed by application"));
+  EXPECT_CALL(*quic_connection_, SendControlFrame(_))
+      .Times(testing::AtMost(1))
+      .WillOnce(Invoke([](const quic::QuicFrame&) { return false; }));
+  envoy_quic_session_.close(Network::ConnectionCloseType::NoFlush);
+}
+
 TEST_P(EnvoyQuicServerSessionTest, NewStream) {
   installReadFilter();
 
@@ -786,6 +801,7 @@ TEST_P(EnvoyQuicServerSessionTest, GoAway) {
 }
 
 TEST_P(EnvoyQuicServerSessionTest, InitializeFilterChain) {
+  read_filter_ = std::make_shared<Network::MockReadFilter>();
   Network::MockFilterChain filter_chain;
   crypto_stream_->setProofSourceDetails(
       std::make_unique<EnvoyQuicProofSourceDetails>(filter_chain));