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
This commit is contained in:
Stefan Eissing 2024-10-17 17:00:41 +02:00 committed by Daniel Stenberg
parent 2ae8d9b579
commit fe2a72029e
No known key found for this signature in database
GPG Key ID: 5CC908FDB71E12C2
3 changed files with 47 additions and 5 deletions

View File

@ -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;
}

View File

@ -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):

View File

@ -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