transfer: fix retry for empty downloads on reuse

When a reused connection did transfer 0 bytes, it assumed the transfer
had failed and needed a retry. Add a check for data->red.done, so we can
successfully accept the transfer of a 0-length file via SFTP.

Add test case 1583 to verfiy.

Fix SFTP disconnect debug trace when there was nothing to
disconnect (like when reusing a connection).

Fixes #19165
Reported-by: Alexander Blach
Closes #19189

diff --git a/lib/transfer.c b/lib/transfer.c
index d7014aab87e5951a4fdf4d2e552e8a1a4f8d8a4a..0269a4c2505fc54c8c132aa2c6cc9d0f386f8939 100644 (file)
--- a/lib/transfer.c
+++ b/lib/transfer.c
@@ -254,6 +254,7 @@ static CURLcode sendrecv_dl(struct Curl_easy *data,
   curl_off_t total_received = 0;
   bool is_multiplex = FALSE;
   bool rcvd_eagain = FALSE;
+  bool is_eos = FALSE;
 
   result = Curl_multi_xfer_buf_borrow(data, &xfer_buf, &xfer_blen);
   if(result)
@@ -262,7 +263,6 @@ static CURLcode sendrecv_dl(struct Curl_easy *data,
   /* This is where we loop until we have read everything there is to
      read or we get a CURLE_AGAIN */
   do {
-    bool is_eos = FALSE;
     size_t bytestoread;
     ssize_t nread;
 
@@ -309,9 +309,14 @@ static CURLcode sendrecv_dl(struct Curl_easy *data,
     blen = (size_t)nread;
     is_eos = (blen == 0);
 
-    if(!blen) {
-      /* if we receive 0 or less here, either the data transfer is done or the
-         server closed the connection and we bail out from this! */
+    if(!blen && (conn->recv[FIRSTSOCKET] == Curl_cf_recv)) {
+      /* if we receive 0 or less here and the protocol handler did not
+         replace the connection's `recv` callback, either the data transfer
+         is done or the server closed the connection and
+         we bail out from this!
+         With a `recv` replacement, we assume the protocol handler knows
+         what it is doing and a 0-length receive is fine. For example,
+         SFTP downloads of an empty file would show this. See #19165. */
       if(is_multiplex)
         DEBUGF(infof(data, "nread == 0, stream closed, bailing"));
       else
@@ -340,9 +345,9 @@ static CURLcode sendrecv_dl(struct Curl_easy *data,
 
   } while(maxloops--);
 
-  if(!Curl_xfer_is_blocked(data) &&
+  if(!is_eos && !Curl_xfer_is_blocked(data) &&
      (!rcvd_eagain || data_pending(data, rcvd_eagain))) {
-    /* Did not read until EAGAIN or there is still data pending
+    /* Did not read until EAGAIN/EOS or there is still data pending
      * in buffers. Mark as read-again via simulated SELECT results. */
     Curl_multi_mark_dirty(data);
     CURL_TRC_M(data, "sendrecv_dl() no EAGAIN/pending data, mark as dirty");
@@ -662,9 +667,10 @@ CURLcode Curl_retry_request(struct Curl_easy *data, char **url)
      !(conn->handler->protocol&(PROTO_FAMILY_HTTP|CURLPROTO_RTSP)))
     return CURLE_OK;
 
-  if((data->req.bytecount + data->req.headerbytecount == 0) &&
-     conn->bits.reuse &&
-     (!data->req.no_body || (conn->handler->protocol & PROTO_FAMILY_HTTP))
+  if(conn->bits.reuse &&
+     (data->req.bytecount + data->req.headerbytecount == 0) &&
+     ((!data->req.no_body && !data->req.done) ||
+      (conn->handler->protocol & PROTO_FAMILY_HTTP))
 #ifndef CURL_DISABLE_RTSP
      && (data->set.rtspreq != RTSPREQ_RECEIVE)
 #endif

diff --git a/lib/vssh/libssh2.c b/lib/vssh/libssh2.c
index f9160944befe9424d47b6eb1c61cc4934dd7f0b8..c0335db9c103dceefa424f94790361563cb9055d 100644 (file)
--- a/lib/vssh/libssh2.c
+++ b/lib/vssh/libssh2.c
@@ -3837,18 +3837,16 @@ static CURLcode sftp_disconnect(struct Curl_easy *data,
   struct SSHPROTO *sshp = Curl_meta_get(data, CURL_META_SSH_EASY);
   (void)dead_connection;
 
-  DEBUGF(infof(data, "SSH DISCONNECT starts now"));
-
-  if(sshc && sshc->ssh_session && sshp) {
-    /* only if there is a session still around to use! */
-    myssh_state(data, sshc, SSH_SFTP_SHUTDOWN);
-    result = ssh_block_statemach(data, sshc, sshp, TRUE);
-  }
-
-  DEBUGF(infof(data, "SSH DISCONNECT is done"));
-  if(sshc)
+  if(sshc) {
+    if(sshc->ssh_session && sshp) {
+      /* only if there is a session still around to use! */
+      DEBUGF(infof(data, "SSH DISCONNECT starts now"));
+      myssh_state(data, sshc, SSH_SFTP_SHUTDOWN);
+      result = ssh_block_statemach(data, sshc, sshp, TRUE);
+      DEBUGF(infof(data, "SSH DISCONNECT is done -> %d", result));
+    }
     sshc_cleanup(sshc, data, TRUE);
-
+  }
   return result;
 
 }

diff --git a/tests/data/Makefile.am b/tests/data/Makefile.am
index 4c13c14fd92603fe7f630944462d6cae2518ec07..595943fb6717bc3f9f5443eb446d7edac2fff637 100644 (file)
--- a/tests/data/Makefile.am
+++ b/tests/data/Makefile.am
@@ -209,7 +209,7 @@ test1548 test1549 test1550 test1551 test1552 test1553 test1554 test1555 \
 test1556 test1557 test1558 test1559 test1560 test1561 test1562 test1563 \
 test1564 test1565 test1566 test1567 test1568 test1569 test1570 test1571 \
 test1572 test1573 test1574 test1575 test1576 test1577 test1578 test1579 \
-test1580 test1581 test1582 \
+test1580 test1581 test1582 test1583 \
 \
 test1590 test1591 test1592 test1593 test1594 test1595 test1596 test1597 \
 test1598 test1599 test1600 test1601 test1602 test1603 test1604 test1605 \

diff --git a/tests/data/test1583 b/tests/data/test1583
new file mode 100644 (file)
index 0000000..b266d7e
--- /dev/null
+++ b/tests/data/test1583
@@ -0,0 +1,37 @@
+<testcase>
+<info>
+<keywords>
+SFTP
+</keywords>
+</info>
+
+#
+# Server-side
+<reply>
+</reply>
+
+#
+# Client-side
+<client>
+<server>
+sftp
+</server>
+<precheck>
+%PERL %SRCDIR/libtest/test613.pl prepare %PWD/%LOGDIR/test%TESTNUMBER.dir
+</precheck>
+<name>
+SFTP dir and empty file
+</name>
+<command>
+--key %LOGDIR/server/curl_client_key --pubkey %LOGDIR/server/curl_client_key.pub -u %USER: --insecure sftp://%HOSTIP:%SSHPORT%SFTP_PWD/%LOGDIR/test%TESTNUMBER.dir/ --next --key %LOGDIR/server/curl_client_key --pubkey %LOGDIR/server/curl_client_key.pub -u %USER: --insecure sftp://%HOSTIP:%SSHPORT%SFTP_PWD/%LOGDIR/test%TESTNUMBER.dir/emptyfile.txt
+</command>
+</client>
+
+#
+# Verify data after the test has been "shot"
+<verify>
+<errorcode>
+0
+</errorcode>
+</verify>
+</testcase>

diff --git a/tests/data/test613 b/tests/data/test613
index 9161bafd4603b6738b3e4e8b191e1c84d90b475f..92c7905065fbd7fa1b87054d127a3b993143b97c 100644 (file)
--- a/tests/data/test613
+++ b/tests/data/test613
@@ -11,6 +11,7 @@ directory
 <reply>
 <datacheck>
 d?????????    N U         U               N ???  N NN:NN asubdir
+-rw???????    1 U         U               0 Jan  1  2000 emptyfile.txt
 -rw???????    1 U         U              37 Jan  1  2000 plainfile.txt
 -r-???????    1 U         U              47 Dec 31  2000 rofile.txt
 </datacheck>

diff --git a/tests/data/test614 b/tests/data/test614
index ad270c6c85ef653582c1ff41cd376b553b826999..a113ecdd45747b3d6cdfa19ac3a6750f58db8dd7 100644 (file)
--- a/tests/data/test614
+++ b/tests/data/test614
@@ -12,6 +12,7 @@ directory
 <reply>
 <datacheck>
 d?????????    N U         U               N ???  N NN:NN asubdir
+-rw???????    1 U         U               0 Jan  1  2000 emptyfile.txt
 -r-???????    1 U         U              37 Jan  1  2000 plainfile.txt
 -r-???????    1 U         U              47 Dec 31  2000 rofile.txt
 </datacheck>

diff --git a/tests/libtest/test613.pl b/tests/libtest/test613.pl
index 48179833f3fc52e076d449bb8ddf916b9f2fc72f..d45e2e4e960a8ca96dfb41e65f416991f0fc8181 100755 (executable)
--- a/tests/libtest/test613.pl
+++ b/tests/libtest/test613.pl
@@ -59,6 +59,14 @@ if($ARGV[0] eq "prepare") {
     utime time, timegm(0,0,12,1,0,100), "plainfile.txt";
     chmod 0666, "plainfile.txt";
 
+    open(FILE, ">emptyfile.txt") || errout "$!";
+    binmode FILE;
+    close(FILE);
+    # The mtime is specifically chosen to be an even number so that it can be
+    # represented exactly on a FAT file system.
+    utime time, timegm(0,0,12,1,0,100), "emptyfile.txt";
+    chmod 0666, "emptyfile.txt";
+
     open(FILE, ">rofile.txt") || errout "$!";
     binmode FILE;
     print FILE "Read-only test file to support curl test suite\n";
@@ -83,6 +91,7 @@ elsif($ARGV[0] eq "postprocess") {
     }
     chmod 0666, "$dirname/rofile.txt";
     unlink "$dirname/rofile.txt";
+    unlink "$dirname/emptyfile.txt";
     unlink "$dirname/plainfile.txt";
     rmdir "$dirname/asubdir";