Callback based padding from application

Now previous padding options are removed and instead we added
select_padding_callback to select padding length for each frame
by application. If this callback is not implemented by application,
no padding is added.

This change also fixes the broken session_detect_idle_stream()
if stream_id is our side.

lib/includes/nghttp2/nghttp2.h

@@ -147,14 +147,6 @@ typedef struct {
  */
 #define NGHTTP2_CLIENT_CONNECTION_HEADER_LEN 24
 
-/**
- * @macro
- *
- * The default value of DATA padding alignment. See
- * :member:`NGHTTP2_OPT_PADDING_BOUNDARY`.
- */
-#define NGHTTP2_PADDING_BOUNDARY 64
-
 /**
  * @enum
  *
@@ -1184,6 +1176,26 @@ typedef int (*nghttp2_on_header_callback)
  const uint8_t *value, size_t valuelen,
  void *user_data);
 
+/**
+ * @functypedef
+ *
+ * Callback function invoked when the library asks application how
+ * much padding is required for the transmission of the |frame|. The
+ * application must choose the total length of payload including
+ * padded bytes in range [frame->hd.length, max_payloadlen],
+ * inclusive. Choosing number not in this range will be treated as
+ * :enum:`NGHTTP2_ERR_CALLBACK_FAILURE`. Returning
+ * ``frame->hd.length`` means no padding is added.  Returning
+ * :enum:`NGHTTP2_ERR_CALLBACK_FAILURE` will make
+ * `nghttp2_session_send()` function immediately return
+ * :enum:`NGHTTP2_ERR_CALLBACK_FAILURE`.
+ */
+typedef ssize_t (*nghttp2_select_padding_callback)
+(nghttp2_session *session,
+ const nghttp2_frame *frame,
+ size_t max_payloadlen,
+ void *user_data);
+
 /**
  * @struct
  *
@@ -1247,6 +1259,11 @@ typedef struct {
    * received.
    */
   nghttp2_on_header_callback on_header_callback;
+  /**
+   * Callback function invoked when the library asks application how
+   * much padding is required for the transmission of the given frame.
+   */
+  nghttp2_select_padding_callback select_padding_callback;
 } nghttp2_session_callbacks;
 
 /**
@@ -1332,15 +1349,7 @@ typedef enum {
    * will be overwritten if the local endpoint receives
    * SETTINGS_MAX_CONCURRENT_STREAMS from the remote endpoint.
    */
-  NGHTTP2_OPT_PEER_MAX_CONCURRENT_STREAMS = 1 << 2,
-  /**
-   * This option specifies the alignment of padding in frame
-   * payload. If this option is set to N, padding is added to frame
-   * payload so that its payload length is divisible by N. For DATA
-   * frame, due to flow control, padding is not always added according
-   * to this alignment. Specifying 0 to this option disables padding.
-   */
-  NGHTTP2_OPT_PADDING_BOUNDARY = 1 << 3
+  NGHTTP2_OPT_PEER_MAX_CONCURRENT_STREAMS = 1 << 2
 } nghttp2_opt;
 
 /**
@@ -1361,10 +1370,6 @@ typedef struct {
    * :enum:`NGHTTP2_OPT_NO_AUTO_CONNECTION_WINDOW_UPDATE`
    */
   uint8_t no_auto_connection_window_update;
-  /**
-   * :enum:`NGHTTP2_OPT_PADDING_BOUNDARY`
-   */
-  uint16_t padding_boundary;
 } nghttp2_opt_set;
 
 /**

lib/nghttp2_frame.c

@@ -227,8 +227,7 @@ ssize_t nghttp2_frame_pack_headers(uint8_t **buf_ptr,
                                    size_t *buflen_ptr,
                                    size_t *bufoff_ptr,
                                    nghttp2_headers *frame,
-                                   nghttp2_hd_deflater *deflater,
-                                   size_t boundary)
+                                   nghttp2_hd_deflater *deflater)
 {
   size_t payloadoff = NGHTTP2_FRAME_HEAD_LENGTH + 2;
   size_t nv_offset =
@@ -244,23 +243,9 @@ ssize_t nghttp2_frame_pack_headers(uint8_t **buf_ptr,
 
   payloadlen = nghttp2_frame_headers_payload_nv_offset(frame) + rv;
 
-  if(boundary > 0) {
-    ssize_t padlen;
-    padlen = nghttp2_frame_add_pad(buf_ptr, buflen_ptr, bufoff_ptr,
-                                   &frame->hd.flags,
-                                   payloadlen,
-                                   payloadlen + boundary,
-                                   boundary);
-    if(padlen < 0) {
-      return padlen;
-    }
-    frame->padlen = padlen;
-    frame->hd.length = payloadlen + padlen;
-  } else {
-    *bufoff_ptr = 2;
-    frame->padlen = 0;
-    frame->hd.length = payloadlen;
-  }
+  *bufoff_ptr = 2;
+  frame->padlen = 0;
+  frame->hd.length = payloadlen;
   /* If frame->nvlen == 0, *buflen_ptr may be smaller than
      nv_offset */
   rv = nghttp2_reserve_buffer(buf_ptr, buflen_ptr, nv_offset);
