http2: fix stream window size after unpausing

When pausing a HTTP/2 transfer, the stream's local window size
is reduced to 0 to prevent the server from sending further data
which curl cannot write out to the application.

When unpausing again, the stream's window size was not correctly
increased again. The attempt to trigger a window update was
ignored by nghttp2, the server never received it and the transfer
stalled.

Add a debug feature to allow use of small window sizes which
reproduces this bug in test_02_21.

Fixes #16955
Closes #16960

diff --git a/docs/libcurl/libcurl-env-dbg.md b/docs/libcurl/libcurl-env-dbg.md
index 471533625f6b6a2b1e8b3d1d887a3beb744cda5a..60c887bfd5a90aa3fc980b7497c8ea4903bec008 100644 (file)
--- a/docs/libcurl/libcurl-env-dbg.md
+++ b/docs/libcurl/libcurl-env-dbg.md
@@ -147,3 +147,8 @@ Make a blocking, graceful shutdown of all remaining connections when
 a multi handle is destroyed. This implicitly triggers for easy handles
 that are run via easy_perform. The value of the environment variable
 gives the shutdown timeout in milliseconds.
+
+## `CURL_H2_STREAM_WIN_MAX`
+
+Set to a positive 32-bit number to override the HTTP/2 stream window's
+default of 10MB. Used in testing to verify correct window update handling.

diff --git a/lib/http2.c b/lib/http2.c
index 88fbcceb713577ba5e393faf6d4eb40225eee082..a1221dcc51dec61eee6e7d846dad4ed37fd42ccd 100644 (file)
--- a/lib/http2.c
+++ b/lib/http2.c
@@ -44,6 +44,7 @@
 #include "connect.h"
 #include "rand.h"
 #include "strdup.h"
+#include "strparse.h"
 #include "transfer.h"
 #include "dynbuf.h"
 #include "headers.h"
@@ -141,6 +142,9 @@ struct cf_h2_ctx {
   uint32_t goaway_error;        /* goaway error code from server */
   int32_t remote_max_sid;       /* max id processed by server */
   int32_t local_max_sid;        /* max id processed by us */
+#ifdef DEBUGBUILD
+  int32_t stream_win_max;       /* max h2 stream window size */
+#endif
   BIT(initialized);
   BIT(via_h1_upgrade);
   BIT(conn_closed);
@@ -166,6 +170,18 @@ static void cf_h2_ctx_init(struct cf_h2_ctx *ctx, bool via_h1_upgrade)
   Curl_hash_offt_init(&ctx->streams, 63, h2_stream_hash_free);
   ctx->remote_max_sid = 2147483647;
   ctx->via_h1_upgrade = via_h1_upgrade;
+#ifdef DEBUGBUILD
+  {
+    const char *p = getenv("CURL_H2_STREAM_WIN_MAX");
+
+    ctx->stream_win_max = H2_STREAM_WINDOW_SIZE_MAX;
+    if(p) {
+      curl_off_t l;
+      if(!Curl_str_number(&p, &l, INT_MAX))
+        ctx->stream_win_max = (int32_t)l;
+    }
+  }
+#endif
   ctx->initialized = TRUE;
 }
 
@@ -285,7 +301,15 @@ static int32_t cf_h2_get_desired_local_win(struct Curl_cfilter *cf,
      * This gets less precise the higher the latency. */
     return (int32_t)data->set.max_recv_speed;
   }
+#ifdef DEBUGBUILD
+  else {
+    struct cf_h2_ctx *ctx = cf->ctx;
+    CURL_TRC_CF(data, cf, "stream_win_max=%d", ctx->stream_win_max);
+    return ctx->stream_win_max;
+  }
+#else
   return H2_STREAM_WINDOW_SIZE_MAX;
+#endif
 }
 
 static CURLcode cf_h2_update_local_win(struct Curl_cfilter *cf,
@@ -302,6 +326,13 @@ static CURLcode cf_h2_update_local_win(struct Curl_cfilter *cf,
     int32_t wsize = nghttp2_session_get_stream_effective_local_window_size(
                       ctx->h2, stream->id);
     if(dwsize > wsize) {
+      rv = nghttp2_session_set_local_window_size(ctx->h2, NGHTTP2_FLAG_NONE,
+                                                 stream->id, dwsize);
+      if(rv) {
+        failf(data, "[%d] nghttp2 set_local_window_size(%d) failed: "
+              "%s(%d)", stream->id, dwsize, nghttp2_strerror(rv), rv);
+        return CURLE_HTTP2;
+      }
       rv = nghttp2_submit_window_update(ctx->h2, NGHTTP2_FLAG_NONE,
                                         stream->id, dwsize - wsize);
       if(rv) {

diff --git a/tests/http/test_02_download.py b/tests/http/test_02_download.py
index 4b9ae3caefab557b36da65e53f36eee2203559ec..b55f022338ad9fd3801e4cde490471b15cc03924 100644 (file)
--- a/tests/http/test_02_download.py
+++ b/tests/http/test_02_download.py
@@ -313,9 +313,9 @@ class TestDownload:
         assert httpd.stop()
         assert httpd.start()
 
-    # download via lib client, 1 at a time, pause/resume at different offsets
+    # download serial via lib client, pause/resume at different offsets
     @pytest.mark.parametrize("pause_offset", [0, 10*1024, 100*1023, 640000])
-    @pytest.mark.parametrize("proto", ['http/1.1', 'h2', 'h3'])
+    @pytest.mark.parametrize("proto", ['http/1.1', 'h3'])
     def test_02_21_lib_serial(self, env: Env, httpd, nghttpx, proto, pause_offset):
         if proto == 'h3' and not env.have_h3():
             pytest.skip("h3 not supported")
@@ -332,6 +332,29 @@ class TestDownload:
         srcfile = os.path.join(httpd.docs_dir, docname)
         self.check_downloads(client, srcfile, count)
 
+    # h2 download parallel via lib client, pause/resume at different offsets
+    # debug-override stream window size to reproduce #16955
+    @pytest.mark.parametrize("pause_offset", [0, 10*1024, 100*1023, 640000])
+    @pytest.mark.parametrize("swin_max", [0, 10*1024])
+    def test_02_21_h2_lib_serial(self, env: Env, httpd, pause_offset, swin_max):
+        proto = 'h2'
+        count = 2
+        docname = 'data-10m'
+        url = f'https://localhost:{env.https_port}/{docname}'
+        run_env = os.environ.copy()
+        run_env['CURL_DEBUG'] = 'multi,http/2'
+        if swin_max > 0:
+            run_env['CURL_H2_STREAM_WIN_MAX'] = f'{swin_max}'
+        client = LocalClient(name='hx-download', env=env, run_env=run_env)
+        if not client.exists():
+            pytest.skip(f'example client not built: {client.name}')
+        r = client.run(args=[
+             '-n', f'{count}', '-P', f'{pause_offset}', '-V', proto, url
+        ])
+        r.check_exit_code(0)
+        srcfile = os.path.join(httpd.docs_dir, docname)
+        self.check_downloads(client, srcfile, count)
+
     # download via lib client, several at a time, pause/resume
     @pytest.mark.parametrize("pause_offset", [100*1023])
     @pytest.mark.parametrize("proto", ['http/1.1', 'h2', 'h3'])