From: Stefan Eissing <stefan@eissing.org>
Date: Fri, 23 May 2025 14:06:57 +0000 (+0200)
Subject: ftp: fix race in upload handling
X-Git-Tag: curl-8_14_0~10
X-Git-Url: http://git.ipfire.org/gitweb/gitweb.cgi?a=commitdiff_plain;h=745c3519a5942853cb96817d6545ba8aaf1cb3f0;p=thirdparty%2Fcurl.git

ftp: fix race in upload handling

When TYPE was skipped for an immediate STORE command and the server
replied fast and the EPRT data connection was not ready, the transfer
was not initated, leading to no upload.

Fixes #17394
Closes #17428
Reported-by: JoelAtWisetech on github
---

diff --git a/lib/ftp.c b/lib/ftp.c
index e28236da01..04c0ad7baa 100644
--- a/lib/ftp.c
+++ b/lib/ftp.c
@@ -3657,7 +3657,11 @@ static CURLcode ftp_do_more(struct Curl_easy *data, int *completep)
         return result;
 
       result = ftp_statemach(data, ftpc, &complete);
-      *completep = (int)complete;
+      /* ftp_nb_type() might have skipped sending `TYPE A|I` when not
+       * deemed necessary and directly sent `STORE name`. If this was
+       * then complete, but we are still waiting on the data connection,
+       * the transfer has not been initiated yet. */
+      *completep = (int)(ftpc->wait_data_conn ? 0 : complete);
     }
     else {
       /* download */
diff --git a/tests/http/test_30_vsftpd.py b/tests/http/test_30_vsftpd.py
index 79fe335066..9c8138cf45 100644
--- a/tests/http/test_30_vsftpd.py
+++ b/tests/http/test_30_vsftpd.py
@@ -185,7 +185,7 @@ class TestVsFTPD:
         r.check_stats(count=count, http_status=226)
         self.check_downloads(curl, srcfile, count)
 
-    def test_30_09_active_upload(self, env: Env, vsftpd: VsFTPD):
+    def test_30_09_active_up_file(self, env: Env, vsftpd: VsFTPD):
         docname = 'upload-1k'
         curl = CurlClient(env=env)
         srcfile = os.path.join(env.gen_dir, docname)
@@ -199,6 +199,20 @@ class TestVsFTPD:
         r.check_stats(count=count, http_status=226)
         self.check_upload(env, vsftpd, docname=docname)
 
+    def test_30_10_active_up_ascii(self, env: Env, vsftpd: VsFTPD):
+        docname = 'upload-1k'
+        curl = CurlClient(env=env)
+        srcfile = os.path.join(env.gen_dir, docname)
+        dstfile = os.path.join(vsftpd.docs_dir, docname)
+        self._rmf(dstfile)
+        count = 1
+        url = f'ftp://{env.ftp_domain}:{vsftpd.port}/'
+        r = curl.ftp_upload(urls=[url], fupload=f'{srcfile}', with_stats=True, extra_args=[
+            '--ftp-port', '127.0.0.1', '--use-ascii'
+        ])
+        r.check_stats(count=count, http_status=226)
+        self.check_upload(env, vsftpd, docname=docname, binary=False)
+
     def check_downloads(self, client, srcfile: str, count: int,
                         complete: bool = True):
         for i in range(count):
@@ -212,7 +226,7 @@ class TestVsFTPD:
                                                     n=1))
                 assert False, f'download {dfile} differs:\n{diff}'
 
-    def check_upload(self, env, vsftpd: VsFTPD, docname):
+    def check_upload(self, env, vsftpd: VsFTPD, docname, binary=True):
         srcfile = os.path.join(env.gen_dir, docname)
         dstfile = os.path.join(vsftpd.docs_dir, docname)
         assert os.path.exists(srcfile)
@@ -223,4 +237,4 @@ class TestVsFTPD:
                                                 fromfile=srcfile,
                                                 tofile=dstfile,
                                                 n=1))
-            assert False, f'upload {dstfile} differs:\n{diff}'
+            assert not binary and len(diff) == 0, f'upload {dstfile} differs:\n{diff}'