From 30de937bda0f65d47059d579f8a21e48ad721e28 Mon Sep 17 00:00:00 2001 From: Stefan Eissing Date: Wed, 22 May 2024 16:52:16 +0200 Subject: [PATCH] transfer: conn close on paused upload - add 2 variations on test_07_42 which PAUSEs uploads and response connections terminating either right away or after the 100-continue response - when detecting the connection being closed in transfer.c readwrite_data(), clear ALL send bits in data->req.keepon. It no longer makes send to wait for a KEEP_SEND_PAUSE or HOLD. - in the protocol client writer add the check for incomplete response bodies. When an EOS is seen and the length is known, check that and fail if bytes are missing. Reported-by: Sergey Bronnikov Fixes #13740 Closes #13750 --- lib/sendf.c | 8 +++++ lib/transfer.c | 5 ++- tests/http/clients/upload-pausing.c | 16 ++++++--- tests/http/test_07_upload.py | 34 +++++++++++++++++-- .../http/testenv/mod_curltest/mod_curltest.c | 32 ++++++++++++++++- 5 files changed, 86 insertions(+), 9 deletions(-) diff --git a/lib/sendf.c b/lib/sendf.c index 68a8bf3b6a..d3db61bf64 100644 --- a/lib/sendf.c +++ b/lib/sendf.c @@ -289,6 +289,13 @@ static CURLcode cw_download_write(struct Curl_easy *data, if(nwrite == wmax) { data->req.download_done = TRUE; } + + if((type & CLIENTWRITE_EOS) && !data->req.no_body && + (data->req.maxdownload > data->req.bytecount)) { + failf(data, "end of response with %" CURL_FORMAT_CURL_OFF_T + " bytes missing", data->req.maxdownload - data->req.bytecount); + return CURLE_PARTIAL_FILE; + } } /* Error on too large filesize is handled below, after writing @@ -694,6 +701,7 @@ static CURLcode cr_in_read(struct Curl_easy *data, return CURLE_READ_ERROR; } /* CURL_READFUNC_PAUSE pauses read callbacks that feed socket writes */ + CURL_TRC_READ(data, "cr_in_read, callback returned CURL_READFUNC_PAUSE"); data->req.keepon |= KEEP_SEND_PAUSE; /* mark socket send as paused */ *pnread = 0; *peos = FALSE; diff --git a/lib/transfer.c b/lib/transfer.c index 744227eb36..7b3500d25d 100644 --- a/lib/transfer.c +++ b/lib/transfer.c @@ -271,7 +271,10 @@ static CURLcode readwrite_data(struct Curl_easy *data, DEBUGF(infof(data, "nread == 0, stream closed, bailing")); else DEBUGF(infof(data, "nread <= 0, server closed connection, bailing")); - k->keepon &= ~(KEEP_RECV|KEEP_SEND); /* stop sending as well */ + /* stop receiving and ALL sending as well, including PAUSE and HOLD. + * We might still be paused on receive client writes though, so + * keep those bits around. */ + k->keepon &= ~(KEEP_RECV|KEEP_SENDBITS); if(k->eos_written) /* already did write this to client, leave */ break; } diff --git a/tests/http/clients/upload-pausing.c b/tests/http/clients/upload-pausing.c index 871fdd382d..8247e4fd9e 100644 --- a/tests/http/clients/upload-pausing.c +++ b/tests/http/clients/upload-pausing.c @@ -133,7 +133,7 @@ static int debug_cb(CURL *handle, curl_infotype type, return 0; } -#define PAUSE_READ_AFTER 10 +#define PAUSE_READ_AFTER 1 static size_t total_read = 0; static size_t read_callback(char *ptr, size_t size, size_t nmemb, @@ -143,11 +143,13 @@ static size_t read_callback(char *ptr, size_t size, size_t nmemb, (void)nmemb; (void)userdata; if(total_read >= PAUSE_READ_AFTER) { + fprintf(stderr, "read_callback, return PAUSE\n"); return CURL_READFUNC_PAUSE; } else { ptr[0] = '\n'; ++total_read; + fprintf(stderr, "read_callback, return 1 byte\n"); return 1; } } @@ -158,13 +160,19 @@ static int progress_callback(void *clientp, double ultotal, double ulnow) { - CURL *curl; (void)dltotal; (void)dlnow; (void)ultotal; (void)ulnow; - curl = (CURL *)clientp; - curl_easy_pause(curl, CURLPAUSE_CONT); + (void)clientp; +#if 0 + /* Used to unpause on progress, but keeping for now. */ + { + CURL *curl = (CURL *)clientp; + curl_easy_pause(curl, CURLPAUSE_CONT); + /* curl_easy_pause(curl, CURLPAUSE_RECV_CONT); */ + } +#endif return 0; } diff --git a/tests/http/test_07_upload.py b/tests/http/test_07_upload.py index 1aa51d0710..95703d3521 100644 --- a/tests/http/test_07_upload.py +++ b/tests/http/test_07_upload.py @@ -464,10 +464,10 @@ class TestUpload: n=1)) assert False, f'download {dfile} differs:\n{diff}' - # upload large data, let connection die while doing it + # upload data, pause, let connection die with an incomplete response # issues #11769 #13260 @pytest.mark.parametrize("proto", ['http/1.1', 'h2', 'h3']) - def test_07_42_upload_disconnect(self, env: Env, httpd, nghttpx, repeat, proto): + def test_07_42a_upload_disconnect(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'): @@ -475,10 +475,38 @@ class TestUpload: client = LocalClient(name='upload-pausing', env=env, timeout=60) if not client.exists(): pytest.skip(f'example client not built: {client.name}') - url = f'http://{env.domain1}:{env.http_port}/curltest/echo?id=[0-0]&die_after=10' + url = f'http://{env.domain1}:{env.http_port}/curltest/echo?id=[0-0]&die_after=0' r = client.run([url]) r.check_exit_code(18) # PARTIAL_FILE + # upload data, pause, let connection die without any response at all + @pytest.mark.parametrize("proto", ['http/1.1', 'h2', 'h3']) + def test_07_42b_upload_disconnect(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") + client = LocalClient(name='upload-pausing', env=env, timeout=60) + if not client.exists(): + pytest.skip(f'example client not built: {client.name}') + url = f'http://{env.domain1}:{env.http_port}/curltest/echo?id=[0-0]&just_die=1' + r = client.run([url]) + r.check_exit_code(52) # GOT_NOTHING + + # upload data, pause, let connection die after 100 continue + @pytest.mark.parametrize("proto", ['http/1.1', 'h2', 'h3']) + def test_07_42c_upload_disconnect(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") + client = LocalClient(name='upload-pausing', env=env, timeout=60) + if not client.exists(): + pytest.skip(f'example client not built: {client.name}') + url = f'http://{env.domain1}:{env.http_port}/curltest/echo?id=[0-0]&die_after_100=1' + r = client.run([url]) + r.check_exit_code(52) # GOT_NOTHING + # 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 abe97a3c0f..09f12256a5 100644 --- a/tests/http/testenv/mod_curltest/mod_curltest.c +++ b/tests/http/testenv/mod_curltest/mod_curltest.c @@ -186,6 +186,7 @@ static int curltest_echo_handler(request_rec *r) char buffer[8192]; const char *ct; apr_off_t die_after_len = -1, total_read_len = 0; + int just_die = 0, die_after_100 = 0; long l; if(strcmp(r->handler, "curltest-echo")) { @@ -208,16 +209,35 @@ static int curltest_echo_handler(request_rec *r) val = s + 1; if(!strcmp("die_after", arg)) { die_after_len = (apr_off_t)apr_atoi64(val); + continue; + } + else if(!strcmp("just_die", arg)) { + just_die = 1; + continue; + } + else if(!strcmp("die_after_100", arg)) { + die_after_100 = 1; + continue; } } } } + if(just_die) { + ap_log_rerror(APLOG_MARK, APLOG_INFO, 0, r, + "echo_handler: dying right away"); + /* Generate no HTTP response at all. */ + ap_remove_output_filter_byhandle(r->output_filters, "HTTP_HEADER"); + r->connection->keepalive = AP_CONN_CLOSE; + return AP_FILTER_ERROR; + } + r->status = 200; if(die_after_len >= 0) { r->clength = die_after_len + 1; r->chunked = 0; - apr_table_set(r->headers_out, "Content-Length", apr_ltoa(r->pool, (long)r->clength)); + apr_table_set(r->headers_out, "Content-Length", + apr_ltoa(r->pool, (long)r->clength)); } else { r->clength = -1; @@ -235,6 +255,14 @@ static int curltest_echo_handler(request_rec *r) bb = apr_brigade_create(r->pool, c->bucket_alloc); /* copy any request body into the response */ if((rv = ap_setup_client_block(r, REQUEST_CHUNKED_DECHUNK))) goto cleanup; + if(die_after_100) { + ap_log_rerror(APLOG_MARK, APLOG_INFO, 0, r, + "echo_handler: dying after 100-continue"); + /* Generate no HTTP response at all. */ + ap_remove_output_filter_byhandle(r->output_filters, "HTTP_HEADER"); + r->connection->keepalive = AP_CONN_CLOSE; + return AP_FILTER_ERROR; + } if(ap_should_client_block(r)) { while(0 < (l = ap_get_client_block(r, &buffer[0], sizeof(buffer)))) { total_read_len += l; @@ -242,6 +270,8 @@ static int curltest_echo_handler(request_rec *r) ap_log_rerror(APLOG_MARK, APLOG_INFO, 0, r, "echo_handler: dying after %ld bytes as requested", (long)total_read_len); + ap_pass_brigade(r->output_filters, bb); + ap_remove_output_filter_byhandle(r->output_filters, "HTTP_HEADER"); r->connection->keepalive = AP_CONN_CLOSE; return DONE; } -- 2.47.3