@@ -269,11 +254,11 @@ ssize_t nghttp2_frame_pack_headers(uint8_t **buf_ptr,
   }
   memset(*buf_ptr + *bufoff_ptr, 0, NGHTTP2_FRAME_HEAD_LENGTH);
   /* pack ctrl header after length is determined */
-  if(NGHTTP2_FRAME_HEAD_LENGTH + NGHTTP2_MAX_FRAME_LENGTH < rv + nv_offset) {
+  if(NGHTTP2_MAX_FRAME_LENGTH < payloadlen) {
     /* Needs CONTINUATION */
     nghttp2_frame_hd hd = frame->hd;
-    hd.flags &= ~(NGHTTP2_FLAG_END_HEADERS |
-                  NGHTTP2_FLAG_PAD_HIGH | NGHTTP2_FLAG_PAD_LOW);
+    hd.flags &= ~NGHTTP2_FLAG_END_HEADERS;
+    hd.length = NGHTTP2_MAX_FRAME_LENGTH;
     nghttp2_frame_pack_frame_hd(*buf_ptr + *bufoff_ptr, &hd);
   } else {
     nghttp2_frame_pack_frame_hd(*buf_ptr + *bufoff_ptr, &frame->hd);
@@ -660,36 +645,31 @@ int nghttp2_iv_check(const nghttp2_settings_entry *iv, size_t niv)
   return 1;
 }
 
-ssize_t nghttp2_frame_add_pad(uint8_t **buf_ptr, size_t *buflen_ptr,
-                              size_t *bufoff_ptr,
-                              uint8_t *flags_ptr,
-                              size_t payloadlen,
-                              size_t payloadmax,
-                              size_t boundary)
+int nghttp2_frame_add_pad(uint8_t **buf_ptr, size_t *buflen_ptr,
+                          size_t *bufoff_ptr,
+                          uint8_t *flags_ptr,
+                          size_t payloadlen,
+                          size_t padlen)
 {
   int rv;
-  size_t nextlen =
-    nghttp2_min(((payloadlen == 0 ? 1 : payloadlen) + boundary - 1)
-                / boundary * boundary,
-                payloadmax);
-  size_t padlen = nextlen - payloadlen;
   size_t trail_padlen = 0;
   /* extra 2 bytes for PAD_HIGH and PAD_LOW. */
-  size_t trail_padoff = 2 + NGHTTP2_FRAME_HEAD_LENGTH + payloadlen;
+  size_t trail_padoff = *bufoff_ptr + NGHTTP2_FRAME_HEAD_LENGTH + payloadlen;
 
   if(padlen > 257) {
-    *bufoff_ptr = 0;
+    uint8_t *p;
+    *bufoff_ptr -= 2;
     trail_padlen = padlen - 2;
     *flags_ptr |= NGHTTP2_FLAG_PAD_HIGH | NGHTTP2_FLAG_PAD_LOW;
-    (*buf_ptr)[NGHTTP2_FRAME_HEAD_LENGTH] = trail_padlen >> 8;
-    (*buf_ptr)[NGHTTP2_FRAME_HEAD_LENGTH + 1] = trail_padlen & 0xff;
+    p = *buf_ptr + *bufoff_ptr + NGHTTP2_FRAME_HEAD_LENGTH;
+    *p++ = trail_padlen >> 8;
+    *p = trail_padlen & 0xff;
   } else if(padlen > 0) {
-    *bufoff_ptr = 1;
+    --*bufoff_ptr;
     trail_padlen = padlen - 1;
     *flags_ptr |= NGHTTP2_FLAG_PAD_LOW;
-    (*buf_ptr)[NGHTTP2_FRAME_HEAD_LENGTH + 1] = trail_padlen;
+    (*buf_ptr)[*bufoff_ptr + NGHTTP2_FRAME_HEAD_LENGTH] = trail_padlen;
   } else {
-    *bufoff_ptr = 2;
     return 0;
   }
 
@@ -702,5 +682,5 @@ ssize_t nghttp2_frame_add_pad(uint8_t **buf_ptr, size_t *buflen_ptr,
      possible internal data to the remote peer */
   memset((*buf_ptr) + trail_padoff, 0, trail_padlen);
 
-  return padlen;
+  return 0;
 }

lib/nghttp2_frame.h

@@ -105,10 +105,8 @@ size_t nghttp2_frame_headers_payload_nv_offset(nghttp2_headers *frame);
  * change.  |*buf_ptr| and |*buflen_ptr| are updated accordingly.
  *
  * The first byte the frame is serialized is returned in the
- * |*bufoff_ptr|.
- *
- * The |boundary| is used as padding boundary. If the |boundary| is
- * zero, no padding is added.
+ * |*bufoff_ptr|. Currently, it is always 2 to account for possible
+ * PAD_HIGH and PAD_LOW.
  *
  * frame->hd.length is assigned after length is determined during
  * packing process. If payload length is strictly larger than
@@ -131,8 +129,7 @@ ssize_t nghttp2_frame_pack_headers(uint8_t **buf_ptr,
                                    size_t *buflen_ptr,
                                    size_t *bufoff_ptr,
                                    nghttp2_headers *frame,
-                                   nghttp2_hd_deflater *deflater,
-                                   size_t boundary);
+                                   nghttp2_hd_deflater *deflater);
 
 /*
  * Unpacks HEADERS frame byte sequence into |frame|. This function
@@ -505,11 +502,10 @@ int nghttp2_iv_check(const nghttp2_settings_entry *iv, size_t niv);
 /*
  * Add padding to the payload in the |*buf_ptr| of length
  * |*buflen_ptr|. The payload length is given in |payloadlen|. The
- * payload must start at offset NGHTTP2_FRAME_HEAD_LENGTH + 2 from
- * |*buf_ptr| to account for PAD_HIGH and PAD_LOW. The maximum payload
- * allowed is given in the |payloadmax|. The padding will not be made
- * more than |payloadmax|. The padding boundary is given in the
- * |boundary|.
+ * frame header starts at offset |*bufoff_ptr|. Therefore, the payload
+ * must start at offset *bufoff_ptr + NGHTTP2_FRAME_HEAD_LENGTH from
+ * |*buf_ptr| to account for PAD_HIGH and PAD_LOW. The padding is
+ * given in the |padlen|.
  *
  * The |*flags_ptr| is updated to include NGHTTP2_FLAG_PAD_LOW and
  * NGHTTP2_FLAG_PAD_HIGH based on the padding length. The
@@ -570,18 +566,14 @@ int nghttp2_iv_check(const nghttp2_settings_entry *iv, size_t niv);
  * additional copy operation is required (if the |*buf_ptr| is large
  * enough to account the additional padding, of course).
  *
- * This function returns the number of padding added to the payload
- * including PAD_HIGH and PAD_LOW if it succeeds, or one of the
- * following negative error codes:
+ * This function returns 0 if it succeeds, or one of the following
+ * negative error codes:
  *
  * NGHTTP2_ERR_NOMEM
  *     Out of memory.
  */
-ssize_t nghttp2_frame_add_pad(uint8_t **buf_ptr, size_t *buflen_ptr,
-                              size_t *bufoff_ptr,
-                              uint8_t *flags_ptr,
-                              size_t payloadlen,
-                              size_t payloadmax,
-                              size_t boundary);
+int nghttp2_frame_add_pad(uint8_t **buf_ptr, size_t *buflen_ptr,
+                          size_t *bufoff_ptr, uint8_t *flags_ptr,
+                          size_t payloadlen, size_t padlen);
 
 #endif /* NGHTTP2_FRAME_H */

lib/nghttp2_session.h

@@ -153,8 +153,6 @@ struct nghttp2_session {
   size_t num_incoming_streams;
   /* The number of bytes allocated for nvbuf */
   size_t nvbuflen;
-  /* padding alignemnt. See NGHTTP2_OPT_PADDING_BOUNDARY. */
-  size_t padding_boundary;
   /* Next Stream ID. Made unsigned int to detect >= (1 << 31). */
   uint32_t next_stream_id;
   /* The largest stream ID received so far */

src/HttpServer.cc

@@ -66,7 +66,7 @@ const std::string NGHTTPD_SERVER = "nghttpd nghttp2/" NGHTTP2_VERSION;
 Config::Config()
   : data_ptr(nullptr),
     output_upper_thres(1024*1024),
-    padding_boundary(NGHTTP2_PADDING_BOUNDARY),
+    padding_boundary(0),
     header_table_size(-1),
     port(0),
     verbose(false),
@@ -362,14 +362,8 @@ int Http2Handler::on_connect()
 {
   int r;
   nghttp2_session_callbacks callbacks;
-  nghttp2_opt_set opt_set;
-
-  memset(&opt_set, 0, sizeof(opt_set));
-  opt_set.padding_boundary = sessions_->get_config()->padding_boundary;
-
   fill_callback(callbacks, sessions_->get_config());
-  r = nghttp2_session_server_new2(&session_, &callbacks, this,
-                                  NGHTTP2_OPT_PADDING_BOUNDARY, &opt_set);
+  r = nghttp2_session_server_new(&session_, &callbacks, this);
   if(r != 0) {
     return r;
   }
@@ -931,6 +925,23 @@ int hd_on_frame_send_callback
 }
 } // namespace
 
+namespace {
+ssize_t select_padding_callback
+(nghttp2_session *session, const nghttp2_frame *frame, size_t max_payload,
+ void *user_data)
+{
+  auto hd = static_cast<Http2Handler*>(user_data);
+  auto bd = hd->get_config()->padding_boundary;
+  if(bd == 0) {
+    return frame->hd.length;
+  }
+  if(frame->hd.length == 0) {
+    return bd;
+  }
+  return std::min(max_payload, (frame->hd.length + bd - 1) / bd * bd);
+}
+} // namespace
+
 namespace {
 int on_data_chunk_recv_callback
 (nghttp2_session *session, uint8_t flags, int32_t stream_id,
@@ -977,6 +988,7 @@ void fill_callback(nghttp2_session_callbacks& callbacks, const Config *config)
   callbacks.on_data_chunk_recv_callback = on_data_chunk_recv_callback;
   callbacks.on_header_callback = on_header_callback;
   callbacks.on_begin_headers_callback = on_begin_headers_callback;
+  callbacks.select_padding_callback = select_padding_callback;
 }
 } // namespace
 

src/nghttp.cc

@@ -100,7 +100,7 @@ struct Config {
   bool continuation;
   Config()
     : output_upper_thres(1024*1024),
-      padding_boundary(NGHTTP2_PADDING_BOUNDARY),
+      padding_boundary(0),
       peer_max_concurrent_streams(NGHTTP2_INITIAL_MAX_CONCURRENT_STREAMS),
       header_table_size(-1),
       pri(NGHTTP2_PRI_DEFAULT),
@@ -716,10 +716,8 @@ struct HttpClient {
     }
     nghttp2_opt_set opt_set;
     opt_set.peer_max_concurrent_streams = config.peer_max_concurrent_streams;
-    opt_set.padding_boundary = config.padding_boundary;
     rv = nghttp2_session_client_new2(&session, callbacks, this,
-                                     NGHTTP2_OPT_PEER_MAX_CONCURRENT_STREAMS |
-                                     NGHTTP2_OPT_PADDING_BOUNDARY,
+                                     NGHTTP2_OPT_PEER_MAX_CONCURRENT_STREAMS,
                                      &opt_set);
     if(rv != 0) {
       return -1;
@@ -964,14 +962,10 @@ int submit_request
      {"accept-encoding", "gzip, deflate"},
      {"user-agent", "nghttp2/" NGHTTP2_VERSION}};
   if(config.continuation) {
-    build_headers.emplace_back("continuation-test-1",
-                               std::string(4096, '-'));
-    build_headers.emplace_back("continuation-test-2",
-                               std::string(4096, '-'));
-    build_headers.emplace_back("continuation-test-3",
-                               std::string(4096, '-'));
-    build_headers.emplace_back("continuation-test-4",
-                               std::string(4096, '-'));
+    for(size_t i = 0; i < 8; ++i) {
+      build_headers.emplace_back("continuation-test-" + util::utos(i+1),
+                                 std::string(4096, '-'));
+    }
   }
   auto num_initial_headers = build_headers.size();
   if(req->data_prd) {
@@ -1132,6 +1126,22 @@ int before_frame_send_callback
 }
 } // namespace
 
+namespace {
+ssize_t select_padding_callback
+(nghttp2_session *session, const nghttp2_frame *frame, size_t max_payload,
+ void *user_data)
+{
+  auto bd = config.padding_boundary;
+  if(bd == 0) {
+    return frame->hd.length;
+  }
+  if(frame->hd.length == 0) {
+    return bd;
+  }
+  return std::min(max_payload, (frame->hd.length + bd - 1) / bd * bd);
+}
+} // namespace
+
 namespace {
 void check_response_header(nghttp2_session *session, Request* req)
 {
@@ -1588,6 +1598,7 @@ int run(char **uris, int n)
   }
   callbacks.on_data_chunk_recv_callback = on_data_chunk_recv_callback;
   callbacks.on_header_callback = on_header_callback;
+  callbacks.select_padding_callback = select_padding_callback;
 
   std::string prev_scheme;
   std::string prev_host;

tests/main.c

@@ -201,8 +201,6 @@ int main(int argc, char* argv[])
                    test_nghttp2_frame_pack_headers) ||
       !CU_add_test(pSuite, "frame_pack_headers_frame_too_large",
                    test_nghttp2_frame_pack_headers_frame_too_large) ||
-      !CU_add_test(pSuite, "frame_pack_headers_with_padding",
-                   test_nghttp2_frame_pack_headers_with_padding) ||
       !CU_add_test(pSuite, "frame_pack_priority",
                    test_nghttp2_frame_pack_priority) ||
       !CU_add_test(pSuite, "frame_pack_rst_stream",

tests/nghttp2_frame_test.c

@@ -91,7 +91,7 @@ void test_nghttp2_frame_pack_headers()
                              1000000007,
                              1 << 20, nva, nvlen);
   framelen = nghttp2_frame_pack_headers(&buf, &buflen, &bufoff, &frame,
-                                        &deflater, 0);
+                                        &deflater);
 
   CU_ASSERT(0 == unpack_frame((nghttp2_frame*)&oframe, buf + bufoff,
                               framelen - bufoff));
