Merge github.com:grpc/grpc into varint_test

9 files changed, 45 insertions, 78 deletions

diff --git a/src/core/transport/chttp2/frame_data.c b/src/core/transport/chttp2/frame_data.c
index e07fbb2cc7..38fa990758 100644
--- a/src/core/transport/chttp2/frame_data.c
+++ b/src/core/transport/chttp2/frame_data.c
@@ -118,7 +118,7 @@ void grpc_chttp2_encode_data(gpr_uint32 id, gpr_slice_buffer *inbuf,
 
   hdr = gpr_slice_malloc(9);
   p = GPR_SLICE_START_PTR(hdr);
-  GPR_ASSERT(write_bytes < 16777316);
+  GPR_ASSERT(write_bytes < (1<<24));
   *p++ = (gpr_uint8)(write_bytes >> 16);
   *p++ = (gpr_uint8)(write_bytes >> 8);
   *p++ = (gpr_uint8)(write_bytes);
diff --git a/src/core/transport/chttp2/frame_data.h b/src/core/transport/chttp2/frame_data.h
index 472f9cebdb..27d4d0043b 100644
--- a/src/core/transport/chttp2/frame_data.h
+++ b/src/core/transport/chttp2/frame_data.h
@@ -94,9 +94,6 @@ grpc_chttp2_parse_error grpc_chttp2_data_parser_parse(
     grpc_chttp2_transport_parsing *transport_parsing,
     grpc_chttp2_stream_parsing *stream_parsing, gpr_slice slice, int is_last);
 
-/* create a slice with an empty data frame and is_last set */
-gpr_slice grpc_chttp2_data_frame_create_empty_close(gpr_uint32 id);
-
 void grpc_chttp2_encode_data(gpr_uint32 id, gpr_slice_buffer *inbuf,
                              gpr_uint32 write_bytes, int is_eof,
                              gpr_slice_buffer *outbuf);
diff --git a/src/core/transport/chttp2/hpack_encoder.c b/src/core/transport/chttp2/hpack_encoder.c
index 28d5433d15..7575031e58 100644
--- a/src/core/transport/chttp2/hpack_encoder.c
+++ b/src/core/transport/chttp2/hpack_encoder.c
@@ -458,12 +458,6 @@ static void deadline_enc(grpc_chttp2_hpack_compressor *c, gpr_timespec deadline,
   GRPC_MDELEM_UNREF(mdelem);
 }
 
-gpr_slice grpc_chttp2_data_frame_create_empty_close(gpr_uint32 id) {
-  gpr_slice slice = gpr_slice_malloc(9);
-  fill_header(GPR_SLICE_START_PTR(slice), GRPC_CHTTP2_FRAME_DATA, id, 0, 1);
-  return slice;
-}
-
 static gpr_uint32 elems_for_bytes(gpr_uint32 bytes) {
   return (bytes + 31) / 32;
 }
diff --git a/src/core/transport/chttp2/hpack_parser.c b/src/core/transport/chttp2/hpack_parser.c
index d38ff68754..30f0d469e3 100644
--- a/src/core/transport/chttp2/hpack_parser.c
+++ b/src/core/transport/chttp2/hpack_parser.c
@@ -728,6 +728,7 @@ static int finish_indexed_field(grpc_chttp2_hpack_parser *p,
 /* parse an indexed field with index < 127 */
 static int parse_indexed_field(grpc_chttp2_hpack_parser *p,
                                const gpr_uint8 *cur, const gpr_uint8 *end) {
+  p->dynamic_table_update_allowed = 0;
   p->index = (*cur) & 0x7f;
   return finish_indexed_field(p, cur + 1, end);
 }
@@ -737,6 +738,7 @@ static int parse_indexed_field_x(grpc_chttp2_hpack_parser *p,
                                  const gpr_uint8 *cur, const gpr_uint8 *end) {
   static const grpc_chttp2_hpack_parser_state and_then[] = {
       finish_indexed_field};
+  p->dynamic_table_update_allowed = 0;
   p->next_state = and_then;
   p->index = 0x7f;
   p->parsing.value = &p->index;
@@ -748,6 +750,7 @@ static int parse_indexed_field_x(grpc_chttp2_hpack_parser *p,
 static int finish_lithdr_incidx(grpc_chttp2_hpack_parser *p,
                                 const gpr_uint8 *cur, const gpr_uint8 *end) {
   grpc_mdelem *md = grpc_chttp2_hptbl_lookup(&p->table, p->index);
+  GPR_ASSERT(md != NULL); /* handled in string parsing */
   return on_hdr(p, grpc_mdelem_from_metadata_strings(GRPC_MDSTR_REF(md->key),
                                                      take_string(p, &p->value)),
                 1) &&
@@ -768,6 +771,7 @@ static int parse_lithdr_incidx(grpc_chttp2_hpack_parser *p,
                                const gpr_uint8 *cur, const gpr_uint8 *end) {
   static const grpc_chttp2_hpack_parser_state and_then[] = {
       parse_value_string_with_indexed_key, finish_lithdr_incidx};
+  p->dynamic_table_update_allowed = 0;
   p->next_state = and_then;
   p->index = (*cur) & 0x3f;
   return parse_string_prefix(p, cur + 1, end);
@@ -779,6 +783,7 @@ static int parse_lithdr_incidx_x(grpc_chttp2_hpack_parser *p,
   static const grpc_chttp2_hpack_parser_state and_then[] = {
       parse_string_prefix, parse_value_string_with_indexed_key,
       finish_lithdr_incidx};
+  p->dynamic_table_update_allowed = 0;
   p->next_state = and_then;
   p->index = 0x3f;
   p->parsing.value = &p->index;
@@ -791,6 +796,7 @@ static int parse_lithdr_incidx_v(grpc_chttp2_hpack_parser *p,
   static const grpc_chttp2_hpack_parser_state and_then[] = {
       parse_key_string, parse_string_prefix,
       parse_value_string_with_literal_key, finish_lithdr_incidx_v};
+  p->dynamic_table_update_allowed = 0;
   p->next_state = and_then;
   return parse_string_prefix(p, cur + 1, end);
 }
@@ -799,6 +805,7 @@ static int parse_lithdr_incidx_v(grpc_chttp2_hpack_parser *p,
 static int finish_lithdr_notidx(grpc_chttp2_hpack_parser *p,
                                 const gpr_uint8 *cur, const gpr_uint8 *end) {
   grpc_mdelem *md = grpc_chttp2_hptbl_lookup(&p->table, p->index);
+  GPR_ASSERT(md != NULL); /* handled in string parsing */
   return on_hdr(p, grpc_mdelem_from_metadata_strings(GRPC_MDSTR_REF(md->key),
                                                      take_string(p, &p->value)),
                 0) &&
@@ -819,6 +826,7 @@ static int parse_lithdr_notidx(grpc_chttp2_hpack_parser *p,
                                const gpr_uint8 *cur, const gpr_uint8 *end) {
   static const grpc_chttp2_hpack_parser_state and_then[] = {
       parse_value_string_with_indexed_key, finish_lithdr_notidx};
+  p->dynamic_table_update_allowed = 0;
   p->next_state = and_then;
   p->index = (*cur) & 0xf;
   return parse_string_prefix(p, cur + 1, end);
@@ -830,6 +838,7 @@ static int parse_lithdr_notidx_x(grpc_chttp2_hpack_parser *p,
   static const grpc_chttp2_hpack_parser_state and_then[] = {
       parse_string_prefix, parse_value_string_with_indexed_key,
       finish_lithdr_notidx};
+  p->dynamic_table_update_allowed = 0;
   p->next_state = and_then;
   p->index = 0xf;
   p->parsing.value = &p->index;
@@ -842,6 +851,7 @@ static int parse_lithdr_notidx_v(grpc_chttp2_hpack_parser *p,
   static const grpc_chttp2_hpack_parser_state and_then[] = {
       parse_key_string, parse_string_prefix,
       parse_value_string_with_literal_key, finish_lithdr_notidx_v};
+  p->dynamic_table_update_allowed = 0;
   p->next_state = and_then;
   return parse_string_prefix(p, cur + 1, end);
 }
@@ -850,6 +860,7 @@ static int parse_lithdr_notidx_v(grpc_chttp2_hpack_parser *p,
 static int finish_lithdr_nvridx(grpc_chttp2_hpack_parser *p,
                                 const gpr_uint8 *cur, const gpr_uint8 *end) {
   grpc_mdelem *md = grpc_chttp2_hptbl_lookup(&p->table, p->index);
+  GPR_ASSERT(md != NULL); /* handled in string parsing */
   return on_hdr(p, grpc_mdelem_from_metadata_strings(GRPC_MDSTR_REF(md->key),
                                                      take_string(p, &p->value)),
                 0) &&
@@ -870,6 +881,7 @@ static int parse_lithdr_nvridx(grpc_chttp2_hpack_parser *p,
                                const gpr_uint8 *cur, const gpr_uint8 *end) {
   static const grpc_chttp2_hpack_parser_state and_then[] = {
       parse_value_string_with_indexed_key, finish_lithdr_nvridx};
+  p->dynamic_table_update_allowed = 0;
   p->next_state = and_then;
   p->index = (*cur) & 0xf;
   return parse_string_prefix(p, cur + 1, end);
@@ -881,6 +893,7 @@ static int parse_lithdr_nvridx_x(grpc_chttp2_hpack_parser *p,
   static const grpc_chttp2_hpack_parser_state and_then[] = {
       parse_string_prefix, parse_value_string_with_indexed_key,
       finish_lithdr_nvridx};
+  p->dynamic_table_update_allowed = 0;
   p->next_state = and_then;
   p->index = 0xf;
   p->parsing.value = &p->index;
@@ -893,6 +906,7 @@ static int parse_lithdr_nvridx_v(grpc_chttp2_hpack_parser *p,
   static const grpc_chttp2_hpack_parser_state and_then[] = {
       parse_key_string, parse_string_prefix,
       parse_value_string_with_literal_key, finish_lithdr_nvridx_v};
+  p->dynamic_table_update_allowed = 0;
   p->next_state = and_then;
   return parse_string_prefix(p, cur + 1, end);
 }
@@ -908,6 +922,10 @@ static int finish_max_tbl_size(grpc_chttp2_hpack_parser *p,
 /* parse a max table size change, max size < 15 */
 static int parse_max_tbl_size(grpc_chttp2_hpack_parser *p, const gpr_uint8 *cur,
                               const gpr_uint8 *end) {
+  if (p->dynamic_table_update_allowed == 0) {
+    return 0;
+  }
+  p->dynamic_table_update_allowed--;
   p->index = (*cur) & 0x1f;
   return finish_max_tbl_size(p, cur + 1, end);
 }
@@ -917,6 +935,10 @@ static int parse_max_tbl_size_x(grpc_chttp2_hpack_parser *p,
                                 const gpr_uint8 *cur, const gpr_uint8 *end) {
   static const grpc_chttp2_hpack_parser_state and_then[] = {
       finish_max_tbl_size};
+  if (p->dynamic_table_update_allowed == 0) {
+    return 0;
+  }
+  p->dynamic_table_update_allowed--;
   p->next_state = and_then;
   p->index = 0x1f;
   p->parsing.value = &p->index;
@@ -1300,7 +1322,10 @@ static is_binary_header is_binary_literal_header(grpc_chttp2_hpack_parser *p) {
 
 static is_binary_header is_binary_indexed_header(grpc_chttp2_hpack_parser *p) {
   grpc_mdelem *elem = grpc_chttp2_hptbl_lookup(&p->table, p->index);
-  if (!elem) return ERROR_HEADER;
+  if (!elem) {
+    gpr_log(GPR_ERROR, "Invalid HPACK index received: %d", p->index);
+    return ERROR_HEADER;
+  }
   return grpc_is_binary_header(
              (const char *)GPR_SLICE_START_PTR(elem->key->slice),
              GPR_SLICE_LENGTH(elem->key->slice))
@@ -1338,15 +1363,7 @@ static int parse_value_string_with_literal_key(grpc_chttp2_hpack_parser *p,
 /* PUBLIC INTERFACE */
 
 static void on_header_not_set(void *user_data, grpc_mdelem *md) {
-  char *keyhex = gpr_dump_slice(md->key->slice, GPR_DUMP_HEX | GPR_DUMP_ASCII);
-  char *valuehex =
-      gpr_dump_slice(md->value->slice, GPR_DUMP_HEX | GPR_DUMP_ASCII);
-  gpr_log(GPR_ERROR, "on_header callback not set; key=%s value=%s", keyhex,
-          valuehex);
-  gpr_free(keyhex);
-  gpr_free(valuehex);
-  GRPC_MDELEM_UNREF(md);
-  abort();
+  GPR_UNREACHABLE_CODE(return );
 }
 
 void grpc_chttp2_hpack_parser_init(grpc_chttp2_hpack_parser *p) {
@@ -1359,6 +1376,7 @@ void grpc_chttp2_hpack_parser_init(grpc_chttp2_hpack_parser *p) {
   p->value.str = NULL;
   p->value.capacity = 0;
   p->value.length = 0;
+  p->dynamic_table_update_allowed = 2;
   grpc_chttp2_hptbl_init(&p->table);
 }
 
@@ -1400,20 +1418,25 @@ grpc_chttp2_parse_error grpc_chttp2_header_parser_parse(
       GPR_TIMER_END("grpc_chttp2_hpack_parser_parse", 0);
       return GRPC_CHTTP2_CONNECTION_ERROR;
     }
-    if (parser->is_boundary) {
-      stream_parsing
-          ->got_metadata_on_parse[stream_parsing->header_frames_received] = 1;
-      stream_parsing->header_frames_received++;
-      grpc_chttp2_list_add_parsing_seen_stream(transport_parsing,
-                                               stream_parsing);
-    }
-    if (parser->is_eof) {
-      stream_parsing->received_close = 1;
+    /* need to check for null stream: this can occur if we receive an invalid
+       stream id on a header */
+    if (stream_parsing != NULL) {
+      if (parser->is_boundary) {
+        stream_parsing
+            ->got_metadata_on_parse[stream_parsing->header_frames_received] = 1;
+        stream_parsing->header_frames_received++;
+        grpc_chttp2_list_add_parsing_seen_stream(transport_parsing,
+                                                 stream_parsing);
+      }
+      if (parser->is_eof) {
+        stream_parsing->received_close = 1;
+      }
     }
     parser->on_header = on_header_not_set;
     parser->on_header_user_data = NULL;
     parser->is_boundary = 0xde;
     parser->is_eof = 0xde;
+    parser->dynamic_table_update_allowed = 2;
   }
   GPR_TIMER_END("grpc_chttp2_hpack_parser_parse", 0);
   return GRPC_CHTTP2_PARSE_OK;
diff --git a/src/core/transport/chttp2/hpack_parser.h b/src/core/transport/chttp2/hpack_parser.h
index fb894b5735..bd36357124 100644
--- a/src/core/transport/chttp2/hpack_parser.h
+++ b/src/core/transport/chttp2/hpack_parser.h
@@ -85,6 +85,8 @@ struct grpc_chttp2_hpack_parser {
   gpr_uint8 binary;
   /* is the current string huffman encoded? */
   gpr_uint8 huff;
+  /* is a dynamic table update allowed? */
+  gpr_uint8 dynamic_table_update_allowed;
   /* set by higher layers, used by grpc_chttp2_header_parser_parse to signal
      it should append a metadata boundary at the end of frame */
   gpr_uint8 is_boundary;
diff --git a/src/core/transport/chttp2/incoming_metadata.c b/src/core/transport/chttp2/incoming_metadata.c
index 956afc8e9d..315bc2faa1 100644
--- a/src/core/transport/chttp2/incoming_metadata.c
+++ b/src/core/transport/chttp2/incoming_metadata.c
@@ -56,16 +56,6 @@ void grpc_chttp2_incoming_metadata_buffer_destroy(
   gpr_free(buffer->elems);
 }
 
-void grpc_chttp2_incoming_metadata_buffer_reset(
-    grpc_chttp2_incoming_metadata_buffer *buffer) {
-  size_t i;
-  GPR_ASSERT(!buffer->published);
-  for (i = 0; i < buffer->count; i++) {
-    GRPC_MDELEM_UNREF(buffer->elems[i].md);
-  }
-  buffer->count = 0;
-}
-
 void grpc_chttp2_incoming_metadata_buffer_add(
     grpc_chttp2_incoming_metadata_buffer *buffer, grpc_mdelem *elem) {
   GPR_ASSERT(!buffer->published);
@@ -83,14 +73,6 @@ void grpc_chttp2_incoming_metadata_buffer_set_deadline(
   buffer->deadline = deadline;
 }
 
-void grpc_chttp2_incoming_metadata_buffer_swap(
-    grpc_chttp2_incoming_metadata_buffer *a,
-    grpc_chttp2_incoming_metadata_buffer *b) {
-  GPR_ASSERT(!a->published);
-  GPR_ASSERT(!b->published);
-  GPR_SWAP(grpc_chttp2_incoming_metadata_buffer, *a, *b);
-}
-
 void grpc_chttp2_incoming_metadata_buffer_publish(
     grpc_chttp2_incoming_metadata_buffer *buffer, grpc_metadata_batch *batch) {
   GPR_ASSERT(!buffer->published);
diff --git a/src/core/transport/chttp2/incoming_metadata.h b/src/core/transport/chttp2/incoming_metadata.h
index 0e1dabe825..ea74cfc64b 100644
--- a/src/core/transport/chttp2/incoming_metadata.h
+++ b/src/core/transport/chttp2/incoming_metadata.h
@@ -49,8 +49,6 @@ void grpc_chttp2_incoming_metadata_buffer_init(
     grpc_chttp2_incoming_metadata_buffer *buffer);
 void grpc_chttp2_incoming_metadata_buffer_destroy(
     grpc_chttp2_incoming_metadata_buffer *buffer);
-void grpc_chttp2_incoming_metadata_buffer_reset(
-    grpc_chttp2_incoming_metadata_buffer *buffer);
 void grpc_chttp2_incoming_metadata_buffer_publish(
     grpc_chttp2_incoming_metadata_buffer *buffer, grpc_metadata_batch *batch);
 
diff --git a/src/core/transport/chttp2/internal.h b/src/core/transport/chttp2/internal.h
index 3952f8a0e8..45d2599cdc 100644
--- a/src/core/transport/chttp2/internal.h
+++ b/src/core/transport/chttp2/internal.h
@@ -512,9 +512,6 @@ void grpc_chttp2_publish_reads(grpc_exec_ctx *exec_ctx,
 void grpc_chttp2_list_add_writable_stream(
     grpc_chttp2_transport_global *transport_global,
     grpc_chttp2_stream_global *stream_global);
-void grpc_chttp2_list_add_first_writable_stream(
-    grpc_chttp2_transport_global *transport_global,
-    grpc_chttp2_stream_global *stream_global);
 int grpc_chttp2_list_pop_writable_stream(
     grpc_chttp2_transport_global *transport_global,
     grpc_chttp2_transport_writing *transport_writing,
diff --git a/src/core/transport/chttp2/stream_lists.c b/src/core/transport/chttp2/stream_lists.c
index a81ffcce79..a4c85b4e57 100644
--- a/src/core/transport/chttp2/stream_lists.c
+++ b/src/core/transport/chttp2/stream_lists.c
@@ -108,23 +108,6 @@ static void stream_list_maybe_remove(grpc_chttp2_transport *t,
   }
 }
 
-static void stream_list_add_head(grpc_chttp2_transport *t,
-                                 grpc_chttp2_stream *s,
-                                 grpc_chttp2_stream_list_id id) {
-  grpc_chttp2_stream *old_head;
-  GPR_ASSERT(!s->included[id]);
-  old_head = t->lists[id].head;
-  s->links[id].next = old_head;
-  s->links[id].prev = NULL;
-  if (old_head) {
-    old_head->links[id].prev = s;
-  } else {
-    t->lists[id].tail = s;
-  }
-  t->lists[id].head = s;
-  s->included[id] = 1;
-}
-
 static void stream_list_add_tail(grpc_chttp2_transport *t,
                                  grpc_chttp2_stream *s,
                                  grpc_chttp2_stream_list_id id) {
@@ -161,15 +144,6 @@ void grpc_chttp2_list_add_writable_stream(
                   STREAM_FROM_GLOBAL(stream_global), GRPC_CHTTP2_LIST_WRITABLE);
 }
 
-void grpc_chttp2_list_add_first_writable_stream(
-    grpc_chttp2_transport_global *transport_global,
-    grpc_chttp2_stream_global *stream_global) {
-  GPR_ASSERT(stream_global->id != 0);
-  stream_list_add_head(TRANSPORT_FROM_GLOBAL(transport_global),
-                       STREAM_FROM_GLOBAL(stream_global),
-                       GRPC_CHTTP2_LIST_WRITABLE);
-}
-
 int grpc_chttp2_list_pop_writable_stream(
     grpc_chttp2_transport_global *transport_global,
     grpc_chttp2_transport_writing *transport_writing,