http: add a test case where metadata exceed size limit

source/common/http/http2/codec_impl.cc

@@ -185,6 +185,7 @@ void ConnectionImpl::StreamImpl::pendingRecvBufferLowWatermark() {
 
 void ConnectionImpl::StreamImpl::decodeHeaders() {
   maybeTransformUpgradeFromH2ToH1();
+  ASSERT(decoder_ != nullptr);
   decoder_->decodeHeaders(std::move(headers_), remote_end_stream_);
 }
 
@@ -334,6 +335,7 @@ MetadataDecoder& ConnectionImpl::StreamImpl::getMetadataDecoder() {
 }
 
 void ConnectionImpl::StreamImpl::onMetadataDecoded(MetadataMapPtr&& metadata_map_ptr) {
+  ASSERT(decoder_ != nullptr);
   decoder_->decodeMetadata(std::move(metadata_map_ptr));
 }
 
@@ -430,6 +432,7 @@ int ConnectionImpl::onFrameReceived(const nghttp2_frame* frame) {
 
       if (stream->headers_->Status()->value() == "100") {
         ASSERT(!stream->remote_end_stream_);
+        ASSERT(stream->decoder_ != nullptr);
         stream->decoder_->decode100ContinueHeaders(std::move(stream->headers_));
       } else {
         stream->decodeHeaders();
@@ -457,6 +460,7 @@ int ConnectionImpl::onFrameReceived(const nghttp2_frame* frame) {
             stats_.too_many_header_frames_.inc();
             throw CodecProtocolException("Unexpected 'trailers' with no end stream.");
           } else {
+            ASSERT(stream->decoder_ != nullptr);
             stream->decoder_->decodeTrailers(std::move(stream->headers_));
           }
         } else {
@@ -487,6 +491,7 @@ int ConnectionImpl::onFrameReceived(const nghttp2_frame* frame) {
     // It's possible that we are waiting to send a deferred reset, so only raise data if local
     // is not complete.
     if (!stream->deferred_reset_) {
+      ASSERT(stream->decoder_ != nullptr);
       stream->decoder_->decodeData(stream->pending_recv_data_, stream->remote_end_stream_);
     }
 
@@ -539,18 +544,24 @@ int ConnectionImpl::onInvalidFrame(int32_t stream_id, int error_code) {
   ENVOY_CONN_LOG(debug, "invalid frame: {} on stream {}", connection_, nghttp2_strerror(error_code),
                  stream_id);
 
+  StreamImpl* stream = getStream(stream_id);
   // The stream is about to be closed due to an invalid header or messaging. Don't kill the
   // entire connection if one stream has bad headers or messaging.
   if (error_code == NGHTTP2_ERR_HTTP_HEADER || error_code == NGHTTP2_ERR_HTTP_MESSAGING) {
     stats_.rx_messaging_error_.inc();
-    StreamImpl* stream = getStream(stream_id);
     if (stream != nullptr) {
       // See comment below in onStreamClose() for why we do this.
       stream->reset_due_to_messaging_error_ = true;
     }
     return 0;
   }
 
+  if (stream != nullptr) {
+    // nghttp2 returns error, and ConnectionManager will call resetAllStreams(). Should not refer to
+    // decoder_ from now on. Null out stream->decoder_.
+    stream->decoder_ = nullptr;
+  }
+
   // Cause dispatch to return with an error code.
   return NGHTTP2_ERR_CALLBACK_FAILURE;
 }
@@ -606,6 +617,13 @@ int ConnectionImpl::onMetadataReceived(int32_t stream_id, const uint8_t* data, s
   }
 
   bool success = stream->getMetadataDecoder().receiveMetadata(data, len);
+
+  if (!success) {
+    // nghttp2 returns error, and ConnectionManager will call resetAllStreams(). Should not refer to
+    // decoder_ from now on. Null out stream->decoder_.
+    stream->decoder_ = nullptr;
+  }
+
   return success ? 0 : NGHTTP2_ERR_CALLBACK_FAILURE;
 }
 
@@ -617,6 +635,11 @@ int ConnectionImpl::onMetadataFrameComplete(int32_t stream_id, bool end_metadata
   ASSERT(stream != nullptr);
 
   bool result = stream->getMetadataDecoder().onMetadataFrameComplete(end_metadata);
+  if (!result) {
+    // nghttp2 returns error, and ConnectionManager will call resetAllStreams(). Should not refer to
+    // decoder_ from now on. Null out stream->decoder_.
+    stream->decoder_ = nullptr;
+  }
   return result ? 0 : NGHTTP2_ERR_CALLBACK_FAILURE;
 }
 

test/integration/http2_integration_test.cc

@@ -472,6 +472,32 @@ TEST_P(Http2MetadataIntegrationTest, ProxyLargeMetadataInRequest) {
   ASSERT_TRUE(response->complete());
 }
 
+TEST_P(Http2MetadataIntegrationTest, RequestMetadataReachSizeLimit) {
+  initialize();
+  codec_client_ = makeHttpConnection(lookupPort("http"));
+  fake_upstreams_[0]->set_allow_unexpected_disconnects(true);
+
+  auto encoder_decoder = codec_client_->startRequest(default_request_headers_);
+  request_encoder_ = &encoder_decoder.first;
+  auto response = std::move(encoder_decoder.second);
+  std::string value = std::string(10 * 1024, '1');
+  Http::MetadataMap metadata_map = {{"key", value}};
+  codec_client_->sendMetadata(*request_encoder_, metadata_map);
+  codec_client_->sendData(*request_encoder_, 1, false);
+  codec_client_->sendMetadata(*request_encoder_, metadata_map);
+  codec_client_->sendData(*request_encoder_, 1, false);
+  for (int i = 0; i < 200; i++) {
+    codec_client_->sendMetadata(*request_encoder_, metadata_map);
+    if (codec_client_->disconnected()) {
+      break;
+    }
+  }
+
+  // Verifies client connection will be closed.
+  codec_client_->waitForDisconnect();
+  ASSERT_FALSE(response->complete());
+}
+
 static std::string request_metadata_filter = R"EOF(
 name: request-metadata-filter
 config: {}