mirror/curl
2
0
Fork 0
mirror of https://github.com/curl/curl.git synced 2024-12-15 06:40:09 +08:00
Code Issues Packages Projects Releases Wiki Activity

http2: upload improvements

Browse Source 
Make send buffer smaller to have progress and "upload done" reporting
closer to reality. Fix handling of send "drain" condition to no longer
trigger once the transfer loop reports it is done sending. Also do not
trigger the send "drain" on RST streams.

Background:
- a upload stall was reported in #11157 that timed out
- test_07_33a reproduces a problem with such a stall if the
  server 404s the request and RSTs the stream.
- test_07_33b verifies a successful PUT, using the parameters
  from #11157 and checks success

Ref: #11157
Closes #11165
This commit is contained in:
Stefan Eissing 2023-05-20 12:26:04 +02:00 committed by Daniel Stenberg
parent 1886eef7fa
commit 0cab1359a1
No known key found for this signature in database
GPG Key ID: 5CC908FDB71E12C2
2 changed files with 65 additions and 13 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

33
lib/http2.c
View File

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

45
tests/http/test_07_upload.py
View File

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