Fix handling of SETTINGS_MAX_CONCURRENT_STREAMS.

The maximum number of outgoing concurrent streams is initially
limited to 100 to avoid issues when the local endpoint submits
lots of requests before receiving initial SETTINGS frame from
the remote endpoint, since sending them at once to the remote
endpoint could lead to rejection of some of the requests.

This initial limit is overwritten with the value advertised in
SETTINGS_MAX_CONCURRENT_STREAMS setting by the remote endpoint,
but previously, it wasn't lifted if the remote endpoint didn't
advertise that setting (implying no limits), in which case the
limit of 100 was retained, even though it was never advertised
by the remote endpoint.

Signed-off-by: Piotr Sikora <piotrsikora@google.com>

lib/includes/nghttp2/nghttp2.h

@@ -2478,15 +2478,15 @@ nghttp2_option_set_no_auto_window_update(nghttp2_option *option, int val);
  *
  * This option sets the SETTINGS_MAX_CONCURRENT_STREAMS value of
  * remote endpoint as if it is received in SETTINGS frame.  Without
- * specifying this option, before the local endpoint receives
+ * specifying this option, the maximum number of outgoing concurrent
- * SETTINGS_MAX_CONCURRENT_STREAMS in SETTINGS frame from remote
+ * streams is initially limited to 100 to avoid issues when the local
- * endpoint, SETTINGS_MAX_CONCURRENT_STREAMS is unlimited.  This may
+ * endpoint submits lots of requests before receiving initial SETTINGS
- * cause problem if local endpoint submits lots of requests initially
+ * frame from the remote endpoint, since sending them at once to the
- * and sending them at once to the remote peer may lead to the
+ * remote endpoint could lead to rejection of some of the requests.
- * rejection of some requests.  Specifying this option to the sensible
+ * This value will be overwritten when the local endpoint receives
- * value, say 100, may avoid this kind of issue. This value will be
+ * initial SETTINGS frame from the remote endpoint, either to the
- * overwritten if the local endpoint receives
+ * value advertised in SETTINGS_MAX_CONCURRENT_STREAMS or to the
- * SETTINGS_MAX_CONCURRENT_STREAMS from the remote endpoint.
+ * default value (unlimited) if none was advertised.
  */
 NGHTTP2_EXTERN void
 nghttp2_option_set_peer_max_concurrent_streams(nghttp2_option *option,

lib/nghttp2_session.c

@@ -4410,6 +4410,12 @@ int nghttp2_session_on_settings_received(nghttp2_session *session,
     return session_call_on_frame_received(session, frame);
   }
 
+  if (!session->remote_settings_received) {
+    session->remote_settings.max_concurrent_streams =
+        NGHTTP2_DEFAULT_MAX_CONCURRENT_STREAMS;
+    session->remote_settings_received = 1;
+  }
+
   for (i = 0; i < frame->settings.niv; ++i) {
     nghttp2_settings_entry *entry = &frame->settings.iv[i];
 

lib/nghttp2_session.h

@@ -303,8 +303,10 @@ struct nghttp2_session {
      increased/decreased by submitting WINDOW_UPDATE. See
      nghttp2_submit_window_update(). */
   int32_t local_window_size;
-  /* Settings value received from the remote endpoint. We just use ID
+  /* This flag is used to indicate that the local endpoint received initial
-     as index. The index = 0 is unused. */
+     SETTINGS frame from the remote endpoint. */
+  uint8_t remote_settings_received;
+  /* Settings value received from the remote endpoint. */
   nghttp2_settings_storage remote_settings;
   /* Settings value of the local endpoint. */
   nghttp2_settings_storage local_settings;

tests/nghttp2_session_test.c

@@ -3294,6 +3294,7 @@ void test_nghttp2_session_on_settings_received(void) {
   nghttp2_outbound_item *item;
   nghttp2_nv nv = MAKE_NV(":authority", "example.org");
   nghttp2_mem *mem;
+  nghttp2_option *option;
 
   mem = nghttp2_mem_default();
 
@@ -3406,6 +3407,23 @@ void test_nghttp2_session_on_settings_received(void) {
   nghttp2_frame_settings_free(&frame.settings, mem);
   nghttp2_session_del(session);
 
+  /* Check that remote SETTINGS_MAX_CONCURRENT_STREAMS is set to a value set by
+     nghttp2_option_set_peer_max_concurrent_streams() and reset to the default
+     value (unlimited) after receiving initial SETTINGS frame from the peer. */
+  nghttp2_option_new(&option);
+  nghttp2_option_set_peer_max_concurrent_streams(option, 1000);
+  nghttp2_session_client_new2(&session, &callbacks, NULL, option);
+  CU_ASSERT(1000 == session->remote_settings.max_concurrent_streams);
+
+  nghttp2_frame_settings_init(&frame.settings, NGHTTP2_FLAG_NONE, NULL, 0);
+  CU_ASSERT(0 == nghttp2_session_on_settings_received(session, &frame, 0));
+  CU_ASSERT(NGHTTP2_DEFAULT_MAX_CONCURRENT_STREAMS ==
+            session->remote_settings.max_concurrent_streams);
+
+  nghttp2_frame_settings_free(&frame.settings, mem);
+  nghttp2_session_del(session);
+  nghttp2_option_del(option);
+
   /* Check too large SETTINGS_MAX_FRAME_SIZE */
   nghttp2_session_server_new(&session, &callbacks, NULL);