request: paused upload on completed download, assess connection

A transfer with a completed download that is still uploading needs to
check the connection state when it is PAUSEd, since connection
close/errors would otherwise go unnoticed.

Reported-by: Sergey Bronnikov
Fixes #13260
Closes #13271

lib/request.c

@@ -395,6 +395,7 @@ CURLcode Curl_req_send_more(struct Curl_easy *data)
   result = req_flush(data);
   if(result == CURLE_AGAIN)
     result = CURLE_OK;
+
   return result;
 }
 

lib/transfer.c

@@ -272,10 +272,9 @@ 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"));
-      if(k->eos_written) { /* already did write this to client, leave */
-        k->keepon = 0; /* stop sending as well */
+      k->keepon = 0; /* stop sending as well */
+      if(k->eos_written) /* already did write this to client, leave */
         break;
-      }
     }
     total_received += blen;
 

tests/http/clients/.gitignore

@@ -9,3 +9,4 @@ ws-pingpong
 h2-upgrade-extreme
 tls-session-reuse
 h2-pausing
+upload-pausing

tests/http/clients/Makefile.inc

@@ -30,4 +30,5 @@ check_PROGRAMS = \
   ws-pingpong \
   h2-upgrade-extreme \
   tls-session-reuse \
-  h2-pausing
+  h2-pausing \
+  upload-pausing

tests/http/clients/upload-pausing.c