@@ -117,7 +117,7 @@ void test_nghttp2_frame_pack_headers()
   /* Next, include PRIORITY flag */
   frame.hd.flags |= NGHTTP2_FLAG_PRIORITY;
   framelen = nghttp2_frame_pack_headers(&buf, &buflen, &bufoff, &frame,
-                                        &deflater, 0);
+                                        &deflater);
 
   CU_ASSERT(0 == unpack_frame((nghttp2_frame*)&oframe, buf + bufoff,
                               framelen - bufoff));
@@ -173,7 +173,7 @@ void test_nghttp2_frame_pack_headers_frame_too_large(void)
                              1000000007,
                              0, nva, nvlen);
   framelen = nghttp2_frame_pack_headers(&buf, &buflen, &bufoff, &frame,
-                                        &deflater, 0);
+                                        &deflater);
   CU_ASSERT_EQUAL(NGHTTP2_ERR_HEADER_COMP, framelen);
 
   nghttp2_frame_headers_free(&frame);
@@ -184,134 +184,6 @@ void test_nghttp2_frame_pack_headers_frame_too_large(void)
   nghttp2_hd_deflate_free(&deflater);
 }
 
-
-void test_nghttp2_frame_pack_headers_with_padding(void)
-{
-  nghttp2_hd_deflater deflater;
-  nghttp2_hd_inflater inflater;
-  nghttp2_headers frame, oframe;
-  uint8_t *buf = NULL;
-  size_t buflen = 0;
-  size_t bufoff;
-  ssize_t framelen;
-  nghttp2_nv *nva;
-  ssize_t nvlen;
-  nva_out out;
-  size_t trail_padlen;
-  size_t header_blocklen;
-
-  nva_out_init(&out);
-  nghttp2_hd_deflate_init(&deflater, NGHTTP2_HD_SIDE_REQUEST);
-  nghttp2_hd_inflate_init(&inflater, NGHTTP2_HD_SIDE_REQUEST);
-
-  /* Payload length is 0, so no padding */
-  nghttp2_frame_headers_init(&frame,
-                             NGHTTP2_FLAG_END_STREAM|NGHTTP2_FLAG_END_HEADERS,
-                             1000000007, NGHTTP2_PRI_DEFAULT, NULL, 0);
-  framelen = nghttp2_frame_pack_headers(&buf, &buflen, &bufoff, &frame,
-                                        &deflater, 128);
-
-  CU_ASSERT(2 == bufoff);
-  CU_ASSERT((ssize_t)bufoff + NGHTTP2_FRAME_HEAD_LENGTH == framelen);
-
-  CU_ASSERT(0 == unpack_frame((nghttp2_frame*)&oframe, buf + bufoff,
-                              framelen - bufoff));
-  check_frame_header(framelen - bufoff - NGHTTP2_FRAME_HEAD_LENGTH,
-                     NGHTTP2_HEADERS,
-                     NGHTTP2_FLAG_END_STREAM | NGHTTP2_FLAG_END_HEADERS,
-                     1000000007, &oframe.hd);
-
-  CU_ASSERT(NGHTTP2_PRI_DEFAULT == oframe.pri);
-  nghttp2_frame_headers_free(&oframe);
-
-  /* Include priroty */
-  frame.hd.flags |= NGHTTP2_FLAG_PRIORITY;
-  frame.pri = 1000000009;
-  bufoff = 0;
-
-  framelen = nghttp2_frame_pack_headers(&buf, &buflen, &bufoff, &frame,
-                                        &deflater, 128);
-
-  CU_ASSERT(1 == bufoff);
-  CU_ASSERT((ssize_t)bufoff + NGHTTP2_FRAME_HEAD_LENGTH + 128 == framelen);
-
-  CU_ASSERT(0 == unpack_frame((nghttp2_frame*)&oframe, buf + bufoff,
-                              framelen - bufoff));
-  check_frame_header(framelen - bufoff - NGHTTP2_FRAME_HEAD_LENGTH,
-                     NGHTTP2_HEADERS,
-                     NGHTTP2_FLAG_END_STREAM | NGHTTP2_FLAG_END_HEADERS |
-                     NGHTTP2_FLAG_PRIORITY | NGHTTP2_FLAG_PAD_LOW,
-                     1000000007, &oframe.hd);
-
-  CU_ASSERT(1000000009 == oframe.pri);
-  nghttp2_frame_headers_free(&oframe);
-
-  /* padding more than 256 */
-  bufoff = 0;
-
-  framelen = nghttp2_frame_pack_headers(&buf, &buflen, &bufoff, &frame,
-                                        &deflater, 512);
-
-  CU_ASSERT(0 == bufoff);
-  CU_ASSERT(NGHTTP2_FRAME_HEAD_LENGTH + 512 == framelen);
-
-  CU_ASSERT(0 == unpack_frame((nghttp2_frame*)&oframe, buf + bufoff,
-                              framelen - bufoff));
-  check_frame_header(framelen - bufoff - NGHTTP2_FRAME_HEAD_LENGTH,
-                     NGHTTP2_HEADERS,
-                     NGHTTP2_FLAG_END_STREAM | NGHTTP2_FLAG_END_HEADERS |
-                     NGHTTP2_FLAG_PRIORITY |
-                     NGHTTP2_FLAG_PAD_HIGH | NGHTTP2_FLAG_PAD_LOW,
-                     1000000007, &oframe.hd);
-
-  CU_ASSERT(1000000009 == oframe.pri);
-  nghttp2_frame_headers_free(&oframe);
-
-  /* Include priority + headers */
-  nva = headers();
-  nvlen = HEADERS_LENGTH;
-  frame.nva = nva;
-  frame.nvlen = nvlen;
-  bufoff = 0;
-
-  framelen = nghttp2_frame_pack_headers(&buf, &buflen, &bufoff, &frame,
-                                        &deflater, 512);
-
-  CU_ASSERT(0 == bufoff);
-  CU_ASSERT(NGHTTP2_FRAME_HEAD_LENGTH + 512 == framelen);
-
-  CU_ASSERT(0 == unpack_frame((nghttp2_frame*)&oframe, buf + bufoff,
-                              framelen - bufoff));
-  check_frame_header(framelen - bufoff - NGHTTP2_FRAME_HEAD_LENGTH,
-                     NGHTTP2_HEADERS,
-                     NGHTTP2_FLAG_END_STREAM | NGHTTP2_FLAG_END_HEADERS |
-                     NGHTTP2_FLAG_PRIORITY |
-                     NGHTTP2_FLAG_PAD_HIGH | NGHTTP2_FLAG_PAD_LOW,
-                     1000000007, &oframe.hd);
-
-  CU_ASSERT(1000000009 == oframe.pri);
-  trail_padlen = (buf[NGHTTP2_FRAME_HEAD_LENGTH] << 8) |
-    buf[NGHTTP2_FRAME_HEAD_LENGTH + 1];
-
-  header_blocklen = framelen - NGHTTP2_FRAME_HEAD_LENGTH - 2 - 4
-    - trail_padlen;
-  CU_ASSERT((ssize_t)header_blocklen ==
-            inflate_hd(&inflater, &out,
-                       buf + NGHTTP2_FRAME_HEAD_LENGTH + 2 + 4,
-                       header_blocklen));
-
-  CU_ASSERT(nvlen == (ssize_t)out.nvlen);
-  assert_nv_equal(nva, out.nva, nvlen);
-
-  nghttp2_frame_headers_free(&oframe);
-  nva_out_reset(&out);
-
-  free(buf);
-  nghttp2_frame_headers_free(&frame);
-  nghttp2_hd_inflate_free(&inflater);
-  nghttp2_hd_deflate_free(&deflater);
-}
-
 void test_nghttp2_frame_pack_priority(void)
 {
   nghttp2_priority frame, oframe;

tests/nghttp2_frame_test.h

@@ -27,7 +27,6 @@
 
 void test_nghttp2_frame_pack_headers(void);
 void test_nghttp2_frame_pack_headers_frame_too_large(void);
-void test_nghttp2_frame_pack_headers_with_padding(void);
 void test_nghttp2_frame_pack_priority(void);
 void test_nghttp2_frame_pack_rst_stream(void);
 void test_nghttp2_frame_pack_settings(void);

tests/nghttp2_session_test.c

@@ -72,6 +72,7 @@ typedef struct {
   int begin_headers_cb_called;
   nghttp2_nv nv;
   size_t data_chunk_len;
+  size_t padding_boundary;
 } my_user_data;
 
 static void scripted_data_feed_init(scripted_data_feed *df,
@@ -202,6 +203,17 @@ static int pause_on_data_chunk_recv_callback(nghttp2_session *session,
   return NGHTTP2_ERR_PAUSE;
 }
 
+static ssize_t select_padding_callback(nghttp2_session *session,
+                                       const nghttp2_frame *frame,
+                                       size_t max_payloadlen,
+                                       void *user_data)
+{
+  my_user_data *ud = (my_user_data*)user_data;
+  return nghttp2_min(max_payloadlen,
+                     (frame->hd.length + ud->padding_boundary - 1)
+                     / ud->padding_boundary * ud->padding_boundary);
+}
+
 static ssize_t fixed_length_data_source_read_callback
 (nghttp2_session *session, int32_t stream_id,
  uint8_t *buf, size_t len, int *eof,
@@ -358,7 +370,7 @@ void test_nghttp2_session_recv(void)
                              1, NGHTTP2_PRI_DEFAULT, nva, nvlen);
   framelen = nghttp2_frame_pack_headers(&framedata, &framedatalen, &bufoff,
                                         &frame.headers,
-                                        &deflater, 0);
+                                        &deflater);
 
   scripted_data_feed_init(&df, framedata + bufoff, framelen - bufoff);
   /* Send 1 byte per each read */
@@ -378,7 +390,7 @@ void test_nghttp2_session_recv(void)
                              5, NGHTTP2_PRI_DEFAULT, NULL, 0);
   framelen = nghttp2_frame_pack_headers(&framedata, &framedatalen, &bufoff,
                                         &frame.headers,
-                                        &deflater, 0);
+                                        &deflater);
 
   nghttp2_frame_headers_free(&frame.headers);
 
