Merge pull request #4266 from ctiller/http2-is-a-spec

Make chttp2 pass h2spec section 6.5

src/core/transport/chttp2/frame_settings.c

@@ -36,27 +36,32 @@
 
 #include <string.h>
 
+#include <grpc/support/log.h>
+#include <grpc/support/useful.h>
+
 #include "src/core/debug/trace.h"
 #include "src/core/transport/chttp2/frame.h"
+#include "src/core/transport/chttp2/http2_errors.h"
 #include "src/core/transport/chttp2_transport.h"
-#include <grpc/support/log.h>
-#include <grpc/support/useful.h>
 
 /* HTTP/2 mandated initial connection settings */
 const grpc_chttp2_setting_parameters
     grpc_chttp2_settings_parameters[GRPC_CHTTP2_NUM_SETTINGS] = {
-        {NULL, 0, 0, 0, GRPC_CHTTP2_DISCONNECT_ON_INVALID_VALUE},
+        {NULL, 0, 0, 0, GRPC_CHTTP2_DISCONNECT_ON_INVALID_VALUE,
+         GRPC_CHTTP2_PROTOCOL_ERROR},
         {"HEADER_TABLE_SIZE", 4096, 0, 0xffffffff,
-         GRPC_CHTTP2_CLAMP_INVALID_VALUE},
-        {"ENABLE_PUSH", 1, 0, 1, GRPC_CHTTP2_DISCONNECT_ON_INVALID_VALUE},
+         GRPC_CHTTP2_CLAMP_INVALID_VALUE, GRPC_CHTTP2_PROTOCOL_ERROR},
+        {"ENABLE_PUSH", 1, 0, 1, GRPC_CHTTP2_DISCONNECT_ON_INVALID_VALUE,
+         GRPC_CHTTP2_PROTOCOL_ERROR},
         {"MAX_CONCURRENT_STREAMS", 0xffffffffu, 0, 0xffffffffu,
-         GRPC_CHTTP2_DISCONNECT_ON_INVALID_VALUE},
-        {"INITIAL_WINDOW_SIZE", 65535, 0, 0xffffffffu,
-         GRPC_CHTTP2_DISCONNECT_ON_INVALID_VALUE},
+         GRPC_CHTTP2_DISCONNECT_ON_INVALID_VALUE, GRPC_CHTTP2_PROTOCOL_ERROR},
+        {"INITIAL_WINDOW_SIZE", 65535, 0, 0x7fffffffu,
+         GRPC_CHTTP2_DISCONNECT_ON_INVALID_VALUE,
+         GRPC_CHTTP2_FLOW_CONTROL_ERROR},
         {"MAX_FRAME_SIZE", 16384, 16384, 16777215,
-         GRPC_CHTTP2_DISCONNECT_ON_INVALID_VALUE},
+         GRPC_CHTTP2_DISCONNECT_ON_INVALID_VALUE, GRPC_CHTTP2_PROTOCOL_ERROR},
         {"MAX_HEADER_LIST_SIZE", 0xffffffffu, 0, 0xffffffffu,
-         GRPC_CHTTP2_CLAMP_INVALID_VALUE},
+         GRPC_CHTTP2_CLAMP_INVALID_VALUE, GRPC_CHTTP2_PROTOCOL_ERROR},
 };
 
 static gpr_uint8 *fill_header(gpr_uint8 *out, gpr_uint32 length,
@@ -218,6 +223,10 @@ grpc_chttp2_parse_error grpc_chttp2_settings_parser_parse(
                     GPR_CLAMP(parser->value, sp->min_value, sp->max_value);
                 break;
               case GRPC_CHTTP2_DISCONNECT_ON_INVALID_VALUE:
+                grpc_chttp2_goaway_append(
+                    transport_parsing->last_incoming_stream_id, sp->error_value,
+                    gpr_slice_from_static_string("HTTP2 settings error"),
+                    &transport_parsing->qbuf);
                 gpr_log(GPR_ERROR, "invalid value %u passed for %s",
                         parser->value, sp->name);
                 return GRPC_CHTTP2_CONNECTION_ERROR;

src/core/transport/chttp2/frame_settings.h

@@ -79,6 +79,7 @@ typedef struct {
   gpr_uint32 min_value;
   gpr_uint32 max_value;
   grpc_chttp2_invalid_value_behavior invalid_value_behavior;
+  gpr_uint32 error_value;
 } grpc_chttp2_setting_parameters;
 
 /* HTTP/2 mandated connection setting parameters */

src/core/transport/chttp2/parsing.c

@@ -115,9 +115,6 @@ void grpc_chttp2_publish_reads(
         transport_parsing->incoming_stream_id;
   }
 
-  /* copy parsing qbuf to global qbuf */
-  gpr_slice_buffer_move_into(&transport_parsing->qbuf, &transport_global->qbuf);
-
   /* update global settings */
   if (transport_parsing->settings_updated) {
     memcpy(transport_global->settings[GRPC_PEER_SETTINGS],

src/core/transport/chttp2_transport.c

@@ -1299,7 +1299,11 @@ static void recv_data(grpc_exec_ctx *exec_ctx, void *tp, int success) {
       ;
     GPR_TIMER_END("recv_data.parse", 0);
     gpr_mu_lock(&t->mu);
+    /* copy parsing qbuf to global qbuf */
+    gpr_slice_buffer_move_into(&t->parsing.qbuf, &t->global.qbuf);
     if (i != t->read_buffer.count) {
+      unlock(exec_ctx, t);
+      lock(t);
       drop_connection(exec_ctx, t);
     }
     /* merge stream lists */