@@ -0,0 +1,261 @@
+/***************************************************************************
+ *                                  _   _ ____  _
+ *  Project                     ___| | | |  _ \| |
+ *                             / __| | | | |_) | |
+ *                            | (__| |_| |  _ <| |___
+ *                             \___|\___/|_| \_\_____|
+ *
+ * Copyright (C) Daniel Stenberg, <daniel@haxx.se>, et al.
+ *
+ * This software is licensed as described in the file COPYING, which
+ * you should have received as part of this distribution. The terms
+ * are also available at https://curl.se/docs/copyright.html.
+ *
+ * You may opt to use, copy, modify, merge, publish, distribute and/or sell
+ * copies of the Software, and permit persons to whom the Software is
+ * furnished to do so, under the terms of the COPYING file.
+ *
+ * This software is distributed on an "AS IS" basis, WITHOUT WARRANTY OF ANY
+ * KIND, either express or implied.
+ *
+ * SPDX-License-Identifier: curl
+ *
+ ***************************************************************************/
+/* <DESC>
+ * upload pausing
+ * </DESC>
+ */
+/* This is based on the poc client of issue #11769
+ */
+#include <stdio.h>
+#include <string.h>
+#include <sys/time.h>
+#include <unistd.h>
+#include <stdlib.h>
+#include <curl/curl.h>
+#include <curl/mprintf.h>
+
+static void log_line_start(FILE *log, const char *idsbuf, curl_infotype type)
+{
+  /*
+   * This is the trace look that is similar to what libcurl makes on its
+   * own.
+   */
+  static const char * const s_infotype[] = {
+    "* ", "< ", "> ", "{ ", "} ", "{ ", "} "
+  };
+  if(idsbuf && *idsbuf)
+    fprintf(log, "%s%s", idsbuf, s_infotype[type]);
+  else
+    fputs(s_infotype[type], log);
+}
+
+#define TRC_IDS_FORMAT_IDS_1  "[%" CURL_FORMAT_CURL_OFF_T "-x] "
+#define TRC_IDS_FORMAT_IDS_2  "[%" CURL_FORMAT_CURL_OFF_T "-%" \
+                                   CURL_FORMAT_CURL_OFF_T "] "
+/*
+** callback for CURLOPT_DEBUGFUNCTION
+*/
+static int debug_cb(CURL *handle, curl_infotype type,
+                    char *data, size_t size,
+                    void *userdata)
+{
+  FILE *output = stderr;
+  static int newl = 0;
+  static int traced_data = 0;
+  char idsbuf[60];
+  curl_off_t xfer_id, conn_id;
+
+  (void)handle; /* not used */
+  (void)userdata;
+
+  if(!curl_easy_getinfo(handle, CURLINFO_XFER_ID, &xfer_id) && xfer_id >= 0) {
+    if(!curl_easy_getinfo(handle, CURLINFO_CONN_ID, &conn_id) &&
+        conn_id >= 0) {
+      curl_msnprintf(idsbuf, sizeof(idsbuf), TRC_IDS_FORMAT_IDS_2,
+                     xfer_id, conn_id);
+    }
+    else {
+      curl_msnprintf(idsbuf, sizeof(idsbuf), TRC_IDS_FORMAT_IDS_1, xfer_id);
+    }
+  }
+  else
+    idsbuf[0] = 0;
+
+  switch(type) {
+  case CURLINFO_HEADER_OUT:
+    if(size > 0) {
+      size_t st = 0;
+      size_t i;
+      for(i = 0; i < size - 1; i++) {
+        if(data[i] == '\n') { /* LF */
+          if(!newl) {
+            log_line_start(output, idsbuf, type);
+          }
+          (void)fwrite(data + st, i - st + 1, 1, output);
+          st = i + 1;
+          newl = 0;
+        }
+      }
+      if(!newl)
+        log_line_start(output, idsbuf, type);
+      (void)fwrite(data + st, i - st + 1, 1, output);
+    }
+    newl = (size && (data[size - 1] != '\n')) ? 1 : 0;
+    traced_data = 0;
+    break;
+  case CURLINFO_TEXT:
+  case CURLINFO_HEADER_IN:
+    if(!newl)
+      log_line_start(output, idsbuf, type);
+    (void)fwrite(data, size, 1, output);
+    newl = (size && (data[size - 1] != '\n')) ? 1 : 0;
+    traced_data = 0;
+    break;
+  case CURLINFO_DATA_OUT:
+  case CURLINFO_DATA_IN:
+  case CURLINFO_SSL_DATA_IN:
+  case CURLINFO_SSL_DATA_OUT:
+    if(!traced_data) {
+      if(!newl)
+        log_line_start(output, idsbuf, type);
+      fprintf(output, "[%ld bytes data]\n", (long)size);
+      newl = 0;
+      traced_data = 1;
+    }
+    break;
+  default: /* nada */
+    newl = 0;
+    traced_data = 1;
+    break;
+  }
+
+  return 0;
+}
+
+#define PAUSE_READ_AFTER  10
+static size_t total_read = 0;
+
+static size_t read_callback(char *ptr, size_t size, size_t nmemb,
+                            void *userdata)
+{
+  (void)size;
+  (void)nmemb;
+  (void)userdata;
+  if(total_read >= PAUSE_READ_AFTER) {
+    return CURL_READFUNC_PAUSE;
+  }
+  else {
+    ptr[0] = '\n';
+    ++total_read;
+    return 1;
+  }
+}
+
+static int progress_callback(void *clientp,
+                             double dltotal,
+                             double dlnow,
+                             double ultotal,
+                             double ulnow)
+{
+  CURL *curl;
+  (void)dltotal;
+  (void)dlnow;
+  (void)ultotal;
+  (void)ulnow;
+  curl = (CURL *)clientp;
+  curl_easy_pause(curl, CURLPAUSE_CONT);
+  return 0;
+}
+
+static int err(void)
+{
+  fprintf(stderr, "something unexpected went wrong - bailing out!\n");
+  exit(2);
+}
+
+
+
+int main(int argc, char *argv[])
+{
+  CURL *curl;
+  CURLcode rc = CURLE_OK;
+  CURLU *cu;
+  struct curl_slist *resolve = NULL;
+  char resolve_buf[1024];
+  char *url, *host = NULL, *port = NULL;
+
+  if(argc != 2) {
+    fprintf(stderr, "ERROR: need URL as argument\n");
+    return 2;
+  }
+  url = argv[1];
+
+  curl_global_init(CURL_GLOBAL_DEFAULT);
+  curl_global_trace("ids,time");
+
+  cu = curl_url();
+  if(!cu) {
+    fprintf(stderr, "out of memory\n");
+    exit(1);
+  }
+  if(curl_url_set(cu, CURLUPART_URL, url, 0)) {
+    fprintf(stderr, "not a URL: '%s'\n", url);
+    exit(1);
+  }
+  if(curl_url_get(cu, CURLUPART_HOST, &host, 0)) {
+    fprintf(stderr, "could not get host of '%s'\n", url);
+    exit(1);
+  }
+  if(curl_url_get(cu, CURLUPART_PORT, &port, 0)) {
+    fprintf(stderr, "could not get port of '%s'\n", url);
+    exit(1);
+  }
+  memset(&resolve, 0, sizeof(resolve));
+  curl_msnprintf(resolve_buf, sizeof(resolve_buf)-1,
+                 "%s:%s:127.0.0.1", host, port);
+  resolve = curl_slist_append(resolve, resolve_buf);
+
+  curl = curl_easy_init();
+  if(!curl) {
+    fprintf(stderr, "out of memory\n");
+    exit(1);
+  }
+  /* We want to use our own read function. */
+  curl_easy_setopt(curl, CURLOPT_READFUNCTION, read_callback);
+
+  /* It will help us to continue the read function. */
+  curl_easy_setopt(curl, CURLOPT_XFERINFOFUNCTION, progress_callback);
+  curl_easy_setopt(curl, CURLOPT_XFERINFODATA, curl);
+  curl_easy_setopt(curl, CURLOPT_NOPROGRESS, 0L);
+
+  /* It will help us to ensure that keepalive does not help. */
+  curl_easy_setopt(curl, CURLOPT_TCP_KEEPALIVE, 1L);
+  curl_easy_setopt(curl, CURLOPT_TCP_KEEPIDLE, 1L);
+  curl_easy_setopt(curl, CURLOPT_TCP_KEEPINTVL, 1L);
+
+  /* Enable uploading. */
+  curl_easy_setopt(curl, CURLOPT_CUSTOMREQUEST, "POST");
+  curl_easy_setopt(curl, CURLOPT_UPLOAD, 1L);
+
+  if(curl_easy_setopt(curl, CURLOPT_VERBOSE, 1L) != CURLE_OK ||
+     curl_easy_setopt(curl, CURLOPT_DEBUGFUNCTION, debug_cb)
+     != CURLE_OK ||
+     curl_easy_setopt(curl, CURLOPT_RESOLVE, resolve) != CURLE_OK)
+    err();
+
+  curl_easy_setopt(curl, CURLOPT_URL, url);
+  rc = curl_easy_perform(curl);
+
+  if(curl) {
+    curl_easy_cleanup(curl);
+  }
+
+  curl_slist_free_all(resolve);
+  curl_free(host);
+  curl_free(port);
+  curl_url_cleanup(cu);
+  curl_global_cleanup();
+
+  return (int)rc;
+}

