0cab1359a1
Stefan Eissing
2023-05-20
Subject
http2: upload improvements
Binary
No binary (build failed)
Architecture & Compiler Flags
—
Build Log
$ git checkout -f 0cab1359a1ab9720afccbec6f84e22b409dc9886
Previous HEAD position was b43915b38f runtests: handle interrupted reads from IPC pipes
HEAD is now at 0cab1359a1 http2: upload improvements
$ git clean -fdx
Removing Makefile.in
Removing aclocal.m4
Removing autom4te.cache/
Removing compile
Removing confdefs.h
Removing config.guess
Removing config.log
Removing config.sub
Removing configure
Removing conftest.c
Removing depcomp
Removing docs/Makefile.in
Removing docs/cmdline-opts/Makefile.in
Removing docs/examples/Makefile.in
Removing docs/libcurl/Makefile.in
Removing docs/libcurl/opts/Makefile.in
Removing include/Makefile.in
Removing include/curl/Makefile.in
Removing install-sh
Removing lib/Makefile.in
Removing lib/curl_config.h.in
Removing ltmain.sh
Removing m4/libtool.m4
Removing m4/ltoptions.m4
Removing m4/ltsugar.m4
Removing m4/ltversion.m4
Removing m4/lt~obsolete.m4
Removing missing
Removing packages/Makefile.in
Removing packages/vms/Makefile.in
Removing run-compiler
Removing scripts/Makefile.in
Removing src/Makefile.in
Removing test-driver
Removing tests/Makefile.in
Removing tests/certs/Makefile.in
Removing tests/certs/scripts/Makefile.in
Removing tests/data/Makefile.in
Removing tests/http/Makefile.in
Removing tests/http/clients/Makefile.in
Removing tests/libtest/Makefile.in
Removing tests/server/Makefile.in
Removing tests/unit/Makefile.in
$ autoreconf -fi
libtoolize: putting auxiliary files in '.'.
libtoolize: copying file './ltmain.sh'
libtoolize: putting macros in AC_CONFIG_MACRO_DIRS, 'm4'.
libtoolize: copying file 'm4/libtool.m4'
libtoolize: copying file 'm4/ltoptions.m4'
libtoolize: copying file 'm4/ltsugar.m4'
libtoolize: copying file 'm4/ltversion.m4'
libtoolize: copying file 'm4/lt~obsolete.m4'
libtoolize: Remember to add 'LT_INIT' to configure.ac.
configure.ac:123: installing './compile'
configure.ac:439: installing './config.guess'
configure.ac:439: installing './config.sub'
configure.ac:123: installing './install-sh'
configure.ac:129: installing './missing'
docs/examples/Makefile.am: installing './depcomp'
parallel-tests: installing './test-driver'
$ bash -c find . -name config.guess -exec cp /usr/share/misc/config.guess {} \;
$ bash -c find . -name config.sub -exec cp /usr/share/misc/config.sub {} \;
$ chmod +x configure
$ ./configure --disable-shared --without-ssl --disable-docs
[TIMEOUT]
Commit Diff GitHub →
lib/http2.c+21 -12
| @@ -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: | |
tests/http/test_07_upload.py+44 -1
| @@ -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): | |