From 3b1e37d16df885474b5c1b7cc2728bfe6ab3a62e Mon Sep 17 00:00:00 2001 From: Craig Tiller Date: Thu, 30 Apr 2015 09:12:31 -0700 Subject: [PATCH] Fix errant empty data frame after trailers With the metadata batching changes recently, I introduced a bug whereby the http2 stream encoder would end up not being able to set the close bit on a header frame, and instead placed it on a new data frame, which is illegal by the HTTP2 spec. --- src/core/transport/chttp2/stream_encoder.c | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/src/core/transport/chttp2/stream_encoder.c b/src/core/transport/chttp2/stream_encoder.c index cf1e66bf8be..cf78ac50cc5 100644 --- a/src/core/transport/chttp2/stream_encoder.c +++ b/src/core/transport/chttp2/stream_encoder.c @@ -122,6 +122,12 @@ static void begin_frame(framer_state *st, frame_type type) { st->output_length_at_start_of_frame = st->output->length; } +static void begin_new_frame(framer_state *st, frame_type type) { + finish_frame(st, 1, 0); + st->last_was_header = 0; + begin_frame(st, type); +} + /* make sure that the current frame is of the type desired, and has sufficient space to add at least about_to_add bytes -- finishes the current frame if needed */ @@ -571,6 +577,7 @@ void grpc_chttp2_encode(grpc_stream_op *ops, size_t ops_count, int eof, a metadata element that needs to be unreffed back into the metadata slot. THIS MAY NOT BE THE SAME ELEMENT (if a decoder table slot got updated). After this loop, we'll do a batch unref of elements. */ + begin_new_frame(&st, HEADER); need_unref |= op->data.metadata.garbage.head != NULL; grpc_metadata_batch_assert_ok(&op->data.metadata); for (l = op->data.metadata.list.head; l; l = l->next) { @@ -580,9 +587,6 @@ void grpc_chttp2_encode(grpc_stream_op *ops, size_t ops_count, int eof, if (gpr_time_cmp(op->data.metadata.deadline, gpr_inf_future) != 0) { deadline_enc(compressor, op->data.metadata.deadline, &st); } - ensure_frame_type(&st, HEADER, 0); - finish_frame(&st, 1, 0); - st.last_was_header = 0; /* force a new header frame */ curop++; break; case GRPC_OP_SLICE: