From 0a79a599a9c09f7173e82eec23607742667a9944 Mon Sep 17 00:00:00 2001 From: Stefan Eissing Date: Wed, 22 Oct 2025 12:37:59 +0200 Subject: [PATCH] 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 --- lib/transfer.c | 24 +++++++++++++++--------- lib/vssh/libssh2.c | 20 +++++++++----------- tests/data/Makefile.am | 2 +- tests/data/test1583 | 37 +++++++++++++++++++++++++++++++++++++ tests/data/test613 | 1 + tests/data/test614 | 1 + tests/libtest/test613.pl | 9 +++++++++ 7 files changed, 73 insertions(+), 21 deletions(-) create mode 100644 tests/data/test1583 diff --git a/lib/transfer.c b/lib/transfer.c index d7014aab87..0269a4c250 100644 --- 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 f9160944be..c0335db9c1 100644 --- 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 4c13c14fd9..595943fb67 100644 --- 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 index 0000000000..b266d7eea8 --- /dev/null +++ b/tests/data/test1583 @@ -0,0 +1,37 @@ + + + +SFTP + + + +# +# Server-side + + + +# +# Client-side + + +sftp + + +%PERL %SRCDIR/libtest/test613.pl prepare %PWD/%LOGDIR/test%TESTNUMBER.dir + + +SFTP dir and empty file + + +--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 + + + +# +# Verify data after the test has been "shot" + + +0 + + + diff --git a/tests/data/test613 b/tests/data/test613 index 9161bafd46..92c7905065 100644 --- a/tests/data/test613 +++ b/tests/data/test613 @@ -11,6 +11,7 @@ directory 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 diff --git a/tests/data/test614 b/tests/data/test614 index ad270c6c85..a113ecdd45 100644 --- a/tests/data/test614 +++ b/tests/data/test614 @@ -12,6 +12,7 @@ directory 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 diff --git a/tests/libtest/test613.pl b/tests/libtest/test613.pl index 48179833f3..d45e2e4e96 100755 --- 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"; -- 2.47.3