From 93ba75b60254db898e849e4f6bd11a9ff46a8cd4 Mon Sep 17 00:00:00 2001
From: Tatsuhiro Tsujikawa <tatsuhiro.t@gmail.com>
Date: Fri, 25 Sep 2015 22:28:03 +0900
Subject: [PATCH] Fix bug that headers in CONTINUATION were ignored after
 HEADERS with padding

---
 lib/nghttp2_frame.c          |  7 +++++
 lib/nghttp2_frame.h          |  3 +-
 tests/nghttp2_session_test.c | 57 ++++++++++++++++++++++++++++++++++++
 3 files changed, 66 insertions(+), 1 deletion(-)

diff --git a/lib/nghttp2_frame.c b/lib/nghttp2_frame.c
index de491205..e324b9c9 100644
--- a/lib/nghttp2_frame.c
+++ b/lib/nghttp2_frame.c
@@ -165,6 +165,13 @@ void nghttp2_frame_window_update_init(nghttp2_window_update *frame,
 void nghttp2_frame_window_update_free(nghttp2_window_update *frame _U_) {}
 
 size_t nghttp2_frame_trail_padlen(nghttp2_frame *frame, size_t padlen) {
+  /* We have iframe->padlen == 0, but iframe->frame.hd.flags may have
+     NGHTTP2_FLAG_PADDED set.  This happens when receiving
+     CONTINUATION frame, since we don't reset flags after HEADERS was
+     received. */
+  if (padlen == 0) {
+    return 0;
+  }
   return padlen - ((frame->hd.flags & NGHTTP2_FLAG_PADDED) > 0);
 }
 
diff --git a/lib/nghttp2_frame.h b/lib/nghttp2_frame.h
index 8a420c56..b0b02ee1 100644
--- a/lib/nghttp2_frame.h
+++ b/lib/nghttp2_frame.h
@@ -442,7 +442,8 @@ void nghttp2_frame_window_update_free(nghttp2_window_update *frame);
 /*
  * Returns the number of padding bytes after payload.  The total
  * padding length is given in the |padlen|.  The returned value does
- * not include the Pad Length field.
+ * not include the Pad Length field.  If |padlen| is 0, this function
+ * returns 0, regardless of frame->hd.flags.
  */
 size_t nghttp2_frame_trail_padlen(nghttp2_frame *frame, size_t padlen);
 
diff --git a/tests/nghttp2_session_test.c b/tests/nghttp2_session_test.c
index aeb5f495..31423d1c 100644
--- a/tests/nghttp2_session_test.c
+++ b/tests/nghttp2_session_test.c
@@ -964,6 +964,63 @@ void test_nghttp2_session_recv_continuation(void) {
   nghttp2_hd_deflate_free(&deflater);
   nghttp2_session_del(session);
 
+  /* HEADERS with padding followed by CONTINUATION */
+  nghttp2_session_server_new(&session, &callbacks, &ud);
+
+  nghttp2_hd_deflate_init(&deflater, mem);
+
+  nvlen = ARRLEN(reqnv);
+  nghttp2_nv_array_copy(&nva, reqnv, nvlen, mem);
+  nghttp2_frame_headers_init(&frame.headers, NGHTTP2_FLAG_NONE, 1,
+                             NGHTTP2_HCAT_HEADERS, NULL, nva, nvlen);
+
+  nghttp2_bufs_reset(&bufs);
+  rv = nghttp2_frame_pack_headers(&bufs, &frame.headers, &deflater);
+
+  CU_ASSERT(0 == rv);
+
+  nghttp2_frame_headers_free(&frame.headers, mem);
+
+  /* make sure that all data is in the first buf */
+  buf = &bufs.head->buf;
+  assert(nghttp2_bufs_len(&bufs) == nghttp2_buf_len(buf));
+
+  /* HEADERS payload is 3 byte (1 for padding field, 1 for padding) */
+  memcpy(data, buf->pos, NGHTTP2_FRAME_HDLEN);
+  nghttp2_put_uint32be(data, (uint32_t)((3 << 8) + data[3]));
+  data[4] |= NGHTTP2_FLAG_PADDED;
+  /* padding field */
+  data[NGHTTP2_FRAME_HDLEN] = 1;
+  data[NGHTTP2_FRAME_HDLEN + 1] = buf->pos[NGHTTP2_FRAME_HDLEN];
+  /* padding */
+  data[NGHTTP2_FRAME_HDLEN + 2] = 0;
+  datalen = NGHTTP2_FRAME_HDLEN + 3;
+  buf->pos += NGHTTP2_FRAME_HDLEN + 1;
+
+  /* CONTINUATION, rest of the bytes */
+  nghttp2_frame_hd_init(&cont_hd, nghttp2_buf_len(buf), NGHTTP2_CONTINUATION,
+                        NGHTTP2_FLAG_END_HEADERS, 1);
+  nghttp2_frame_pack_frame_hd(data + datalen, &cont_hd);
+  datalen += NGHTTP2_FRAME_HDLEN;
+
+  memcpy(data + datalen, buf->pos, cont_hd.length);
+  datalen += cont_hd.length;
+  buf->pos += cont_hd.length;
+
+  CU_ASSERT(0 == nghttp2_buf_len(buf));
+
+  ud.header_cb_called = 0;
+  ud.begin_frame_cb_called = 0;
+
+  rv = nghttp2_session_mem_recv(session, data, datalen);
+
+  CU_ASSERT((ssize_t)datalen == rv);
+  CU_ASSERT(4 == ud.header_cb_called);
+  CU_ASSERT(2 == ud.begin_frame_cb_called);
+
+  nghttp2_hd_deflate_free(&deflater);
+  nghttp2_session_del(session);
+
   /* Expecting CONTINUATION, but get the other frame */
   nghttp2_session_server_new(&session, &callbacks, &ud);