[http2] Dont drop connections on metadata limit exceeded (#32309)

* [http] Dont drop connections on metadata limit exceeded * remove bad test * Automated change: Fix sanity tests --------- Co-authored-by: ctiller <ctiller@users.noreply.github.com>
grpc · May 18, 2023 · 40c0898 · 40c0898
1 parent 129ea3c
commit 40c0898
Show file tree

Hide file tree

Showing 10 changed files with 128 additions and 418 deletions.
diff --git a/CMakeLists.txt b/CMakeLists.txt
diff --git a/build_autogenerated.yaml b/build_autogenerated.yaml
diff --git a/src/core/ext/transport/chttp2/transport/hpack_parser.cc b/src/core/ext/transport/chttp2/transport/hpack_parser.cc
@@ -1227,13 +1227,19 @@ class HPackParser::Parser {
         absl::StrCat("; adding ", md.key(), " (length ", md.transport_size(),
                      "B)", summary.empty() ? "" : " to ", summary);
     if (metadata_buffer_ != nullptr) metadata_buffer_->Clear();
+    // StreamId is used as a signal to skip this stream but keep the connection
+    // alive
     return input_->MaybeSetErrorAndReturn(
         [this, summary = std::move(summary)] {
           return grpc_error_set_int(
-              GRPC_ERROR_CREATE(absl::StrCat(
-                  "received initial metadata size exceeds limit (",
-                  *frame_length_, " vs. ", metadata_size_limit_, ")", summary)),
-              StatusIntProperty::kRpcStatus, GRPC_STATUS_RESOURCE_EXHAUSTED);
+              grpc_error_set_int(
+                  GRPC_ERROR_CREATE(absl::StrCat(
+                      "received initial metadata size exceeds limit (",
+                      *frame_length_, " vs. ", metadata_size_limit_, ")",
+                      summary)),
+                  StatusIntProperty::kRpcStatus,
+                  GRPC_STATUS_RESOURCE_EXHAUSTED),
+              StatusIntProperty::kStreamId, 0);
         },
         false);
   }

diff --git a/src/core/ext/transport/chttp2/transport/internal.h b/src/core/ext/transport/chttp2/transport/internal.h
@@ -567,8 +567,6 @@ struct grpc_chttp2_stream {
 
   grpc_core::Timestamp deadline = grpc_core::Timestamp::InfFuture();
 
-  /// saw some stream level error
-  grpc_error_handle forced_close_error;
   /// how many header frames have we received?
   uint8_t header_frames_received = 0;
   /// number of bytes received - reset at end of parse thread execution

diff --git a/src/core/ext/transport/chttp2/transport/parsing.cc b/src/core/ext/transport/chttp2/transport/parsing.cc
@@ -23,6 +23,7 @@
 
 #include <initializer_list>
 #include <string>
+#include <utility>
 
 #include "absl/base/attributes.h"
 #include "absl/status/status.h"
@@ -807,10 +808,7 @@ static grpc_error_handle parse_frame_slice(grpc_chttp2_transport* t,
                          &unused)) {
     grpc_chttp2_parsing_become_skip_parser(t);
     if (s) {
-      s->forced_close_error = err;
-      grpc_chttp2_add_rst_stream_to_next_write(t, t->incoming_stream_id,
-                                               GRPC_HTTP2_PROTOCOL_ERROR,
-                                               &s->stats.outgoing);
+      grpc_chttp2_cancel_stream(t, s, std::exchange(err, absl::OkStatus()));
     }
   }
   return err;

diff --git a/test/core/bad_client/generate_tests.bzl b/test/core/bad_client/generate_tests.bzl
@@ -29,7 +29,6 @@ BAD_CLIENT_TESTS = {
     "headers": test_options(),
     "initial_settings_frame": test_options(),
     "head_of_line_blocking": test_options(),
-    "large_metadata": test_options(),
     "out_of_bounds": test_options(),
     "server_registered_method": test_options(),
     "simple_request": test_options(),

diff --git a/test/core/bad_client/tests/large_metadata.cc b/test/core/bad_client/tests/large_metadata.cc