@@ -440,7 +452,7 @@ void test_nghttp2_session_recv_invalid_stream_id(void)
                              NGHTTP2_PRI_DEFAULT, NULL, 0);
   framelen = nghttp2_frame_pack_headers(&framedata, &framedatalen, &bufoff,
                                         &frame.headers,
-                                        &deflater, 0);
+                                        &deflater);
 
   scripted_data_feed_init(&df, framedata + bufoff, framelen - bufoff);
   nghttp2_frame_headers_free(&frame.headers);
@@ -485,7 +497,7 @@ void test_nghttp2_session_recv_invalid_frame(void)
                              NGHTTP2_PRI_DEFAULT, nva, nvlen);
   framelen = nghttp2_frame_pack_headers(&framedata, &framedatalen, &bufoff,
                                         &frame.headers,
-                                        &deflater, 0);
+                                        &deflater);
 
   scripted_data_feed_init(&df, framedata + bufoff, framelen - bufoff);
 
@@ -687,7 +699,7 @@ void test_nghttp2_session_recv_continuation(void)
   framedatalen = nghttp2_frame_pack_headers(&framedata, &framedatacap,
                                             &bufoff,
                                             &frame.headers,
-                                            &deflater, 0);
+                                            &deflater);
   nghttp2_frame_headers_free(&frame.headers);
 
   memcpy(data, framedata + bufoff, 9);
