Handle multiple SETTINGS_HEADER_TABLE_SIZE in incoming SETTINGS frame

Previously we just assumed that if same settings ID is found in
SETTINGS, it is enough to process last seen entry.  But it turns out
it is not enough for SETTINGS_HEADER_TABLE_SIZE.  If we have 0 and
4096 for SETTINGS_HEADER_TABLE_SIZE in one SETTINGS, we must first
shrink dynamic table to 0 and then enlarge it to 4096.  This means
that we have to remember the minimum value and last value.

lib/includes/nghttp2/nghttp2.h

@@ -516,6 +516,8 @@ typedef enum {
    */
   NGHTTP2_SETTINGS_INITIAL_WINDOW_SIZE = 0x04
 } nghttp2_settings_id;
+/* Note: If we add SETTINGS, update the capacity of
+   NGHTTP2_INBOUND_NUM_IV as well */
 
 /**
  * @macro

lib/nghttp2_session.c

@@ -282,6 +282,9 @@ static void session_inbound_frame_reset(nghttp2_session *session)
   iframe->payloadleft = 0;
   iframe->padlen = 0;
   iframe->headers_payload_length = 0;
+  iframe->iv[NGHTTP2_INBOUND_NUM_IV - 1].settings_id =
+    NGHTTP2_SETTINGS_HEADER_TABLE_SIZE;
+  iframe->iv[NGHTTP2_INBOUND_NUM_IV - 1].value = UINT32_MAX;
 }
 
 static void init_settings(nghttp2_settings_storage *settings)
@@ -3299,11 +3302,7 @@ int nghttp2_session_on_settings_received(nghttp2_session *session,
                                          int noack)
 {
   int rv;
-  int i;
+  size_t i;
-  int header_table_size_seen = 0;
-  int enable_push_seen = 0;
-  int max_concurrent_streams_seen = 0;
-  int initial_window_size_seen = 0;
 
   if(frame->hd.stream_id != 0) {
     return session_handle_invalid_connection
@@ -3338,20 +3337,11 @@ int nghttp2_session_on_settings_received(nghttp2_session *session,
     return session_call_on_frame_received(session, frame);
   }
 
-  for(i = (int)frame->settings.niv - 1; i >= 0; --i) {
+  for(i = 0; i < frame->settings.niv; ++i) {
     nghttp2_settings_entry *entry = &frame->settings.iv[i];
 
-    /* The spec says the settings values are processed in the order
-       they appear in the payload.  In other words, if the multiple
-       values for the same ID were found, use the last one and ignore
-       the rest. */
     switch(entry->settings_id) {
     case NGHTTP2_SETTINGS_HEADER_TABLE_SIZE:
-      if(header_table_size_seen) {
-        break;
-      }
-
-      header_table_size_seen = 1;
 
       if(entry->value > NGHTTP2_MAX_HEADER_TABLE_SIZE) {
         return session_handle_invalid_connection
@@ -3374,11 +3364,6 @@ int nghttp2_session_on_settings_received(nghttp2_session *session,
 
       break;
     case NGHTTP2_SETTINGS_ENABLE_PUSH:
-      if(enable_push_seen) {
-        break;
-      }
-
-      enable_push_seen = 1;
 
       if(entry->value != 0 && entry->value != 1) {
         return session_handle_invalid_connection
@@ -3396,21 +3381,11 @@ int nghttp2_session_on_settings_received(nghttp2_session *session,
 
       break;
     case NGHTTP2_SETTINGS_MAX_CONCURRENT_STREAMS:
-      if(max_concurrent_streams_seen) {
-        break;
-      }
-
-      max_concurrent_streams_seen = 1;
 
       session->remote_settings.max_concurrent_streams = entry->value;
 
       break;
     case NGHTTP2_SETTINGS_INITIAL_WINDOW_SIZE:
-      if(initial_window_size_seen) {
-        break;
-      }
-
-      initial_window_size_seen = 1;
 
       /* Update the initial window size of the all active streams */
       /* Check that initial_window_size < (1u << 31) */
@@ -3458,6 +3433,27 @@ static int session_process_settings_frame(nghttp2_session *session)
   int rv;
   nghttp2_inbound_frame *iframe = &session->iframe;
   nghttp2_frame *frame = &iframe->frame;
+  size_t i;
+  nghttp2_settings_entry min_header_size_entry;
+
+  min_header_size_entry = iframe->iv[NGHTTP2_INBOUND_NUM_IV - 1];
+
+  if(min_header_size_entry.value < UINT32_MAX) {
+    /* If we have less value, then we must have
+       SETTINGS_HEADER_TABLE_SIZE in i < iframe->niv */
+    for(i = 0; i < iframe->niv; ++i) {
+      if(iframe->iv[i].settings_id == NGHTTP2_SETTINGS_HEADER_TABLE_SIZE) {
+        break;
+      }
+    }
+
+    assert(i < iframe->niv);
+
+    if(min_header_size_entry.value != iframe->iv[i].value) {
+      iframe->iv[iframe->niv++] = iframe->iv[i];
+      iframe->iv[i] = min_header_size_entry;
+    }
+  }
 
   rv = nghttp2_frame_unpack_settings_payload(&frame->settings,
                                              iframe->iv, iframe->niv);
@@ -4115,6 +4111,12 @@ static void inbound_frame_set_settings_entry(nghttp2_inbound_frame *iframe)
   if(i == iframe->niv) {
     iframe->iv[iframe->niv++] = iv;
   }
+
+  if(iv.settings_id == NGHTTP2_SETTINGS_HEADER_TABLE_SIZE &&
+     iv.value < iframe->iv[NGHTTP2_INBOUND_NUM_IV - 1].value) {
+
+    iframe->iv[NGHTTP2_INBOUND_NUM_IV - 1] = iv;
+  }
 }
 
 /*

lib/nghttp2_session.h

@@ -81,6 +81,8 @@ typedef enum {
   NGHTTP2_IB_IGN_DATA
 } nghttp2_inbound_state;
 
+#define NGHTTP2_INBOUND_NUM_IV 5
+
 typedef struct {
   nghttp2_frame frame;
   /* Storage for extension frame payload.  frame->ext.payload points
@@ -88,8 +90,9 @@ typedef struct {
   nghttp2_ext_frame_payload ext_frame_payload;
   /* The received SETTINGS entry. The protocol says that we only cares
      about the defined settings ID. If unknown ID is received, it is
-     subject to connection error */
+     ignored.  We use last entry to hold minimum header table size if
-  nghttp2_settings_entry iv[5];
+     same settings are multiple times. */
+  nghttp2_settings_entry iv[NGHTTP2_INBOUND_NUM_IV];
   /* buffer pointers to small buffer, raw_sbuf */
   nghttp2_buf sbuf;
   /* buffer pointers to large buffer, raw_lbuf */

tests/main.c

@@ -93,6 +93,8 @@ int main(int argc, char* argv[])
                    test_nghttp2_session_recv_unknown_frame) ||
       !CU_add_test(pSuite, "session_recv_unexpected_continuation",
                    test_nghttp2_session_recv_unexpected_continuation) ||
+      !CU_add_test(pSuite, "session_recv_settings_header_table_size",
+                   test_nghttp2_session_recv_settings_header_table_size) ||
       !CU_add_test(pSuite, "session_continue", test_nghttp2_session_continue) ||
       !CU_add_test(pSuite, "session_add_frame",
                    test_nghttp2_session_add_frame) ||

tests/nghttp2_session_test.c

@@ -1231,6 +1231,182 @@ void test_nghttp2_session_recv_unexpected_continuation(void)
   nghttp2_session_del(session);
 }
 
+void test_nghttp2_session_recv_settings_header_table_size(void)
+{
+  nghttp2_session *session;
+  nghttp2_session_callbacks callbacks;
+  nghttp2_frame frame;
+  nghttp2_bufs bufs;
+  nghttp2_buf *buf;
+  ssize_t rv;
+  my_user_data ud;
+  nghttp2_settings_entry iv[3];
+  nghttp2_nv nv = MAKE_NV(":authority", "example.org");
+
+  frame_pack_bufs_init(&bufs);
+
+  memset(&callbacks, 0, sizeof(nghttp2_session_callbacks));
+  callbacks.on_frame_recv_callback = on_frame_recv_callback;
+  callbacks.send_callback = null_send_callback;
+
+  nghttp2_session_client_new(&session, &callbacks, &ud);
+
+  iv[0].settings_id = NGHTTP2_SETTINGS_HEADER_TABLE_SIZE;
+  iv[0].value = 3000;
+
+  iv[1].settings_id = NGHTTP2_SETTINGS_INITIAL_WINDOW_SIZE;
+  iv[1].value = 16384;
+
+  nghttp2_frame_settings_init(&frame.settings, NGHTTP2_FLAG_NONE,
+                              dup_iv(iv, 2), 2);
+
+  rv = nghttp2_frame_pack_settings(&bufs, &frame.settings);
+
+  CU_ASSERT(0 == rv);
+  CU_ASSERT(nghttp2_bufs_len(&bufs) > 0);
+
+  nghttp2_frame_settings_free(&frame.settings);
+
+  buf = &bufs.head->buf;
+  assert(nghttp2_bufs_len(&bufs) == nghttp2_buf_len(buf));
+
+  ud.frame_recv_cb_called = 0;
+
+  rv = nghttp2_session_mem_recv(session, buf->pos, nghttp2_buf_len(buf));
+
+  CU_ASSERT(rv == nghttp2_buf_len(buf));
+  CU_ASSERT(1 == ud.frame_recv_cb_called);
+
+  CU_ASSERT(3000 == session->remote_settings.header_table_size);
+  CU_ASSERT(16384 == session->remote_settings.initial_window_size);
+
+  nghttp2_bufs_reset(&bufs);
+
+  /* 2 SETTINGS_HEADER_TABLE_SIZE */
+  iv[0].settings_id = NGHTTP2_SETTINGS_HEADER_TABLE_SIZE;
+  iv[0].value = 3001;
+
+  iv[1].settings_id = NGHTTP2_SETTINGS_INITIAL_WINDOW_SIZE;
+  iv[1].value = 16383;
+
+  iv[2].settings_id = NGHTTP2_SETTINGS_HEADER_TABLE_SIZE;
+  iv[2].value = 3001;
+
+  nghttp2_frame_settings_init(&frame.settings, NGHTTP2_FLAG_NONE,
+                              dup_iv(iv, 3), 3);
+
+  rv = nghttp2_frame_pack_settings(&bufs, &frame.settings);
+
+  CU_ASSERT(0 == rv);
+  CU_ASSERT(nghttp2_bufs_len(&bufs) > 0);
+
+  nghttp2_frame_settings_free(&frame.settings);
+
+  buf = &bufs.head->buf;
+  assert(nghttp2_bufs_len(&bufs) == nghttp2_buf_len(buf));
+
+  ud.frame_recv_cb_called = 0;
+
+  rv = nghttp2_session_mem_recv(session, buf->pos, nghttp2_buf_len(buf));
+
+  CU_ASSERT(rv == nghttp2_buf_len(buf));
+  CU_ASSERT(1 == ud.frame_recv_cb_called);
+
+  CU_ASSERT(3001 == session->remote_settings.header_table_size);
+  CU_ASSERT(16383 == session->remote_settings.initial_window_size);
+
+  nghttp2_bufs_reset(&bufs);
+
+  /* 2 SETTINGS_HEADER_TABLE_SIZE; first entry clears dynamic header
+     table. */
+
+  nghttp2_submit_request(session, NULL, &nv, 1, NULL, NULL);
+  nghttp2_session_send(session);
+
+  CU_ASSERT(0 < session->hd_deflater.ctx.hd_table.len);
+
+  iv[0].settings_id = NGHTTP2_SETTINGS_HEADER_TABLE_SIZE;
+  iv[0].value = 0;
+
+  iv[1].settings_id = NGHTTP2_SETTINGS_INITIAL_WINDOW_SIZE;
+  iv[1].value = 16382;
+
+  iv[2].settings_id = NGHTTP2_SETTINGS_HEADER_TABLE_SIZE;
+  iv[2].value = 4096;
+
+  nghttp2_frame_settings_init(&frame.settings, NGHTTP2_FLAG_NONE,
+                              dup_iv(iv, 3), 3);
+
+  rv = nghttp2_frame_pack_settings(&bufs, &frame.settings);
+
+  CU_ASSERT(0 == rv);
+  CU_ASSERT(nghttp2_bufs_len(&bufs) > 0);
+
+  nghttp2_frame_settings_free(&frame.settings);
+
+  buf = &bufs.head->buf;
+  assert(nghttp2_bufs_len(&bufs) == nghttp2_buf_len(buf));
+
+  ud.frame_recv_cb_called = 0;
+
+  rv = nghttp2_session_mem_recv(session, buf->pos, nghttp2_buf_len(buf));
+
+  CU_ASSERT(rv == nghttp2_buf_len(buf));
+  CU_ASSERT(1 == ud.frame_recv_cb_called);
+
+  CU_ASSERT(4096 == session->remote_settings.header_table_size);
+  CU_ASSERT(16382 == session->remote_settings.initial_window_size);
+  CU_ASSERT(0 == session->hd_deflater.ctx.hd_table.len);
+
+  nghttp2_bufs_reset(&bufs);
+
+  /* 2 SETTINGS_HEADER_TABLE_SIZE; second entry clears dynamic header
+     table. */
+
+  nghttp2_submit_request(session, NULL, &nv, 1, NULL, NULL);
+  nghttp2_session_send(session);
+
+  CU_ASSERT(0 < session->hd_deflater.ctx.hd_table.len);
+
+  iv[0].settings_id = NGHTTP2_SETTINGS_HEADER_TABLE_SIZE;
+  iv[0].value = 3000;
+
+  iv[1].settings_id = NGHTTP2_SETTINGS_INITIAL_WINDOW_SIZE;
+  iv[1].value = 16381;
+
+  iv[2].settings_id = NGHTTP2_SETTINGS_HEADER_TABLE_SIZE;
+  iv[2].value = 0;
+
+  nghttp2_frame_settings_init(&frame.settings, NGHTTP2_FLAG_NONE,
+                              dup_iv(iv, 3), 3);
+
+  rv = nghttp2_frame_pack_settings(&bufs, &frame.settings);
+
+  CU_ASSERT(0 == rv);
+  CU_ASSERT(nghttp2_bufs_len(&bufs) > 0);
+
+  nghttp2_frame_settings_free(&frame.settings);
+
+  buf = &bufs.head->buf;
+  assert(nghttp2_bufs_len(&bufs) == nghttp2_buf_len(buf));
+
+  ud.frame_recv_cb_called = 0;
+
+  rv = nghttp2_session_mem_recv(session, buf->pos, nghttp2_buf_len(buf));
+
+  CU_ASSERT(rv == nghttp2_buf_len(buf));
+  CU_ASSERT(1 == ud.frame_recv_cb_called);
+
+  CU_ASSERT(0 == session->remote_settings.header_table_size);
+  CU_ASSERT(16381 == session->remote_settings.initial_window_size);
+  CU_ASSERT(0 == session->hd_deflater.ctx.hd_table.len);
+
+  nghttp2_bufs_reset(&bufs);
+
+  nghttp2_bufs_free(&bufs);
+  nghttp2_session_del(session);
+}
+
 void test_nghttp2_session_continue(void)
 {
   nghttp2_session *session;
@@ -1868,6 +2044,7 @@ void test_nghttp2_session_on_settings_received(void)
   const size_t niv = 5;
   nghttp2_settings_entry iv[255];
   nghttp2_outbound_item *item;
+  nghttp2_nv nv = MAKE_NV(":authority", "example.org");
 
   iv[0].settings_id = NGHTTP2_SETTINGS_MAX_CONCURRENT_STREAMS;
   iv[0].value = 50;
@@ -1885,6 +2062,8 @@ void test_nghttp2_session_on_settings_received(void)
   iv[4].value = 0;
 
   memset(&callbacks, 0, sizeof(nghttp2_session_callbacks));
+  callbacks.send_callback = null_send_callback;
+
   nghttp2_session_client_new(&session, &callbacks, &user_data);
   session->remote_settings.initial_window_size = 16*1024;
 
@@ -1952,6 +2131,34 @@ void test_nghttp2_session_on_settings_received(void)
 
   nghttp2_frame_settings_free(&frame.settings);
   nghttp2_session_del(session);
+
+  /* Check that 2 SETTINGS_HEADER_TABLE_SIZE 0 and 4096 are included
+     and header table size is once cleared to 0. */
+  nghttp2_session_client_new(&session, &callbacks, NULL);
+
+  nghttp2_submit_request(session, NULL, &nv, 1, NULL, NULL);
+
+  nghttp2_session_send(session);
+
+  CU_ASSERT(session->hd_deflater.ctx.hd_table.len > 0);
+
+  iv[0].settings_id = NGHTTP2_SETTINGS_HEADER_TABLE_SIZE;
+  iv[0].value = 0;
+
+  iv[1].settings_id = NGHTTP2_SETTINGS_HEADER_TABLE_SIZE;
+  iv[1].value = 2048;
+
+  nghttp2_frame_settings_init(&frame.settings, NGHTTP2_FLAG_NONE,
+                              dup_iv(iv, 2), 2);
+
+  CU_ASSERT(0 == nghttp2_session_on_settings_received(session, &frame, 0));
+
+  CU_ASSERT(0 == session->hd_deflater.ctx.hd_table.len);
+  CU_ASSERT(2048 == session->hd_deflater.ctx.hd_table_bufsize_max);
+  CU_ASSERT(2048 == session->remote_settings.header_table_size);
+
+  nghttp2_frame_settings_free(&frame.settings);
+  nghttp2_session_del(session);
 }
 
 void test_nghttp2_session_on_push_promise_received(void)

tests/nghttp2_session_test.h

@@ -36,6 +36,7 @@ void test_nghttp2_session_recv_premature_headers(void);
 void test_nghttp2_session_recv_altsvc(void);
 void test_nghttp2_session_recv_unknown_frame(void);
 void test_nghttp2_session_recv_unexpected_continuation(void);
+void test_nghttp2_session_recv_settings_header_table_size(void);
 void test_nghttp2_session_continue(void);
 void test_nghttp2_session_add_frame(void);
 void test_nghttp2_session_on_request_headers_received(void);