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

diff --git a/lib/sendf.c b/lib/sendf.c
index 68a8bf3b6ab6ee326f0b798b37033013058c0f40..d3db61bf6404db44d1ded722a3fdb30b014b3a0e 100644 (file)
--- 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 744227eb368a8fd5c9f30bb15cd6125436627e5f..7b3500d25dbb80f3ddd16db496dbd63f38be7b18 100644 (file)
--- 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 871fdd382da96deb650d3938370e2be52e732d3c..8247e4fd9efbef1b6992684c8e0b7798ee517b76 100644 (file)
--- 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 1aa51d07106162f6409db582a645b0e5c3e9f179..95703d3521f487eb54e1b9ffd38df543cc03c367 100644 (file)
--- 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 abe97a3c0f922ed0c8d38a8818503102ca2f5827..09f12256a5a77d85e4ed5b84a9fe229c017880f2 100644 (file)
--- 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;
       }