@@ -742,7 +754,7 @@ void test_nghttp2_session_recv_continuation(void)
                              1, NGHTTP2_PRI_DEFAULT, nva, nvlen);
   framedatalen = nghttp2_frame_pack_headers(&framedata, &framedatacap, &bufoff,
                                             &frame.headers,
-                                            &deflater, 0);
+                                            &deflater);
   nghttp2_frame_headers_free(&frame.headers);
   memcpy(data, framedata + bufoff, framedatalen - bufoff);
   datalen = framedatalen - bufoff;
@@ -812,7 +824,7 @@ void test_nghttp2_session_continue(void)
                              1, NGHTTP2_PRI_DEFAULT, nva, nvlen);
   framelen1 = nghttp2_frame_pack_headers(&framedata, &framedatalen, &bufoff,
                                          &frame.headers,
-                                         &deflater, 0);
+                                         &deflater);
   nghttp2_frame_headers_free(&frame.headers);
 
   memcpy(buffer, framedata + bufoff, framelen1 - bufoff);
@@ -823,7 +835,7 @@ void test_nghttp2_session_continue(void)
                              3, NGHTTP2_PRI_DEFAULT, nva, nvlen);
   framelen2 = nghttp2_frame_pack_headers(&framedata, &framedatalen, &bufoff,
                                          &frame.headers,
-                                         &deflater, 0);
+                                         &deflater);
   nghttp2_frame_headers_free(&frame.headers);
 
   memcpy(buffer + framelen1, framedata + bufoff, framelen2 - bufoff);
@@ -963,7 +975,6 @@ void test_nghttp2_session_add_frame(void)
   acc.length = 0;
   user_data.acc = &acc;
   CU_ASSERT(0 == nghttp2_session_client_new(&session, &callbacks, &user_data));
-  session_disable_pad(session);
 
   frame = malloc(sizeof(nghttp2_frame));
   nvlen = nghttp2_nv_array_copy(&nva, nv, ARRLEN(nv));
@@ -1488,11 +1499,17 @@ void test_nghttp2_session_on_push_promise_received(void)
   CU_ASSERT(0 == user_data.begin_headers_cb_called);
   CU_ASSERT(NULL == nghttp2_session_get_stream(session, 8));
   item = nghttp2_session_get_next_ob_item(session);
-  CU_ASSERT(NGHTTP2_RST_STREAM == OB_CTRL_TYPE(item));
-  CU_ASSERT(8 == OB_CTRL(item)->hd.stream_id);
-  CU_ASSERT(NGHTTP2_REFUSED_STREAM == OB_CTRL(item)->rst_stream.error_code);
+  CU_ASSERT(NGHTTP2_GOAWAY == OB_CTRL_TYPE(item));
+  CU_ASSERT(0 == OB_CTRL(item)->hd.stream_id);
+  CU_ASSERT(NGHTTP2_PROTOCOL_ERROR == OB_CTRL(item)->goaway.error_code);
   CU_ASSERT(0 == nghttp2_session_send(session));
 
