Merge branch 'merge-parse' into clean-up

src/core/ext/transport/chttp2/transport/chttp2_transport.c

@@ -102,12 +102,6 @@ static void push_setting(grpc_exec_ctx *exec_ctx, grpc_chttp2_transport *t,
 static void drop_connection(grpc_exec_ctx *exec_ctx, grpc_chttp2_transport *t,
                             grpc_error *error);
 
-/** Cancel a stream: coming from the transport API */
-static void cancel_from_api(grpc_exec_ctx *exec_ctx,
-                            grpc_chttp2_transport_global *transport_global,
-                            grpc_chttp2_stream_global *stream_global,
-                            grpc_error *error);
-
 static void close_from_api(grpc_exec_ctx *exec_ctx,
                            grpc_chttp2_transport_global *transport_global,
                            grpc_chttp2_stream_global *stream_global,
@@ -899,10 +893,11 @@ static void maybe_start_some_streams(
   while (transport_global->next_stream_id >= MAX_CLIENT_STREAM_ID &&
          grpc_chttp2_list_pop_waiting_for_concurrency(transport_global,
                                                       &stream_global)) {
-    cancel_from_api(exec_ctx, transport_global, stream_global,
-                    grpc_error_set_int(
-                        GRPC_ERROR_CREATE("Stream IDs exhausted"),
-                        GRPC_ERROR_INT_GRPC_STATUS, GRPC_STATUS_UNAVAILABLE));
+    grpc_chttp2_cancel_stream(
+        exec_ctx, transport_global, stream_global,
+        grpc_error_set_int(GRPC_ERROR_CREATE("Stream IDs exhausted"),
+                           GRPC_ERROR_INT_GRPC_STATUS,
+                           GRPC_STATUS_UNAVAILABLE));
   }
 }
 
@@ -992,8 +987,8 @@ static void perform_stream_op_locked(grpc_exec_ctx *exec_ctx, void *stream_op,
   }
 
   if (op->cancel_error != GRPC_ERROR_NONE) {
-    cancel_from_api(exec_ctx, transport_global, stream_global,
-                    GRPC_ERROR_REF(op->cancel_error));
+    grpc_chttp2_cancel_stream(exec_ctx, transport_global, stream_global,
+                              GRPC_ERROR_REF(op->cancel_error));
   }
 
   if (op->close_error != GRPC_ERROR_NONE) {
@@ -1017,7 +1012,7 @@ static void perform_stream_op_locked(grpc_exec_ctx *exec_ctx, void *stream_op,
                        stream_global->send_initial_metadata->deadline);
     }
     if (metadata_size > metadata_peer_limit) {
-      cancel_from_api(
+      grpc_chttp2_cancel_stream(
           exec_ctx, transport_global, stream_global,
           grpc_error_set_int(
               grpc_error_set_int(
@@ -1084,7 +1079,7 @@ static void perform_stream_op_locked(grpc_exec_ctx *exec_ctx, void *stream_op,
         transport_global->settings[GRPC_PEER_SETTINGS]
                                   [GRPC_CHTTP2_SETTINGS_MAX_HEADER_LIST_SIZE];
     if (metadata_size > metadata_peer_limit) {
-      cancel_from_api(
+      grpc_chttp2_cancel_stream(
           exec_ctx, transport_global, stream_global,
           grpc_error_set_int(
               grpc_error_set_int(
@@ -1298,14 +1293,6 @@ static void check_read_ops(grpc_exec_ctx *exec_ctx,
                     &stream_global->incoming_frames)) != NULL) {
           incoming_byte_stream_destroy_locked(exec_ctx, bs, GRPC_ERROR_NONE);
         }
-        if (stream_global->exceeded_metadata_size) {
-          cancel_from_api(
-              exec_ctx, transport_global, stream_global,
-              grpc_error_set_int(
-                  GRPC_ERROR_CREATE(
-                      "received initial metadata size exceeds limit"),
-                  GRPC_ERROR_INT_GRPC_STATUS, GRPC_STATUS_RESOURCE_EXHAUSTED));
-        }
       }
       grpc_chttp2_incoming_metadata_buffer_publish(
           &stream_global->metadata_buffer[0],
@@ -1342,14 +1329,6 @@ static void check_read_ops(grpc_exec_ctx *exec_ctx,
                     &stream_global->incoming_frames)) != NULL) {
           incoming_byte_stream_destroy_locked(exec_ctx, bs, GRPC_ERROR_NONE);
         }
-        if (stream_global->exceeded_metadata_size) {
-          cancel_from_api(
-              exec_ctx, transport_global, stream_global,
-              grpc_error_set_int(
-                  GRPC_ERROR_CREATE(
-                      "received trailing metadata size exceeds limit"),
-                  GRPC_ERROR_INT_GRPC_STATUS, GRPC_STATUS_RESOURCE_EXHAUSTED));
-        }
       }
       if (stream_global->all_incoming_byte_streams_finished) {
         grpc_chttp2_incoming_metadata_buffer_publish(
@@ -1432,10 +1411,10 @@ static void status_codes_from_error(grpc_error *error, gpr_timespec deadline,
   }
 }
 
-static void cancel_from_api(grpc_exec_ctx *exec_ctx,
-                            grpc_chttp2_transport_global *transport_global,
-                            grpc_chttp2_stream_global *stream_global,
-                            grpc_error *due_to_error) {
+void grpc_chttp2_cancel_stream(grpc_exec_ctx *exec_ctx,
+                               grpc_chttp2_transport_global *transport_global,
+                               grpc_chttp2_stream_global *stream_global,
+                               grpc_error *due_to_error) {
   if (!stream_global->read_closed || !stream_global->write_closed) {
     grpc_status_code grpc_status;
     grpc_chttp2_error_code http_error;
@@ -1727,8 +1706,8 @@ typedef struct {
 static void cancel_stream_cb(void *user_data, uint32_t key, void *stream) {
   cancel_stream_cb_args *args = user_data;
   grpc_chttp2_stream *s = stream;
-  cancel_from_api(args->exec_ctx, &args->t->global, &s->global,
-                  GRPC_ERROR_REF(args->error));
+  grpc_chttp2_cancel_stream(args->exec_ctx, &args->t->global, &s->global,
+                            GRPC_ERROR_REF(args->error));
 }
 
 static void end_all_the_calls(grpc_exec_ctx *exec_ctx, grpc_chttp2_transport *t,

src/core/ext/transport/chttp2/transport/hpack_parser.h

@@ -45,7 +45,8 @@
 typedef struct grpc_chttp2_hpack_parser grpc_chttp2_hpack_parser;
 
 typedef grpc_error *(*grpc_chttp2_hpack_parser_state)(
-    grpc_chttp2_hpack_parser *p, const uint8_t *beg, const uint8_t *end);
+    grpc_exec_ctx *exec_ctx, grpc_chttp2_hpack_parser *p, const uint8_t *beg,
+    const uint8_t *end);
 
 typedef struct {
   char *str;
@@ -55,7 +56,7 @@ typedef struct {
 
 struct grpc_chttp2_hpack_parser {
   /* user specified callback for each header output */
-  void (*on_header)(void *user_data, grpc_mdelem *md);
+  void (*on_header)(grpc_exec_ctx *exec_ctx, void *user_data, grpc_mdelem *md);
   void *on_header_user_data;
 
   grpc_error *last_error;
@@ -104,7 +105,8 @@ void grpc_chttp2_hpack_parser_destroy(grpc_chttp2_hpack_parser *p);
 void grpc_chttp2_hpack_parser_set_has_priority(grpc_chttp2_hpack_parser *p);
 
 /* returns 1 on success, 0 on error */
-grpc_error *grpc_chttp2_hpack_parser_parse(grpc_chttp2_hpack_parser *p,
+grpc_error *grpc_chttp2_hpack_parser_parse(grpc_exec_ctx *exec_ctx,
+                                           grpc_chttp2_hpack_parser *p,
                                            const uint8_t *beg,
                                            const uint8_t *end);
 

src/core/ext/transport/chttp2/transport/internal.h

@@ -409,7 +409,6 @@ struct grpc_chttp2_stream_global {
   /** Has this stream seen an error.
       If true, then pending incoming frames can be thrown away. */
   bool seen_error;
-  bool exceeded_metadata_size;
 
   /** the error that resulted in this stream being read-closed */
   grpc_error *read_closed_error;
@@ -749,4 +748,9 @@ void grpc_chttp2_become_writable(grpc_exec_ctx *exec_ctx,
                                  grpc_chttp2_stream_global *stream_global,
                                  bool covered_by_poller, const char *reason);
 
+void grpc_chttp2_cancel_stream(grpc_exec_ctx *exec_ctx,
+                               grpc_chttp2_transport_global *transport_global,
+                               grpc_chttp2_stream_global *stream_global,
+                               grpc_error *due_to_error);
+
 #endif /* GRPC_CORE_EXT_TRANSPORT_CHTTP2_TRANSPORT_INTERNAL_H */

src/core/ext/transport/chttp2/transport/parsing.c

@@ -354,7 +354,9 @@ static grpc_error *skip_parser(grpc_exec_ctx *exec_ctx, void *parser,
   return GRPC_ERROR_NONE;
 }
 
-static void skip_header(void *tp, grpc_mdelem *md) { GRPC_MDELEM_UNREF(md); }
+static void skip_header(grpc_exec_ctx *exec_ctx, void *tp, grpc_mdelem *md) {
+  GRPC_MDELEM_UNREF(md);
+}
 
 static grpc_error *init_skip_frame_parser(
     grpc_exec_ctx *exec_ctx, grpc_chttp2_transport_global *transport_global,
@@ -458,7 +460,8 @@ static grpc_error *init_data_frame_parser(
 
 static void free_timeout(void *p) { gpr_free(p); }
 
-static void on_initial_header(void *tp, grpc_mdelem *md) {
+static void on_initial_header(grpc_exec_ctx *exec_ctx, void *tp,
+                              grpc_mdelem *md) {
   grpc_chttp2_transport_global *transport_global = tp;
   grpc_chttp2_stream_global *stream_global = transport_global->incoming_stream;
 
@@ -500,14 +503,17 @@ static void on_initial_header(void *tp, grpc_mdelem *md) {
         transport_global->settings[GRPC_ACKED_SETTINGS]
                                   [GRPC_CHTTP2_SETTINGS_MAX_HEADER_LIST_SIZE];
     if (new_size > metadata_size_limit) {
-      if (!stream_global->exceeded_metadata_size) {
-        gpr_log(GPR_DEBUG,
-                "received initial metadata size exceeds limit (%" PRIuPTR
-                " vs. %" PRIuPTR ")",
-                new_size, metadata_size_limit);
-        stream_global->seen_error = true;
-        stream_global->exceeded_metadata_size = true;
-      }
+      gpr_log(GPR_DEBUG,
+              "received initial metadata size exceeds limit (%" PRIuPTR
+              " vs. %" PRIuPTR ")",
+              new_size, metadata_size_limit);
+      grpc_chttp2_cancel_stream(
+          exec_ctx, transport_global, stream_global,
+          grpc_error_set_int(
+              GRPC_ERROR_CREATE("received initial metadata size exceeds limit"),
+              GRPC_ERROR_INT_GRPC_STATUS, GRPC_STATUS_RESOURCE_EXHAUSTED));
+      grpc_chttp2_parsing_become_skip_parser(exec_ctx, transport_global);
+      stream_global->seen_error = true;
       GRPC_MDELEM_UNREF(md);
     } else {
       grpc_chttp2_incoming_metadata_buffer_add(
@@ -518,7 +524,8 @@ static void on_initial_header(void *tp, grpc_mdelem *md) {
   GPR_TIMER_END("on_initial_header", 0);
 }
 
-static void on_trailing_header(void *tp, grpc_mdelem *md) {
+static void on_trailing_header(grpc_exec_ctx *exec_ctx, void *tp,
+                               grpc_mdelem *md) {
   grpc_chttp2_transport_global *transport_global = tp;
   grpc_chttp2_stream_global *stream_global = transport_global->incoming_stream;
 
@@ -542,14 +549,17 @@ static void on_trailing_header(void *tp, grpc_mdelem *md) {
       transport_global->settings[GRPC_ACKED_SETTINGS]
                                 [GRPC_CHTTP2_SETTINGS_MAX_HEADER_LIST_SIZE];
   if (new_size > metadata_size_limit) {
-    if (!stream_global->exceeded_metadata_size) {
-      gpr_log(GPR_DEBUG,
-              "received trailing metadata size exceeds limit (%" PRIuPTR
-              " vs. %" PRIuPTR ")",
-              new_size, metadata_size_limit);
-      stream_global->seen_error = true;
-      stream_global->exceeded_metadata_size = true;
-    }
+    gpr_log(GPR_DEBUG,
+            "received trailing metadata size exceeds limit (%" PRIuPTR
+            " vs. %" PRIuPTR ")",
+            new_size, metadata_size_limit);
+    grpc_chttp2_cancel_stream(
+        exec_ctx, transport_global, stream_global,
+        grpc_error_set_int(
+            GRPC_ERROR_CREATE("received trailing metadata size exceeds limit"),
+            GRPC_ERROR_INT_GRPC_STATUS, GRPC_STATUS_RESOURCE_EXHAUSTED));
+    grpc_chttp2_parsing_become_skip_parser(exec_ctx, transport_global);
+    stream_global->seen_error = true;
     GRPC_MDELEM_UNREF(md);
   } else {
     grpc_chttp2_incoming_metadata_buffer_add(&stream_global->metadata_buffer[1],

test/core/bad_client/tests/large_metadata.c

@@ -50,6 +50,7 @@
   "\x00\x00\x00\x04\x00\x00\x00\x00\x00" /* headers: generated from        \
                                             large_metadata.headers in this \
                                             directory */                   \
+  "\x00\x00\x00\x04\x01\x00\x00\x00\x00"                                   \
   "\x00"                                                                   \
   "5{\x01\x05\x00\x00\x00\x01"                                             \
   "\x10\x05:path\x08/foo/bar"                                              \
@@ -92,6 +93,7 @@
                                                                     in this                \
                                                                     directory              \
                                                                     */                     \
+  "\x00\x00\x00\x04\x01\x00\x00\x00\x00"                                                   \
   "\x00\x00\xc9\x01\x04\x00\x00\x00\x01"                                                   \
   "\x10\x05:path\x08/foo/bar"                                                              \
   "\x10\x07:scheme\x04http"                                                                \

test/core/transport/chttp2/hpack_parser_fuzzer_test.c

@@ -43,7 +43,9 @@
 bool squelch = true;
 bool leak_check = true;
 
-static void onhdr(void *ud, grpc_mdelem *md) { GRPC_MDELEM_UNREF(md); }
+static void onhdr(grpc_exec_ctx *exec_ctx, void *ud, grpc_mdelem *md) {
+  GRPC_MDELEM_UNREF(md);
+}
 static void dont_log(gpr_log_func_args *args) {}
 
 int LLVMFuzzerTestOneInput(const uint8_t *data, size_t size) {
@@ -53,7 +55,10 @@ int LLVMFuzzerTestOneInput(const uint8_t *data, size_t size) {
   grpc_chttp2_hpack_parser parser;
   grpc_chttp2_hpack_parser_init(&parser);
   parser.on_header = onhdr;
-  GRPC_ERROR_UNREF(grpc_chttp2_hpack_parser_parse(&parser, data, data + size));
+  grpc_exec_ctx exec_ctx = GRPC_EXEC_CTX_INIT;
+  GRPC_ERROR_UNREF(
+      grpc_chttp2_hpack_parser_parse(&exec_ctx, &parser, data, data + size));
+  grpc_exec_ctx_finish(&exec_ctx);
   grpc_chttp2_hpack_parser_destroy(&parser);
   grpc_shutdown();
   return 0;

test/core/transport/chttp2/hpack_parser_test.c

@@ -45,7 +45,7 @@
 
 typedef struct { va_list args; } test_checker;
 
-static void onhdr(void *ud, grpc_mdelem *md) {
+static void onhdr(grpc_exec_ctx *exec_ctx, void *ud, grpc_mdelem *md) {
   const char *ekey, *evalue;
   test_checker *chk = ud;
   ekey = va_arg(chk->args, char *);
@@ -75,9 +75,11 @@ static void test_vector(grpc_chttp2_hpack_parser *parser,
   gpr_slice_unref(input);
 
   for (i = 0; i < nslices; i++) {
+    grpc_exec_ctx exec_ctx = GRPC_EXEC_CTX_INIT;
     GPR_ASSERT(grpc_chttp2_hpack_parser_parse(
-                   parser, GPR_SLICE_START_PTR(slices[i]),
+                   &exec_ctx, parser, GPR_SLICE_START_PTR(slices[i]),
                    GPR_SLICE_END_PTR(slices[i])) == GRPC_ERROR_NONE);
+    grpc_exec_ctx_finish(&exec_ctx);
   }
 
   for (i = 0; i < nslices; i++) {