From: Stefan Eissing <stefan@eissing.org>
Date: Thu, 17 Oct 2024 15:00:41 +0000 (+0200)
Subject: http2: auto reset stream on server eos
X-Git-Tag: curl-8_11_0~123
X-Git-Url: http://git.ipfire.org/?a=commitdiff_plain;h=fe2a72029e4c29864d4be845068e4e065764f8b2;p=thirdparty%2Fcurl.git

http2: auto reset stream on server eos

When a server signals EOS from its side and the curl upload is
unfinished and the server has not given a positive HTTP status response,
auto RST the stream to signal that the upload is incomplete and that the
whole transfer can be stopped.

Fixes the case where the server responds with 413 on an upload but does
not RST the stream from its side, as httpd and others do.

Reported-by: jkamp-aws on github
Fixes #15316
Closes #15325
---

diff --git a/lib/http2.c b/lib/http2.c
index 451fa90de6..e98b64e8ea 100644
--- a/lib/http2.c
+++ b/lib/http2.c
@@ -1119,9 +1119,6 @@ static CURLcode on_stream_frame(struct Curl_cfilter *cf,
         return CURLE_RECV_ERROR;
       }
     }
-    if(frame->hd.flags & NGHTTP2_FLAG_END_STREAM) {
-      drain_stream(cf, data, stream);
-    }
     break;
   case NGHTTP2_HEADERS:
     if(stream->bodystarted) {
@@ -1137,10 +1134,10 @@ static CURLcode on_stream_frame(struct Curl_cfilter *cf,
       return CURLE_RECV_ERROR;
 
     /* Only final status code signals the end of header */
-    if(stream->status_code / 100 != 1) {
+    if(stream->status_code / 100 != 1)
       stream->bodystarted = TRUE;
+    else
       stream->status_code = -1;
-    }
 
     h2_xfer_write_resp_hd(cf, data, stream, STRCONST("\r\n"), stream->closed);
 
@@ -1187,6 +1184,22 @@ static CURLcode on_stream_frame(struct Curl_cfilter *cf,
   default:
     break;
   }
+
+  if(frame->hd.flags & NGHTTP2_FLAG_END_STREAM) {
+    if(!stream->closed && !stream->body_eos &&
+       ((stream->status_code >= 400) || (stream->status_code < 200))) {
+      /* The server did not give us a positive response and we are not
+       * done uploading the request body. We need to stop doing that and
+       * also inform the server that we aborted our side. */
+      CURL_TRC_CF(data, cf, "[%d] EOS frame with unfinished upload and "
+                  "HTTP status %d, abort upload by RST",
+                  stream_id, stream->status_code);
+      nghttp2_submit_rst_stream(ctx->h2, NGHTTP2_FLAG_NONE,
+                                stream->id, NGHTTP2_STREAM_CLOSED);
+      stream->closed = TRUE;
+    }
+    drain_stream(cf, data, stream);
+  }
   return CURLE_OK;
 }
 
diff --git a/tests/http/test_07_upload.py b/tests/http/test_07_upload.py
index 84a901606e..2ce2707cc1 100644
--- a/tests/http/test_07_upload.py
+++ b/tests/http/test_07_upload.py
@@ -589,6 +589,22 @@ class TestUpload:
             exp_code = 0  # we get a 500 from the server
         r.check_exit_code(exp_code)  # GOT_NOTHING
 
+    @pytest.mark.parametrize("proto", ['http/1.1', 'h2', 'h3'])
+    def test_07_43_upload_denied(self, env: Env, httpd, nghttpx, repeat, proto):
+        if proto == 'h3' and not env.have_h3():
+            pytest.skip("h3 not supported")
+        if proto == 'h3' and env.curl_uses_lib('msh3'):
+            pytest.skip("msh3 fails here")
+        fdata = os.path.join(env.gen_dir, 'data-10m')
+        count = 1
+        max_upload = 128 * 1024
+        curl = CurlClient(env=env)
+        url = f'https://{env.authority_for(env.domain1, proto)}/curltest/put?'\
+            f'id=[0-{count-1}]&max_upload={max_upload}'
+        r = curl.http_put(urls=[url], fdata=fdata, alpn_proto=proto,
+                             extra_args=['--trace-config', 'all'])
+        r.check_stats(count=count, http_status=413, exitcode=0)
+
     # speed limited on put handler
     @pytest.mark.parametrize("proto", ['http/1.1', 'h2', 'h3'])
     def test_07_50_put_speed_limit(self, env: Env, httpd, nghttpx, proto, repeat):
diff --git a/tests/http/testenv/mod_curltest/mod_curltest.c b/tests/http/testenv/mod_curltest/mod_curltest.c
index a55a64ee1c..2b57a082b7 100644
--- a/tests/http/testenv/mod_curltest/mod_curltest.c
+++ b/tests/http/testenv/mod_curltest/mod_curltest.c
@@ -534,6 +534,7 @@ static int curltest_put_handler(request_rec *r)
   char buffer[128*1024];
   const char *ct;
   apr_off_t rbody_len = 0;
+  apr_off_t rbody_max_len = -1;
   const char *s_rbody_len;
   const char *request_id = "none";
   apr_time_t read_delay = 0, chunk_delay = 0;
@@ -573,6 +574,10 @@ static int curltest_put_handler(request_rec *r)
             continue;
           }
         }
+        else if(!strcmp("max_upload", arg)) {
+          rbody_max_len = (int)apr_atoi64(val);
+          continue;
+        }
       }
       ap_log_rerror(APLOG_MARK, APLOG_ERR, 0, r, "query parameter not "
                     "understood: '%s' in %s",
@@ -611,6 +616,10 @@ static int curltest_put_handler(request_rec *r)
         apr_sleep(chunk_delay);
       }
       rbody_len += l;
+      if((rbody_max_len > 0) && (rbody_len > rbody_max_len)) {
+        r->status = 413;
+        break;
+      }
     }
   }
   /* we are done */
@@ -625,6 +634,10 @@ static int curltest_put_handler(request_rec *r)
 
   rv = ap_pass_brigade(r->output_filters, bb);
 
+  if(r->status == 413) {
+    apr_sleep(apr_time_from_sec(1));
+  }
+
 cleanup:
   if(rv == APR_SUCCESS
      || r->status != HTTP_OK