+  nghttp2_session_del(session);
+
+  nghttp2_session_client_new(&session, &callbacks, &user_data);
+  memset(session->iframe.buf, 0, 4);
+  session->iframe.buflen = 4;
+
   /* Same ID twice */
   stream->state = NGHTTP2_STREAM_OPENING;
 
@@ -2182,7 +2199,7 @@ void test_nghttp2_submit_request_without_data(void)
   memset(&callbacks, 0, sizeof(nghttp2_session_callbacks));
   callbacks.send_callback = accumulator_send_callback;
   CU_ASSERT(0 == nghttp2_session_client_new(&session, &callbacks, &ud));
-  session_disable_pad(session);
+
   nghttp2_hd_inflate_init(&inflater, NGHTTP2_HD_SIDE_REQUEST);
   CU_ASSERT(0 == nghttp2_submit_request(session, NGHTTP2_PRI_DEFAULT,
                                         nva, ARRLEN(nva), &data_prd, NULL));
@@ -2254,7 +2271,7 @@ void test_nghttp2_submit_response_without_data(void)
   memset(&callbacks, 0, sizeof(nghttp2_session_callbacks));
   callbacks.send_callback = accumulator_send_callback;
   CU_ASSERT(0 == nghttp2_session_server_new(&session, &callbacks, &ud));