tests/http/test_07_upload.py

@@ -31,7 +31,7 @@ import os
 import time
 import pytest
 
-from testenv import Env, CurlClient
+from testenv import Env, CurlClient, LocalClient
 
 
 log = logging.getLogger(__name__)
@@ -464,6 +464,21 @@ class TestUpload:
                                                     n=1))
                 assert False, f'download {dfile} differs:\n{diff}'
 
+    # upload large data, let connection die while doing it
+    # 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):
+        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=10'
+        r = client.run([url])
+        r.check_exit_code(18)  # PARTIAL_FILE
+
     # 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):

tests/http/testenv/mod_curltest/mod_curltest.c

@@ -21,6 +21,8 @@
  * SPDX-License-Identifier: curl
  *
  ***************************************************************************/
+#include <assert.h>
+
 #include <apr_optional.h>
 #include <apr_optional_hooks.h>
 #include <apr_strings.h>
@@ -181,6 +183,7 @@ static int curltest_echo_handler(request_rec *r)
   apr_status_t rv;
   char buffer[8192];
   const char *ct;
+  apr_off_t die_after_len = -1, total_read_len = 0;
   long l;
 
   if(strcmp(r->handler, "curltest-echo")) {
@@ -191,10 +194,34 @@ static int curltest_echo_handler(request_rec *r)
   }
 
   ap_log_rerror(APLOG_MARK, APLOG_TRACE1, 0, r, "echo_handler: processing");
+  if(r->args) {
+    apr_array_header_t *args = NULL;
+    int i;
+    args = apr_cstr_split(r->args, "&", 1, r->pool);
+    for(i = 0; i < args->nelts; ++i) {
+      char *s, *val, *arg = APR_ARRAY_IDX(args, i, char*);
+      s = strchr(arg, '=');
+      if(s) {
+        *s = '\0';
+        val = s + 1;
+        if(!strcmp("die_after", arg)) {
+          die_after_len = (apr_off_t)apr_atoi64(val);
+        }
+      }
+    }
+  }
+
   r->status = 200;
-  r->clength = -1;
-  r->chunked = 1;
-  apr_table_unset(r->headers_out, "Content-Length");
+  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));
+  }
+  else {
+    r->clength = -1;
+    r->chunked = 1;
+    apr_table_unset(r->headers_out, "Content-Length");
+  }
   /* Discourage content-encodings */
   apr_table_unset(r->headers_out, "Content-Encoding");
   apr_table_setn(r->subprocess_env, "no-brotli", "1");
@@ -208,6 +235,14 @@ static int curltest_echo_handler(request_rec *r)
   if((rv = ap_setup_client_block(r, REQUEST_CHUNKED_DECHUNK))) goto cleanup;
   if(ap_should_client_block(r)) {
     while(0 < (l = ap_get_client_block(r, &buffer[0], sizeof(buffer)))) {
+      total_read_len += l;
+      if(die_after_len >= 0 && total_read_len >= die_after_len) {
+        ap_log_rerror(APLOG_MARK, APLOG_INFO, 0, r,
+                      "echo_handler: dying after %ld bytes as requested",
+                      (long)total_read_len);
+        r->connection->keepalive = AP_CONN_CLOSE;
+        return DONE;
+      }
       ap_log_rerror(APLOG_MARK, APLOG_TRACE1, 0, r,
                     "echo_handler: copying %ld bytes from request body", l);
       rv = apr_brigade_write(bb, NULL, NULL, buffer, l);