Fix local window size adjustments

Now shrinking local window size properly limits the amount of
WINDOW_UPDATE value so that shrinked window is honored.

lib/includes/nghttp2/nghttp2.h

@@ -1990,11 +1990,12 @@ int nghttp2_submit_goaway(nghttp2_session *session,
  * should be submitted, then WINDOW_UPDATE is queued with the current
  * received bytes count.
  *
+ * If the |window_size_increment| is 0, the function does nothing and
+ * returns 0.
+ *
  * This function returns 0 if it succeeds, or one of the following
  * negative error codes:
  *
- * :enum:`NGHTTP2_ERR_INVALID_ARGUMENT`
- *     The |delta_window_size| is 0.
  * :enum:`NGHTTP2_ERR_FLOW_CONTROL`
  *     The local window size overflow or gets negative.
  * :enum:`NGHTTP2_ERR_STREAM_CLOSED`

lib/nghttp2_helper.c

@@ -94,25 +94,54 @@ void nghttp2_downcase(uint8_t *s, size_t len)
 
 int nghttp2_adjust_local_window_size(int32_t *local_window_size_ptr,
                                      int32_t *recv_window_size_ptr,
-                                     int32_t delta)
+                                     int32_t *recv_reduction_ptr,
+                                     int32_t *delta_ptr)
 {
-  if(delta > 0) {
-    int32_t new_recv_window_size = *recv_window_size_ptr - delta;
+  if(*delta_ptr > 0) {
+    int32_t new_recv_window_size =
+      nghttp2_max(0, *recv_window_size_ptr) - *delta_ptr;
     if(new_recv_window_size < 0) {
+      /* The delta size is strictly more than received bytes. Increase
+         local_window_size by that difference. */
+      int32_t recv_reduction_diff;
       if(*local_window_size_ptr >
          NGHTTP2_MAX_WINDOW_SIZE + new_recv_window_size) {
         return NGHTTP2_ERR_FLOW_CONTROL;
       }
       *local_window_size_ptr -= new_recv_window_size;
-      new_recv_window_size = 0;
+      /* If there is recv_reduction due to earlier window_size
+         reduction, we have to adjust it too. */
+      recv_reduction_diff = nghttp2_min(*recv_reduction_ptr,
+                                        -new_recv_window_size);
+      *recv_reduction_ptr -= recv_reduction_diff;
+      if(*recv_window_size_ptr < 0) {
+        *recv_window_size_ptr += recv_reduction_diff;
+      } else {
+        /* If *recv_window_size_ptr > 0, then those bytes are
+           considered to be backed to the remote peer, so it is
+           effectively 0 now. */
+        *recv_window_size_ptr = recv_reduction_diff;
+      }
+      /* recv_reduction_diff must be paied from *delta_ptr, since it
+         was added in window size reduction (see below). */
+      *delta_ptr -= recv_reduction_diff;
+    } else {
+      *recv_window_size_ptr = new_recv_window_size;
     }
-    *recv_window_size_ptr = new_recv_window_size;
     return 0;
   } else {
-    if(*local_window_size_ptr + delta < 0) {
+    if(*local_window_size_ptr + *delta_ptr < 0 ||
+       *recv_window_size_ptr < INT32_MIN - *delta_ptr ||
+       *recv_reduction_ptr > INT32_MAX + *delta_ptr) {
       return NGHTTP2_ERR_FLOW_CONTROL;
     }
-    *local_window_size_ptr += delta;
+    /* Decreasing local window size. Note that we achieve this without
+       noticing to the remote peer. To do this, we cut
+       recv_window_size by -delta. This means that we don't send
+       WINDOW_UPDATE for -delta bytes. */
+    *local_window_size_ptr += *delta_ptr;
+    *recv_window_size_ptr += *delta_ptr;
+    *recv_reduction_ptr -= *delta_ptr;
   }
   return 0;
 }

lib/nghttp2_helper.h

@@ -92,11 +92,12 @@ void* nghttp2_memdup(const void* src, size_t n);
 void nghttp2_downcase(uint8_t *s, size_t len);
 
 /*
- * Adjusts |*local_window_size_ptr| and |*recv_window_size_ptr| with
- * |delta| which is the WINDOW_UPDATE's window_size_increment sent
- * from local side. If |delta| is strictly larger than
- * |*recv_window_size_ptr|, |*local_window_size_ptr| is increased by
- * delta - *recv_window_size_ptr. If |delta| is negative,
+ * Adjusts |*local_window_size_ptr|, |*recv_window_size_ptr|,
+ * |*recv_reduction_ptr| with |*delta_ptr| which is the
+ * WINDOW_UPDATE's window_size_increment sent from local side. If
+ * |delta| is strictly larger than |*recv_window_size_ptr|,
+ * |*local_window_size_ptr| is increased by delta -
+ * *recv_window_size_ptr. If |delta| is negative,
  * |*local_window_size_ptr| is decreased by delta.
  *
  * This function returns 0 if it succeeds, or one of the following
@@ -107,7 +108,8 @@ void nghttp2_downcase(uint8_t *s, size_t len);
  */
 int nghttp2_adjust_local_window_size(int32_t *local_window_size_ptr,
                                      int32_t *recv_window_size_ptr,
-                                     int32_t delta);
+                                     int32_t *recv_reduction_ptr,
+                                     int32_t *delta_ptr);
 
 /*
  * Returns non-zero if the function decided that WINDOW_UPDATE should

lib/nghttp2_session.c

@@ -183,6 +183,7 @@ static int nghttp2_session_new(nghttp2_session **session_ptr,
   (*session_ptr)->local_flow_control = 1;
   (*session_ptr)->remote_window_size = NGHTTP2_INITIAL_CONNECTION_WINDOW_SIZE;
   (*session_ptr)->recv_window_size = 0;
+  (*session_ptr)->recv_reduction = 0;
   (*session_ptr)->local_window_size = NGHTTP2_INITIAL_CONNECTION_WINDOW_SIZE;
 
   (*session_ptr)->goaway_flags = NGHTTP2_GOAWAY_NONE;

lib/nghttp2_session.h

@@ -173,6 +173,9 @@ struct nghttp2_session {
   /* Keep track of the number of bytes received without
      WINDOW_UPDATE. */
   int32_t recv_window_size;
+  /* The amount of recv_window_size cut using submitting negative
+     value to WINDOW_UPDATE */
+  int32_t recv_reduction;
   /* window size for local flow control. It is initially set to
      NGHTTP2_INITIAL_CONNECTION_WINDOW_SIZE and could be
      increased/decreased by submitting WINDOW_UPDATE. See

lib/nghttp2_stream.c

@@ -49,6 +49,7 @@ void nghttp2_stream_init(nghttp2_stream *stream, int32_t stream_id,
   stream->remote_window_size = remote_initial_window_size;
   stream->local_window_size = local_initial_window_size;
   stream->recv_window_size = 0;
+  stream->recv_reduction = 0;
 }
 
 void nghttp2_stream_free(nghttp2_stream *stream)

lib/nghttp2_stream.h

@@ -115,6 +115,9 @@ typedef struct {
   /* Keep track of the number of bytes received without
      WINDOW_UPDATE. */
   int32_t recv_window_size;
+  /* The amount of recv_window_size cut using submitting negative
+     value to WINDOW_UPDATE */
+  int32_t recv_reduction;
   /* window size for local flow control. It is initially set to
      NGHTTP2_INITIAL_WINDOW_SIZE and could be increased/decreased by
      submitting WINDOW_UPDATE. See nghttp2_submit_window_update(). */

lib/nghttp2_submit.c

@@ -285,7 +285,7 @@ int nghttp2_submit_window_update(nghttp2_session *session, uint8_t flags,
   int rv;
   nghttp2_stream *stream;
   if(window_size_increment == 0) {
-    return NGHTTP2_ERR_INVALID_ARGUMENT;
+    return 0;
   }
   flags = 0;
   if(stream_id == 0) {
@@ -294,7 +294,8 @@ int nghttp2_submit_window_update(nghttp2_session *session, uint8_t flags,
     }
     rv = nghttp2_adjust_local_window_size(&session->local_window_size,
                                           &session->recv_window_size,
-                                          window_size_increment);
+                                          &session->recv_reduction,
+                                          &window_size_increment);
     if(rv != 0) {
       return rv;
     }
@@ -314,7 +315,8 @@ int nghttp2_submit_window_update(nghttp2_session *session, uint8_t flags,
       }
       rv = nghttp2_adjust_local_window_size(&stream->local_window_size,
                                             &stream->recv_window_size,
-                                            window_size_increment);
+                                            &stream->recv_reduction,
+                                            &window_size_increment);
       if(rv != 0) {
         return rv;
       }

src/nghttp.cc

@@ -691,9 +691,10 @@ struct HttpClient {
       }
     }
     if(config.connection_window_bits != -1) {
+      int32_t wininc = (1 << config.connection_window_bits) - 1
+        - NGHTTP2_INITIAL_CONNECTION_WINDOW_SIZE;
       rv = nghttp2_submit_window_update
-        (session, NGHTTP2_FLAG_NONE, 0,
-         (1 << config.connection_window_bits) - 1);
+        (session, NGHTTP2_FLAG_NONE, 0, wininc);
       if(rv != 0) {
         return -1;
       }

tests/nghttp2_helper_test.c

@@ -32,46 +32,93 @@ void test_nghttp2_adjust_local_window_size(void)
 {
   int32_t local_window_size = 100;
   int32_t recv_window_size = 50;
+  int32_t recv_reduction = 0;
+  int32_t delta;
 
+  delta = 0;
   CU_ASSERT(0 == nghttp2_adjust_local_window_size(&local_window_size,
-                                                  &recv_window_size, 0));
+                                                  &recv_window_size,
+                                                  &recv_reduction,
+                                                  &delta));
   CU_ASSERT(100 == local_window_size);
   CU_ASSERT(50 == recv_window_size);
+  CU_ASSERT(0 == recv_reduction);
+  CU_ASSERT(0 == delta);
 
+  delta = 49;
   CU_ASSERT(0 == nghttp2_adjust_local_window_size(&local_window_size,
-                                                  &recv_window_size, 49));
+                                                  &recv_window_size,
+                                                  &recv_reduction,
+                                                  &delta));
   CU_ASSERT(100 == local_window_size);
   CU_ASSERT(1 == recv_window_size);
+  CU_ASSERT(0 == recv_reduction);
+  CU_ASSERT(49 == delta);
 
+  delta = 1;
   CU_ASSERT(0 == nghttp2_adjust_local_window_size(&local_window_size,
-                                                  &recv_window_size, 1));
+                                                  &recv_window_size,
+                                                  &recv_reduction,
+                                                  &delta));
   CU_ASSERT(100 == local_window_size);
   CU_ASSERT(0 == recv_window_size);
+  CU_ASSERT(0 == recv_reduction);
+  CU_ASSERT(1 == delta);
 
+  delta = 1;
   CU_ASSERT(0 == nghttp2_adjust_local_window_size(&local_window_size,
-                                                  &recv_window_size, 1));
+                                                  &recv_window_size,
+                                                  &recv_reduction,
+                                                  &delta));
   CU_ASSERT(101 == local_window_size);
   CU_ASSERT(0 == recv_window_size);
