diff --git a/lib/http2.c b/lib/http2.c index 0ebe630abe..2dc032f233 100644 --- a/lib/http2.c +++ b/lib/http2.c @@ -75,7 +75,9 @@ #define H2_NW_SEND_CHUNKS 1 /* stream recv/send chunks are a result of window / chunk sizes */ #define H2_STREAM_RECV_CHUNKS (H2_STREAM_WINDOW_SIZE / H2_CHUNK_SIZE) -#define H2_STREAM_SEND_CHUNKS (H2_STREAM_WINDOW_SIZE / H2_CHUNK_SIZE) +/* keep smaller stream upload buffer (default h2 window size) to have + * our progress bars and "upload done" reporting closer to reality */ +#define H2_STREAM_SEND_CHUNKS ((64 * 1024) / H2_CHUNK_SIZE) /* spare chunks we keep for a full window */ #define H2_STREAM_POOL_SPARES (H2_STREAM_WINDOW_SIZE / H2_CHUNK_SIZE) @@ -185,6 +187,8 @@ struct stream_ctx { bool reset; /* TRUE on stream reset */ bool close_handled; /* TRUE if stream closure is handled by libcurl */ bool bodystarted; + bool send_closed; /* transfer is done sending, we might have still + buffered data in stream->sendbuf to upload. */ }; #define H2_STREAM_CTX(d) ((struct stream_ctx *)(((d) && (d)->req.p.http)? \ @@ -205,7 +209,7 @@ static void drain_stream(struct Curl_cfilter *cf, (void)cf; bits = CURL_CSELECT_IN; - if(stream->upload_left) + if(!stream->send_closed && stream->upload_left) bits |= CURL_CSELECT_OUT; if(data->state.dselect_bits != bits) { data->state.dselect_bits = bits; @@ -1039,6 +1043,8 @@ static CURLcode on_stream_frame(struct Curl_cfilter *cf, DEBUGF(LOG_CF(data, cf, "[h2sid=%d] FRAME[RST]", stream_id)); stream->closed = TRUE; stream->reset = TRUE; + stream->send_closed = TRUE; + data->req.keepon &= ~KEEP_SEND_HOLD; drain_stream(cf, data, stream); break; case NGHTTP2_WINDOW_UPDATE: @@ -1438,7 +1444,7 @@ static ssize_t req_body_read_callback(nghttp2_session *session, nread = 0; } - if(nread > 0 && data_s->state.infilesize != -1) + if(nread > 0 && stream->upload_left != -1) stream->upload_left -= nread; DEBUGF(LOG_CF(data_s, cf, "[h2sid=%d] req_body_read(len=%zu) left=%zd" @@ -1517,15 +1523,18 @@ static CURLcode http2_data_done_send(struct Curl_cfilter *cf, goto out; DEBUGF(LOG_CF(data, cf, "[h2sid=%d] data done send", stream->id)); - if(stream->upload_left) { - /* If the stream still thinks there's data left to upload. */ - if(stream->upload_left == -1) - stream->upload_left = 0; /* DONE! */ - - /* resume sending here to trigger the callback to get called again so - that it can signal EOF to nghttp2 */ - (void)nghttp2_session_resume_data(ctx->h2, stream->id); - drain_stream(cf, data, stream); + if(!stream->send_closed) { + stream->send_closed = TRUE; + if(stream->upload_left) { + /* If we operated with unknown length, we now know that everything + * that is buffered is all we have to send. */ + if(stream->upload_left == -1) + stream->upload_left = Curl_bufq_len(&stream->sendbuf); + /* resume sending here to trigger the callback to get called again so + that it can signal EOF to nghttp2 */ + (void)nghttp2_session_resume_data(ctx->h2, stream->id); + drain_stream(cf, data, stream); + } } out: diff --git a/tests/http/test_07_upload.py b/tests/http/test_07_upload.py index 6b8f3dbc4d..37fa45cb9e 100644 --- a/tests/http/test_07_upload.py +++ b/tests/http/test_07_upload.py @@ -240,7 +240,50 @@ class TestUpload: url = f'https://{env.authority_for(env.domain1, proto)}/curltest/put?id=[0-{count-1}]' r = curl.http_put(urls=[url], fdata=fdata, alpn_proto=proto) r.check_response(count=count, http_status=200) - r.check_response(count=count, http_status=200) + + # issue #11157, upload that is 404'ed by server, needs to terminate + # correctly and not time out on sending + def test_07_33_issue_11157a(self, env: Env, httpd, nghttpx, repeat): + proto = 'h2' + fdata = os.path.join(env.gen_dir, 'data-10m') + # send a POST to our PUT handler which will send immediately a 404 back + url = f'https://{env.authority_for(env.domain1, proto)}/curltest/put' + curl = CurlClient(env=env) + r = curl.run_direct(with_stats=True, args=[ + '--resolve', f'{env.authority_for(env.domain1, proto)}:127.0.0.1', + '--cacert', env.ca.cert_file, + '--request', 'POST', + '--max-time', '5', '-v', + '--url', url, + '--form', 'idList=12345678', + '--form', 'pos=top', + '--form', 'name=mr_test', + '--form', f'fileSource=@{fdata};type=application/pdf', + ]) + assert r.exit_code == 0, f'{r}' + r.check_stats(1, 404) + + # issue #11157, send upload that is slowly read in + def test_07_33_issue_11157b(self, env: Env, httpd, nghttpx, repeat): + proto = 'h2' + fdata = os.path.join(env.gen_dir, 'data-10m') + # tell our test PUT handler to read the upload more slowly, so + # that the send buffering and transfer loop needs to wait + url = f'https://{env.authority_for(env.domain1, proto)}/curltest/put?chunk_delay=2ms' + curl = CurlClient(env=env) + r = curl.run_direct(with_stats=True, args=[ + '--resolve', f'{env.authority_for(env.domain1, proto)}:127.0.0.1', + '--cacert', env.ca.cert_file, + '--request', 'PUT', + '--max-time', '5', '-v', + '--url', url, + '--form', 'idList=12345678', + '--form', 'pos=top', + '--form', 'name=mr_test', + '--form', f'fileSource=@{fdata};type=application/pdf', + ]) + assert r.exit_code == 0, f'{r}' + r.check_stats(1, 200) def check_download(self, count, srcfile, curl): for i in range(count):