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 e28236da01f34727d03ce5fc5bf514e7ccdc79d6..04c0ad7baa97247a041559fee747bc08f5d0be04 100644 (file)
--- 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 79fe33506661d618c9ec8854ce89a5a6edd82f2f..9c8138cf459e614630f45fee23f9f3363da7c469 100644 (file)
--- 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}'