+  CU_ASSERT(0 == recv_reduction);
+  CU_ASSERT(1 == delta);
 
+  delta = -1;
   CU_ASSERT(0 == nghttp2_adjust_local_window_size(&local_window_size,
-                                                  &recv_window_size, -1));
+                                                  &recv_window_size,
+                                                  &recv_reduction,
+                                                  &delta));
   CU_ASSERT(100 == local_window_size);
+  CU_ASSERT(-1 == recv_window_size);
+  CU_ASSERT(1 == recv_reduction);
+  CU_ASSERT(-1 == delta);
+
+  delta = 1;
+  CU_ASSERT(0 == nghttp2_adjust_local_window_size(&local_window_size,
+                                                  &recv_window_size,
+                                                  &recv_reduction,
+                                                  &delta));
+  CU_ASSERT(101 == local_window_size);
   CU_ASSERT(0 == recv_window_size);
+  CU_ASSERT(0 == recv_reduction);
+  CU_ASSERT(0 == delta);
 
+  local_window_size = 100;
   recv_window_size = 50;
+  recv_reduction = 0;
+  delta = INT32_MAX;
   CU_ASSERT(NGHTTP2_ERR_FLOW_CONTROL ==
             nghttp2_adjust_local_window_size(&local_window_size,
                                              &recv_window_size,
-                                             INT32_MAX));
+                                             &recv_reduction,
+                                             &delta));
   CU_ASSERT(100 == local_window_size);
   CU_ASSERT(50 == recv_window_size);
+  CU_ASSERT(0 == recv_reduction);
+  CU_ASSERT(INT32_MAX == delta);
 
+  delta = INT32_MIN;
   CU_ASSERT(NGHTTP2_ERR_FLOW_CONTROL ==
             nghttp2_adjust_local_window_size(&local_window_size,
                                              &recv_window_size,
-                                             INT32_MIN));
+                                             &recv_reduction,
+                                             &delta));
   CU_ASSERT(100 == local_window_size);
   CU_ASSERT(50 == recv_window_size);
+  CU_ASSERT(0 == recv_reduction);
+  CU_ASSERT(INT32_MIN == delta);
 }
 
 static int check_header_name(const char *s)

tests/nghttp2_session_test.c

@@ -2432,7 +2432,7 @@ void test_nghttp2_submit_window_update(void)
   CU_ASSERT(0 == nghttp2_session_send(session));
   CU_ASSERT(0 == stream->recv_window_size);
 
-  CU_ASSERT(NGHTTP2_ERR_INVALID_ARGUMENT ==
+  CU_ASSERT(0 ==
             nghttp2_submit_window_update(session, NGHTTP2_FLAG_NONE, 2, 0));
   /* Disable local flow control */
   stream->local_flow_control = 0;
@@ -2472,17 +2472,26 @@ void test_nghttp2_submit_window_update_local_window_size(void)
   CU_ASSERT(0 == nghttp2_session_send(session));
 
   /* Let's decrement local window size */
-  stream->recv_window_size = 32768;
+  stream->recv_window_size = 65536;
   CU_ASSERT(0 == nghttp2_submit_window_update(session, NGHTTP2_FLAG_NONE, 2,
                                               -stream->local_window_size / 2));
   CU_ASSERT(32768 == stream->local_window_size);
   CU_ASSERT(0 == stream->recv_window_size);
+  CU_ASSERT(32768 == stream->recv_reduction);
+
   item = nghttp2_session_get_next_ob_item(session);
   CU_ASSERT(NGHTTP2_WINDOW_UPDATE == OB_CTRL_TYPE(item));
   CU_ASSERT(32768 == OB_CTRL(item)->window_update.window_size_increment);
-
   CU_ASSERT(0 == nghttp2_session_send(session));
 
+  /* Increase local window size */
+  CU_ASSERT(0 == nghttp2_submit_window_update(session, NGHTTP2_FLAG_NONE, 2,
+                                              16384));
+  CU_ASSERT(49152 == stream->local_window_size);
+  CU_ASSERT(16384 == stream->recv_window_size);
+  CU_ASSERT(16384 == stream->recv_reduction);
+  CU_ASSERT(NULL == nghttp2_session_get_next_ob_item(session));
+
   CU_ASSERT(NGHTTP2_ERR_FLOW_CONTROL ==
             nghttp2_submit_window_update(session, NGHTTP2_FLAG_NONE, 2,
                                          NGHTTP2_MAX_WINDOW_SIZE));
@@ -2503,17 +2512,25 @@ void test_nghttp2_submit_window_update_local_window_size(void)
   CU_ASSERT(0 == nghttp2_session_send(session));
 
   /* Go decrement part */
-  session->recv_window_size = 32768;
+  session->recv_window_size = 65536;
   CU_ASSERT(0 == nghttp2_submit_window_update(session, NGHTTP2_FLAG_NONE, 0,
                                               -session->local_window_size/2));
   CU_ASSERT(32768 == session->local_window_size);
   CU_ASSERT(0 == session->recv_window_size);
+  CU_ASSERT(32768 == session->recv_reduction);
   item = nghttp2_session_get_next_ob_item(session);
   CU_ASSERT(NGHTTP2_WINDOW_UPDATE == OB_CTRL_TYPE(item));
   CU_ASSERT(32768 == OB_CTRL(item)->window_update.window_size_increment);
-
   CU_ASSERT(0 == nghttp2_session_send(session));
 
+  /* Increase local window size */
+  CU_ASSERT(0 == nghttp2_submit_window_update(session, NGHTTP2_FLAG_NONE, 0,
+                                              16384));
+  CU_ASSERT(49152 == session->local_window_size);
+  CU_ASSERT(16384 == session->recv_window_size);
+  CU_ASSERT(16384 == session->recv_reduction);
+  CU_ASSERT(NULL == nghttp2_session_get_next_ob_item(session));
+
   CU_ASSERT(NGHTTP2_ERR_FLOW_CONTROL ==
             nghttp2_submit_window_update(session, NGHTTP2_FLAG_NONE, 0,
                                          NGHTTP2_MAX_WINDOW_SIZE));