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 451fa90de61f26e1dad1551b164e8f9fedb0303a..e98b64e8eab92b3a20699f94f3e5a3f8a5f429c8 100644 (file)
--- 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 84a901606e5b7161e2e01dd829c63dce00844d1d..2ce2707cc10e1f548e46643e998a08d25bf8b395 100644 (file)
--- 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 a55a64ee1c0425fb7282b0e37a669ec0293b5592..2b57a082b7172603f0f285b80790fab7ee4616ec 100644 (file)
--- 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