Merge pull request #6966 from ctiller/get_settings_first

Force receiving a SETTINGS frame as the first frame

4 files changed, 18 insertions, 7 deletions

diff --git a/src/core/ext/transport/chttp2/transport/chttp2_transport.c b/src/core/ext/transport/chttp2/transport/chttp2_transport.c
index 9aa39ba26c..4eb5ba8270 100644
--- a/src/core/ext/transport/chttp2/transport/chttp2_transport.c
+++ b/src/core/ext/transport/chttp2/transport/chttp2_transport.c
@@ -264,6 +264,7 @@ static void init_transport(grpc_exec_ctx *exec_ctx, grpc_chttp2_transport *t,
   t->parsing.is_client = is_client;
   t->parsing.deframe_state =
       is_client ? GRPC_DTS_FH_0 : GRPC_DTS_CLIENT_PREFIX_0;
+  t->parsing.is_first_frame = true;
   t->writing.is_client = is_client;
   t->optional_drop_message = gpr_empty_slice();
   grpc_connectivity_state_init(
@@ -643,8 +644,7 @@ static void finish_global_actions(grpc_exec_ctx *exec_ctx,
 
   for (;;) {
     if (!t->executor.writing_active && !t->closed &&
-        grpc_chttp2_unlocking_check_writes(exec_ctx, &t->global, &t->writing,
-                                           t->executor.parsing_active)) {
+        grpc_chttp2_unlocking_check_writes(exec_ctx, &t->global, &t->writing)) {
       t->executor.writing_active = 1;
       REF_TRANSPORT(t, "writing");
       prevent_endpoint_shutdown(t);
@@ -1805,7 +1805,7 @@ static void post_reading_action_locked(grpc_exec_ctx *exec_ctx,
     UNREF_TRANSPORT(exec_ctx, t, "reading_action");
   }
 
-  GRPC_ERROR_UNREF(error);
+  GRPC_LOG_IF_ERROR("close_transport", error);
 }
 
 /*******************************************************************************
diff --git a/src/core/ext/transport/chttp2/transport/internal.h b/src/core/ext/transport/chttp2/transport/internal.h
index 54e72ebd24..909c406f8a 100644
--- a/src/core/ext/transport/chttp2/transport/internal.h
+++ b/src/core/ext/transport/chttp2/transport/internal.h
@@ -265,6 +265,7 @@ struct grpc_chttp2_transport_parsing {
   uint8_t incoming_frame_type;
   uint8_t incoming_frame_flags;
   uint8_t header_eof;
+  bool is_first_frame;
   uint32_t expect_continuation_stream_id;
   uint32_t incoming_frame_size;
   uint32_t incoming_stream_id;
@@ -524,8 +525,7 @@ struct grpc_chttp2_stream {
     are required, and schedule them if so */
 int grpc_chttp2_unlocking_check_writes(grpc_exec_ctx *exec_ctx,
                                        grpc_chttp2_transport_global *global,
-                                       grpc_chttp2_transport_writing *writing,
-                                       int is_parsing);
+                                       grpc_chttp2_transport_writing *writing);
 void grpc_chttp2_perform_writes(
     grpc_exec_ctx *exec_ctx, grpc_chttp2_transport_writing *transport_writing,
     grpc_endpoint *endpoint);
diff --git a/src/core/ext/transport/chttp2/transport/parsing.c b/src/core/ext/transport/chttp2/transport/parsing.c
index 785134091f..991d7729af 100644
--- a/src/core/ext/transport/chttp2/transport/parsing.c
+++ b/src/core/ext/transport/chttp2/transport/parsing.c
@@ -468,6 +468,17 @@ grpc_error *grpc_chttp2_perform_read(
 
 static grpc_error *init_frame_parser(
     grpc_exec_ctx *exec_ctx, grpc_chttp2_transport_parsing *transport_parsing) {
+  if (transport_parsing->is_first_frame &&
+      transport_parsing->incoming_frame_type != GRPC_CHTTP2_FRAME_SETTINGS) {
+    char *msg;
+    gpr_asprintf(
+        &msg, "Expected SETTINGS frame as the first frame, got frame type %d",
+        transport_parsing->incoming_frame_type);
+    grpc_error *err = GRPC_ERROR_CREATE(msg);
+    gpr_free(msg);
+    return err;
+  }
+  transport_parsing->is_first_frame = false;
   if (transport_parsing->expect_continuation_stream_id != 0) {
     if (transport_parsing->incoming_frame_type !=
         GRPC_CHTTP2_FRAME_CONTINUATION) {
diff --git a/src/core/ext/transport/chttp2/transport/writing.c b/src/core/ext/transport/chttp2/transport/writing.c
index add7678182..b19f5f068d 100644
--- a/src/core/ext/transport/chttp2/transport/writing.c
+++ b/src/core/ext/transport/chttp2/transport/writing.c
@@ -45,7 +45,7 @@ static void finalize_outbuf(grpc_exec_ctx *exec_ctx,
 
 int grpc_chttp2_unlocking_check_writes(
     grpc_exec_ctx *exec_ctx, grpc_chttp2_transport_global *transport_global,
-    grpc_chttp2_transport_writing *transport_writing, int is_parsing) {
+    grpc_chttp2_transport_writing *transport_writing) {
   grpc_chttp2_stream_global *stream_global;
   grpc_chttp2_stream_writing *stream_writing;
 
@@ -61,7 +61,7 @@ int grpc_chttp2_unlocking_check_writes(
                                 [GRPC_CHTTP2_SETTINGS_HEADER_TABLE_SIZE]);
 
   if (transport_global->dirtied_local_settings &&
-      !transport_global->sent_local_settings && !is_parsing) {
+      !transport_global->sent_local_settings) {
     gpr_slice_buffer_add(
         &transport_writing->outbuf,
         grpc_chttp2_settings_create(