From: Stefan Eissing <stefan@eissing.org>
Date: Wed, 8 Feb 2023 14:56:57 +0000 (+0100)
Subject: http2: minor buffer and error path fixes
X-Git-Tag: curl-7_88_0~41
X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=8c762f59983a3e9e2b80fdb34aa5e08f1d9a1c7d;p=thirdparty%2Fcurl.git

http2: minor buffer and error path fixes

- use memory buffer in full available size
- fail receive of reset/errored streams early

pytest:
- make test_05 error cases more reliable

Closes #10444
---

diff --git a/lib/http2.c b/lib/http2.c
index 778b588413..d5eed385e2 100644
--- a/lib/http2.c
+++ b/lib/http2.c
@@ -1905,8 +1905,9 @@ static ssize_t cf_h2_recv(struct Curl_cfilter *cf, struct Curl_easy *data,
 
   if(stream->memlen) {
     ssize_t retlen = stream->memlen;
-    DEBUGF(LOG_CF(data, cf, "[h2sid=%u] recv: returns %zd",
-                  stream->stream_id, retlen));
+
+    /* TODO: all this buffer handling is very brittle */
+    stream->len += stream->memlen;
     stream->memlen = 0;
 
     if(ctx->pause_stream_id == stream->stream_id) {
@@ -1918,6 +1919,10 @@ static ssize_t cf_h2_recv(struct Curl_cfilter *cf, struct Curl_easy *data,
       Curl_expire(data, 0, EXPIRE_RUN_NOW);
     }
     else if(stream->closed) {
+      if(stream->reset || stream->error) {
+        nread = http2_handle_stream_close(cf, data, stream, err);
+        goto out;
+      }
       /* this stream is closed, trigger a another read ASAP to detect that */
       DEBUGF(LOG_CF(data, cf, "[h2sid=%u] is closed now, run again",
                     stream->stream_id));
@@ -1928,11 +1933,15 @@ static ssize_t cf_h2_recv(struct Curl_cfilter *cf, struct Curl_easy *data,
       drained_transfer(cf, data);
 
     nread = retlen;
+    DEBUGF(LOG_CF(data, cf, "[h2sid=%u] cf_h2_recv -> %zd",
+                  stream->stream_id, nread));
     goto out;
   }
 
-  if(stream->closed)
-    return http2_handle_stream_close(cf, data, stream, err);
+  if(stream->closed) {
+    nread = http2_handle_stream_close(cf, data, stream, err);
+    goto out;
+  }
 
   if(!data->state.drain && Curl_conn_cf_data_pending(cf->next, data)) {
     DEBUGF(LOG_CF(data, cf, "[h2sid=%u] pending data, set drain",
diff --git a/tests/tests-httpd/test_05_errors.py b/tests/tests-httpd/test_05_errors.py
index 10d6786b7b..bb6e9217f1 100644
--- a/tests/tests-httpd/test_05_errors.py
+++ b/tests/tests-httpd/test_05_errors.py
@@ -57,30 +57,34 @@ class TestErrors:
         urln = f'https://{env.authority_for(env.domain1, proto)}' \
                f'/curltest/tweak?id=[0-{count - 1}]'\
                '&chunks=3&chunk_size=16000&body_error=reset'
-        r = curl.http_download(urls=[urln], alpn_proto=proto)
+        r = curl.http_download(urls=[urln], alpn_proto=proto, extra_args=[
+            '--retry', '0'
+        ])
         assert r.exit_code != 0, f'{r}'
         invalid_stats = []
         for idx, s in enumerate(r.stats):
-            if 'exitcode' not in s or s['exitcode'] not in [18, 56]:
+            if 'exitcode' not in s or s['exitcode'] not in [18, 56, 92]:
                 invalid_stats.append(f'request {idx} exit with {s["exitcode"]}')
         assert len(invalid_stats) == 0, f'failed: {invalid_stats}'
 
-    # download 20 file, check that we get CURLE_PARTIAL_FILE for all
+    # download files, check that we get CURLE_PARTIAL_FILE for all
     @pytest.mark.parametrize("proto", ['h2', 'h3'])
     def test_05_02_partial_20(self, env: Env, httpd, nghttpx, repeat,
                               proto):
         if proto == 'h3' and not env.have_h3():
             pytest.skip("h3 not supported")
-        count = 20
+        count = 5
         curl = CurlClient(env=env)
         urln = f'https://{env.authority_for(env.domain1, proto)}' \
                f'/curltest/tweak?id=[0-{count - 1}]'\
                '&chunks=3&chunk_size=16000&body_error=reset'
-        r = curl.http_download(urls=[urln], alpn_proto=proto)
+        r = curl.http_download(urls=[urln], alpn_proto=proto, extra_args=[
+            '--retry', '0', '--parallel',
+        ])
         assert r.exit_code != 0, f'{r}'
         assert len(r.stats) == count, f'did not get all stats: {r}'
         invalid_stats = []
         for idx, s in enumerate(r.stats):
-            if 'exitcode' not in s or s['exitcode'] not in [18, 56]:
-                invalid_stats.append(f'request {idx} exit with {s["exitcode"]}')
+            if 'exitcode' not in s or s['exitcode'] not in [18, 56, 92]:
+                invalid_stats.append(f'request {idx} exit with {s["exitcode"]}\n{s}')
         assert len(invalid_stats) == 0, f'failed: {invalid_stats}'
diff --git a/tests/tests-httpd/testenv/httpd.py b/tests/tests-httpd/testenv/httpd.py
index 1e9541b519..89a697dbf5 100644
--- a/tests/tests-httpd/testenv/httpd.py
+++ b/tests/tests-httpd/testenv/httpd.py
@@ -263,12 +263,12 @@ class Httpd:
             ]))
 
     def _get_log_level(self):
-        if self.env.verbose > 3:
-            return 'trace2'
-        if self.env.verbose > 2:
-            return 'trace1'
-        if self.env.verbose > 1:
-            return 'debug'
+        #if self.env.verbose > 3:
+        #    return 'trace2'
+        #if self.env.verbose > 2:
+        #    return 'trace1'
+        #if self.env.verbose > 1:
+        #    return 'debug'
         return 'info'
 
     def _curltest_conf(self) -> List[str]: