Add a test of bad settings frame (and fix a bug!)

src/core/transport/chttp2_transport.c

@@ -1575,12 +1575,21 @@ static int init_goaway_parser(transport *t) {
 }
 
 static int init_settings_frame_parser(transport *t) {
-  int ok = GRPC_CHTTP2_PARSE_OK ==
+  int ok;
+
+  if (t->incoming_stream_id != 0) {
+    gpr_log(GPR_ERROR, "settings frame received for stream %d", t->incoming_stream_id);
+    drop_connection(t);
+    return 0;
+  }
+
+  ok = GRPC_CHTTP2_PARSE_OK ==
            grpc_chttp2_settings_parser_begin_frame(
                &t->simple_parsers.settings, t->incoming_frame_size,
                t->incoming_frame_flags, t->settings[PEER_SETTINGS]);
   if (!ok) {
     drop_connection(t);
+    return 0;
   }
   if (t->incoming_frame_flags & GRPC_CHTTP2_FLAG_ACK) {
     memcpy(t->settings[ACKED_SETTINGS], t->settings[SENT_SETTINGS],

test/core/bad_client/bad_client.c

@@ -38,6 +38,7 @@
 #include "src/core/iomgr/endpoint_pair.h"
 #include "src/core/surface/completion_queue.h"
 #include "src/core/surface/server.h"
+#include "src/core/support/string.h"
 #include "src/core/transport/chttp2_transport.h"
 
 #include <grpc/support/sync.h>
@@ -72,17 +73,21 @@ static grpc_transport_setup_result server_setup_transport(
                                      grpc_server_get_channel_args(a->server));
 }
 
-void grpc_run_bad_client_test(const char *name, const char *client_payload,
+void grpc_run_bad_client_test(grpc_bad_client_server_side_validator validator,
+                              const char *client_payload,
                               size_t client_payload_length,
-                              grpc_bad_client_server_side_validator validator) {
+                              gpr_uint32 flags) {
   grpc_endpoint_pair sfd;
   thd_args a;
   gpr_thd_id id;
+  char* hex;
   gpr_slice slice =
       gpr_slice_from_copied_buffer(client_payload, client_payload_length);
 
+  hex = gpr_hexdump(client_payload, client_payload_length, GPR_HEXDUMP_PLAINTEXT);
+
   /* Add a debug log */
-  gpr_log(GPR_INFO, "TEST: %s", name);
+  gpr_log(GPR_INFO, "TEST: %s", hex);
 
   /* Init grpc */
   grpc_init();
@@ -126,10 +131,18 @@ void grpc_run_bad_client_test(const char *name, const char *client_payload,
   /* Await completion */
   GPR_ASSERT(
       gpr_event_wait(&a.done_write, GRPC_TIMEOUT_SECONDS_TO_DEADLINE(5)));
+
+  if (flags & GRPC_BAD_CLIENT_DISCONNECT) {
+    grpc_endpoint_destroy(sfd.client);
+    sfd.client = NULL;
+  }
+
   GPR_ASSERT(gpr_event_wait(&a.done_thd, GRPC_TIMEOUT_SECONDS_TO_DEADLINE(5)));
 
   /* Shutdown */
-  grpc_endpoint_destroy(sfd.client);
+  if (sfd.client) {
+    grpc_endpoint_destroy(sfd.client);
+  }
   grpc_server_destroy(a.server);
   grpc_completion_queue_destroy(a.cq);
 

test/core/bad_client/bad_client.h

@@ -40,13 +40,19 @@
 typedef void (*grpc_bad_client_server_side_validator)(
     grpc_server *server, grpc_completion_queue *cq);
 
+#define GRPC_BAD_CLIENT_DISCONNECT 1
+
 /* Test runner.
 
    Create a server, and send client_payload to it as bytes from a client.
    Execute validator in a separate thread to assert that the bytes are
    handled as expected. */
-void grpc_run_bad_client_test(const char *name, const char *client_payload,
+void grpc_run_bad_client_test(grpc_bad_client_server_side_validator validator,
+	                            const char *client_payload,
                               size_t client_payload_length,
-                              grpc_bad_client_server_side_validator validator);
+                              gpr_uint32 flags);
+
+#define GRPC_RUN_BAD_CLIENT_TEST(validator, payload, flags) \
+    grpc_run_bad_client_test(validator, payload, sizeof(payload)-1, flags)
 
 #endif /* GRPC_TEST_CORE_BAD_CLIENT_BAD_CLIENT_H */

test/core/bad_client/gen_build_json.py

@@ -41,6 +41,7 @@ default_test_options = TestOptions(False)
 # maps test names to options
 BAD_CLIENT_TESTS = {
     'connection_prefix': default_test_options,
+    'initial_settings_frame': default_test_options,
 }
 
 def main():

test/core/bad_client/tests/connection_prefix.c

@@ -45,35 +45,29 @@ static void verifier(grpc_server *server, grpc_completion_queue *cq) {
 int main(int argc, char **argv) {
   grpc_test_init(argc, argv);
 
-  grpc_run_bad_client_test("conpfx_1", "X", 1, verifier);
+  GRPC_RUN_BAD_CLIENT_TEST(verifier, "X", 0);
-  grpc_run_bad_client_test("conpfx_2", "PX", 2, verifier);
+  GRPC_RUN_BAD_CLIENT_TEST(verifier, "PX", 0);
-  grpc_run_bad_client_test("conpfx_3", "PRX", 3, verifier);
+  GRPC_RUN_BAD_CLIENT_TEST(verifier, "PRX", 0);
-  grpc_run_bad_client_test("conpfx_4", "PRIX", 4, verifier);
+  GRPC_RUN_BAD_CLIENT_TEST(verifier, "PRIX", 0);
-  grpc_run_bad_client_test("conpfx_5", "PRI X", 5, verifier);
+  GRPC_RUN_BAD_CLIENT_TEST(verifier, "PRI X", 0);
-  grpc_run_bad_client_test("conpfx_6", "PRI *X", 6, verifier);
+  GRPC_RUN_BAD_CLIENT_TEST(verifier, "PRI *X", 0);
-  grpc_run_bad_client_test("conpfx_7", "PRI * X", 7, verifier);
+  GRPC_RUN_BAD_CLIENT_TEST(verifier, "PRI * X", 0);
-  grpc_run_bad_client_test("conpfx_8", "PRI * HX", 8, verifier);
+  GRPC_RUN_BAD_CLIENT_TEST(verifier, "PRI * HX", 0);
-  grpc_run_bad_client_test("conpfx_9", "PRI * HTX", 9, verifier);
+  GRPC_RUN_BAD_CLIENT_TEST(verifier, "PRI * HTX", 0);
-  grpc_run_bad_client_test("conpfx_10", "PRI * HTTX", 10, verifier);
+  GRPC_RUN_BAD_CLIENT_TEST(verifier, "PRI * HTTX", 0);
-  grpc_run_bad_client_test("conpfx_11", "PRI * HTTPX", 11, verifier);
+  GRPC_RUN_BAD_CLIENT_TEST(verifier, "PRI * HTTPX", 0);
-  grpc_run_bad_client_test("conpfx_12", "PRI * HTTP/X", 12, verifier);
+  GRPC_RUN_BAD_CLIENT_TEST(verifier, "PRI * HTTP/X", 0);
-  grpc_run_bad_client_test("conpfx_13", "PRI * HTTP/2X", 13, verifier);
+  GRPC_RUN_BAD_CLIENT_TEST(verifier, "PRI * HTTP/2X", 0);
-  grpc_run_bad_client_test("conpfx_14", "PRI * HTTP/2.X", 14, verifier);
+  GRPC_RUN_BAD_CLIENT_TEST(verifier, "PRI * HTTP/2.X", 0);
-  grpc_run_bad_client_test("conpfx_15", "PRI * HTTP/2.0X", 15, verifier);
+  GRPC_RUN_BAD_CLIENT_TEST(verifier, "PRI * HTTP/2.0X", 0);
-  grpc_run_bad_client_test("conpfx_16", "PRI * HTTP/2.0\rX", 16, verifier);
+  GRPC_RUN_BAD_CLIENT_TEST(verifier, "PRI * HTTP/2.0\rX", 0);
-  grpc_run_bad_client_test("conpfx_17", "PRI * HTTP/2.0\r\nX", 17, verifier);
+  GRPC_RUN_BAD_CLIENT_TEST(verifier, "PRI * HTTP/2.0\r\nX", 0);
-  grpc_run_bad_client_test("conpfx_18", "PRI * HTTP/2.0\r\n\rX", 18, verifier);
+  GRPC_RUN_BAD_CLIENT_TEST(verifier, "PRI * HTTP/2.0\r\n\rX", 0);
-  grpc_run_bad_client_test("conpfx_19", "PRI * HTTP/2.0\r\n\r\nX", 19,
+  GRPC_RUN_BAD_CLIENT_TEST(verifier, "PRI * HTTP/2.0\r\n\r\nX", 0);
-                           verifier);
+  GRPC_RUN_BAD_CLIENT_TEST(verifier, "PRI * HTTP/2.0\r\n\r\nSX", 0);
-  grpc_run_bad_client_test("conpfx_20", "PRI * HTTP/2.0\r\n\r\nSX", 20,
+  GRPC_RUN_BAD_CLIENT_TEST(verifier, "PRI * HTTP/2.0\r\n\r\nSMX", 0);
-                           verifier);
+  GRPC_RUN_BAD_CLIENT_TEST(verifier, "PRI * HTTP/2.0\r\n\r\nSM\rX", 0);
-  grpc_run_bad_client_test("conpfx_21", "PRI * HTTP/2.0\r\n\r\nSMX", 21,
+  GRPC_RUN_BAD_CLIENT_TEST(verifier, "PRI * HTTP/2.0\r\n\r\nSM\r\nX", 0);
-                           verifier);
+  GRPC_RUN_BAD_CLIENT_TEST(verifier, "PRI * HTTP/2.0\r\n\r\nSM\r\n\rX", 0);
-  grpc_run_bad_client_test("conpfx_22", "PRI * HTTP/2.0\r\n\r\nSM\rX", 22,
-                           verifier);
-  grpc_run_bad_client_test("conpfx_23", "PRI * HTTP/2.0\r\n\r\nSM\r\nX", 23,
-                           verifier);
-  grpc_run_bad_client_test("conpfx_24", "PRI * HTTP/2.0\r\n\r\nSM\r\n\rX", 24,
-                           verifier);
   return 0;
 }

test/core/bad_client/tests/initial_settings_frame.c

@@ -0,0 +1,75 @@
+/*
+ *
+ * Copyright 2015, Google Inc.
+ * All rights reserved.
+ *
+ * Redistribution and use in source and binary forms, with or without
+ * modification, are permitted provided that the following conditions are
+ * met:
+ *
+ *     * Redistributions of source code must retain the above copyright
+ * notice, this list of conditions and the following disclaimer.
+ *     * Redistributions in binary form must reproduce the above
+ * copyright notice, this list of conditions and the following disclaimer
+ * in the documentation and/or other materials provided with the
+ * distribution.
+ *     * Neither the name of Google Inc. nor the names of its
+ * contributors may be used to endorse or promote products derived from
+ * this software without specific prior written permission.
+ *
+ * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS
+ * "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT
+ * LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR
+ * A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT
+ * OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL,
+ * SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT
+ * LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE,
+ * DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY
+ * THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
+ * (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE
+ * OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
+ *
+ */
+
+#include "test/core/bad_client/bad_client.h"
+#include "src/core/surface/server.h"
+
+#define PFX_STR "PRI * HTTP/2.0\r\n\r\nSM\r\n\r\n"
+
+static void verifier(grpc_server *server, grpc_completion_queue *cq) {
+  while (grpc_server_has_open_connections(server)) {
+    GPR_ASSERT(grpc_completion_queue_next(
+                   cq, GRPC_TIMEOUT_MILLIS_TO_DEADLINE(20)).type ==
+               GRPC_QUEUE_TIMEOUT);
+  }
+}
+
+int main(int argc, char **argv) {
+  grpc_test_init(argc, argv);
+
+  /* various partial prefixes */
+  GRPC_RUN_BAD_CLIENT_TEST(verifier, PFX_STR "\x00", GRPC_BAD_CLIENT_DISCONNECT);
+  GRPC_RUN_BAD_CLIENT_TEST(verifier, PFX_STR "\x00\x00", GRPC_BAD_CLIENT_DISCONNECT);
+  GRPC_RUN_BAD_CLIENT_TEST(verifier, PFX_STR "\x00\x00\x00", GRPC_BAD_CLIENT_DISCONNECT);
+  GRPC_RUN_BAD_CLIENT_TEST(verifier, PFX_STR "\x06", GRPC_BAD_CLIENT_DISCONNECT);
+  GRPC_RUN_BAD_CLIENT_TEST(verifier, PFX_STR "\x00\x06", GRPC_BAD_CLIENT_DISCONNECT);
+  GRPC_RUN_BAD_CLIENT_TEST(verifier, PFX_STR "\x00\x00\x06", GRPC_BAD_CLIENT_DISCONNECT);
+  GRPC_RUN_BAD_CLIENT_TEST(verifier, PFX_STR "\x00\x00\x00\x04", GRPC_BAD_CLIENT_DISCONNECT);
+  GRPC_RUN_BAD_CLIENT_TEST(verifier, PFX_STR "\x00\x00\x00\x04\x00", GRPC_BAD_CLIENT_DISCONNECT);
+  GRPC_RUN_BAD_CLIENT_TEST(verifier, PFX_STR "\x00\x00\x00\x04\x01", GRPC_BAD_CLIENT_DISCONNECT);
+  GRPC_RUN_BAD_CLIENT_TEST(verifier, PFX_STR "\x00\x00\x00\x04\xff", GRPC_BAD_CLIENT_DISCONNECT);
+  GRPC_RUN_BAD_CLIENT_TEST(verifier, PFX_STR "\x00\x00\x00\x04\x00\x00", GRPC_BAD_CLIENT_DISCONNECT);
+  GRPC_RUN_BAD_CLIENT_TEST(verifier, PFX_STR "\x00\x00\x00\x04\x00\x00\x00", GRPC_BAD_CLIENT_DISCONNECT);
+  GRPC_RUN_BAD_CLIENT_TEST(verifier, PFX_STR "\x00\x00\x00\x04\x00\x00\x00\x00", GRPC_BAD_CLIENT_DISCONNECT);
+  /* must not send frames with stream id != 0 */
+  GRPC_RUN_BAD_CLIENT_TEST(verifier, PFX_STR "\x00\x00\x00\x04\x00\x00\x00\x00\x01", 0);
+  GRPC_RUN_BAD_CLIENT_TEST(verifier, PFX_STR "\x00\x00\x00\x04\x00\x40\x00\x00\x00", 0);
+  /* settings frame must be a multiple of six bytes long */
+  GRPC_RUN_BAD_CLIENT_TEST(verifier, PFX_STR "\x00\x00\x01\x04\x00\x00\x00\x00\x00", 0);
+  GRPC_RUN_BAD_CLIENT_TEST(verifier, PFX_STR "\x00\x00\x02\x04\x00\x00\x00\x00\x00", 0);
+  GRPC_RUN_BAD_CLIENT_TEST(verifier, PFX_STR "\x00\x00\x03\x04\x00\x00\x00\x00\x00", 0);
+  GRPC_RUN_BAD_CLIENT_TEST(verifier, PFX_STR "\x00\x00\x04\x04\x00\x00\x00\x00\x00", 0);
+  GRPC_RUN_BAD_CLIENT_TEST(verifier, PFX_STR "\x00\x00\x05\x04\x00\x00\x00\x00\x00", 0);
+
+  return 0;
+}

tools/run_tests/tests.json

@@ -4673,6 +4673,15 @@
       "windows", 
       "posix"
     ]
+  }, 
+  {
+    "flaky": false, 
+    "language": "c", 
+    "name": "initial_settings_frame_bad_client_test", 
+    "platforms": [
+      "windows", 
+      "posix"
+    ]
   }
 ]