-  session_disable_pad(session);
+
   nghttp2_hd_inflate_init(&inflater, NGHTTP2_HD_SIDE_RESPONSE);
   nghttp2_session_open_stream(session, 1, NGHTTP2_FLAG_END_STREAM,
                               NGHTTP2_PRI_DEFAULT,
@@ -2426,7 +2443,7 @@ void test_nghttp2_submit_headers(void)
   callbacks.on_frame_send_callback = on_frame_send_callback;
 
   CU_ASSERT(0 == nghttp2_session_client_new(&session, &callbacks, &ud));
-  session_disable_pad(session);
+
   nghttp2_hd_inflate_init(&inflater, NGHTTP2_HD_SIDE_REQUEST);
   CU_ASSERT(0 == nghttp2_submit_headers(session,
                                         NGHTTP2_FLAG_END_STREAM,
@@ -3867,10 +3884,13 @@ void test_nghttp2_session_pack_data_with_padding(void)
   memset(&callbacks, 0, sizeof(callbacks));
   callbacks.send_callback = block_count_send_callback;
   callbacks.on_frame_send_callback = on_frame_send_callback;
+  callbacks.select_padding_callback = select_padding_callback;
+
   data_prd.read_callback = fixed_length_data_source_read_callback;
 
   nghttp2_session_client_new(&session, &callbacks, &ud);
-  session->padding_boundary = 512;
+
+  ud.padding_boundary = 512;
 
   nghttp2_submit_request(session, NGHTTP2_PRI_DEFAULT, NULL, 0, &data_prd,
                          NULL);
@@ -3881,7 +3901,8 @@ void test_nghttp2_session_pack_data_with_padding(void)
   CU_ASSERT(NGHTTP2_HEADERS == ud.sent_frame_type);
 
   frame = OB_DATA(session->aob.item);
-  CU_ASSERT(session->padding_boundary - datalen == frame->padlen);
+
+  CU_ASSERT(ud.padding_boundary - datalen == frame->padlen);
   CU_ASSERT(frame->hd.flags & NGHTTP2_FLAG_PAD_LOW);
   CU_ASSERT(frame->hd.flags & NGHTTP2_FLAG_PAD_HIGH);
 
@@ -3896,6 +3917,8 @@ void test_nghttp2_session_pack_data_with_padding(void)
   /* Check without PAD_HIGH */
   nghttp2_session_client_new(&session, &callbacks, &ud);
 
+  ud.padding_boundary = 64;
+
   nghttp2_submit_request(session, NGHTTP2_PRI_DEFAULT, NULL, 0, &data_prd,
                          NULL);
   ud.block_count = 1;
@@ -3905,7 +3928,7 @@ void test_nghttp2_session_pack_data_with_padding(void)
   CU_ASSERT(NGHTTP2_HEADERS == ud.sent_frame_type);
 
   frame = OB_DATA(session->aob.item);
-  CU_ASSERT((frame->padlen + datalen) % session->padding_boundary == 0);
+  CU_ASSERT((frame->padlen + datalen) % ud.padding_boundary == 0);
   CU_ASSERT(frame->hd.flags & NGHTTP2_FLAG_PAD_LOW);
   CU_ASSERT(0 == (frame->hd.flags & NGHTTP2_FLAG_PAD_HIGH));
 

tests/nghttp2_test_helper.c

@@ -163,8 +163,3 @@ ssize_t inflate_hd(nghttp2_hd_inflater *inflater, nva_out *out,
   nghttp2_hd_inflate_end_headers(inflater);
   return initial - buflen;
 }
-
-void session_disable_pad(nghttp2_session *session)
-{
-  session->pad_alignment = 0;
-}

tests/nghttp2_test_helper.h

@@ -29,7 +29,6 @@
 #  include <config.h>
 #endif /* HAVE_CONFIG_H */
 
-#include "nghttp2_session.h"
 #include "nghttp2_frame.h"
 #include "nghttp2_hd.h"
 
@@ -76,6 +75,4 @@ void add_out(nva_out *out, nghttp2_nv *nv);
 ssize_t inflate_hd(nghttp2_hd_inflater *inflater, nva_out *out,
                    uint8_t *buf, size_t buflen);
 
-void session_disable_pad(nghttp2_session *session);
-
 #endif /* NGHTTP2_TEST_HELPER_H */