Hi.
I could not make a GitHub pull request at this point in time, I hope it is
acceptable to send the patch this way instead.
The patch is attached.
This reverts commit 49f3117a238b6eac0e22a32f50699a9eddcb66ab from 2019, that
introduced a subtle logic error in FTP file upload handling in active transfer
mode. Longer description of the error is in the commit message.
There are some other similar commits made at the same time, related to
https://github.com/curl/curl/issues/4374, that may be wrong as well. I have not
looked into that. You may want to review them a second time.
From 5cec7646b40988ed60c34d63a911d635b2dff724 Mon Sep 17 00:00:00 2001
From: Jonathan Wernberg <jonathan.wernb...@axis.com>
Date: Wed, 7 Jul 2021 14:55:33 +0200
Subject: [PATCH] Revert "ftp: Expression 'ftpc->wait_data_conn' is always
false"
The reverted commit introduced a logic error in code that was
correct.
The client using libcurl would notice the error since FTP file
uploads in active transfer mode would somtimes complete with
success despite no transfer having been performed and the
"uploaded" file thus not being on the remote server afterwards.
The FTP server would notice the error because it receives a
RST on the data connection it has established with the client
before any data was transferred at all.
The logic error happens if the STOR response from the server have
arrived by the time ftp_multi_statemach() in the affected code path
is called, but the incoming data connection have not arrived yet.
In that case, the processing of the STOR response will cause
'ftpc->wait_data_conn' to be set to TRUE, contradicting the comment
in the code. Since 'complete' will also be set, later logic would
believe the transfer was done.
In most cases, the STOR response will not have arrived yet when
the affected code path is executed, or the incoming connection will
also have arrived, and thus the error would not express itself.
But if the speed difference of the device using libcurl and the
FTP server is exactly right, the error may happen as often as in
one out of hundred file transfers.
This reverts commit 49f3117a238b6eac0e22a32f50699a9eddcb66ab.
---
lib/ftp.c | 9 +++++++--
1 file changed, 7 insertions(+), 2 deletions(-)
diff --git a/lib/ftp.c b/lib/ftp.c
index 444cf35f5..f48741408 100644
--- a/lib/ftp.c
+++ b/lib/ftp.c
@@ -3644,8 +3644,13 @@ static CURLcode ftp_do_more(struct Curl_easy *data, int *completep)
return result;
result = ftp_multi_statemach(data, &complete);
- /* ftpc->wait_data_conn is always false here */
- *completep = (int)complete;
+ if(ftpc->wait_data_conn)
+ /* if we reach the end of the FTP state machine here, *complete will be
+ TRUE but so is ftpc->wait_data_conn, which says we need to wait for
+ the data connection and therefore we're not actually complete */
+ *completep = 0;
+ else
+ *completep = (int)complete;
}
else {
/* download */
--
2.20.1
-------------------------------------------------------------------
Unsubscribe: https://cool.haxx.se/list/listinfo/curl-library
Etiquette: https://curl.se/mail/etiquette.html
