From: Stefan Eissing <stefan@eissing.org>
Date: Wed, 4 Sep 2024 14:06:16 +0000 (+0200)
Subject: cf-socket: fix listen pollset for FTP active mode
X-Git-Tag: curl-8_10_0~41
X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=db5eae1127eb5fce076b93e19518cdc9693da490;p=thirdparty%2Fcurl.git

cf-socket: fix listen pollset for FTP active mode

Follow-up to a07ba37b5e88a89bf4bcc6b0c927f7a42d7ea4f2 which did not
solve the issue of corrent polling for FTP active data connections.

Added test cases for active up-/download.

Closes #14786
---

diff --git a/lib/cf-socket.c b/lib/cf-socket.c
index 0e5db5e3ec..729e38f889 100644
--- a/lib/cf-socket.c
+++ b/lib/cf-socket.c
@@ -1409,17 +1409,19 @@ static void cf_socket_adjust_pollset(struct Curl_cfilter *cf,
   struct cf_socket_ctx *ctx = cf->ctx;
 
   if(ctx->sock != CURL_SOCKET_BAD) {
-    if(!cf->connected) {
-      if(ctx->listening) {
-        Curl_pollset_set_in_only(data, ps, ctx->sock);
-        CURL_TRC_CF(data, cf, "adjust_pollset, listening, POLLIN fd=%"
-                    FMT_SOCKET_T, ctx->sock);
-      }
-      else {
-        Curl_pollset_set_out_only(data, ps, ctx->sock);
-        CURL_TRC_CF(data, cf, "adjust_pollset, !connected, POLLOUT fd=%"
-                    FMT_SOCKET_T, ctx->sock);
-      }
+    /* A listening socket filter needs to be connected before the accept
+     * for some weird FTP interaction. This should be rewritten, so that
+     * FTP no longer does the socket checks and accept calls and delegates
+     * all that to the filter. TODO. */
+    if(ctx->listening) {
+      Curl_pollset_set_in_only(data, ps, ctx->sock);
+      CURL_TRC_CF(data, cf, "adjust_pollset, listening, POLLIN fd=%"
+                  FMT_SOCKET_T, ctx->sock);
+    }
+    else if(!cf->connected) {
+      Curl_pollset_set_out_only(data, ps, ctx->sock);
+      CURL_TRC_CF(data, cf, "adjust_pollset, !connected, POLLOUT fd=%"
+                  FMT_SOCKET_T, ctx->sock);
     }
     else if(!ctx->active) {
       Curl_pollset_add_in(data, ps, ctx->sock);
diff --git a/tests/http/test_30_vsftpd.py b/tests/http/test_30_vsftpd.py
index 477b27affd..d90f84611c 100644
--- a/tests/http/test_30_vsftpd.py
+++ b/tests/http/test_30_vsftpd.py
@@ -163,6 +163,32 @@ class TestVsFTPD:
         assert r.tcpdump
         assert len(r.tcpdump.stats) == 0, f'Unexpected TCP RSTs packets'
 
+    def test_30_08_active_download(self, env: Env, vsftpd: VsFTPD):
+        docname = 'data-10k'
+        curl = CurlClient(env=env)
+        srcfile = os.path.join(vsftpd.docs_dir, f'{docname}')
+        count = 1
+        url = f'ftp://{env.ftp_domain}:{vsftpd.port}/{docname}?[0-{count-1}]'
+        r = curl.ftp_get(urls=[url], with_stats=True, extra_args=[
+            '--ftp-port', '127.0.0.1'
+        ])
+        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):
+        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'
+        ])
+        r.check_stats(count=count, http_status=226)
+        self.check_upload(env, vsftpd, docname=docname)
+
     def check_downloads(self, client, srcfile: str, count: int,
                         complete: bool = True):
         for i in range(count):
diff --git a/tests/http/test_31_vsftpds.py b/tests/http/test_31_vsftpds.py
index d575d89f5a..95f3957cad 100644
--- a/tests/http/test_31_vsftpds.py
+++ b/tests/http/test_31_vsftpds.py
@@ -194,6 +194,32 @@ class TestVsFTPD:
             f'expected source with {newlines} lines to be that much larger,'\
             f'instead srcsize={srcsize}, upload size={dstsize}, diff={dstsize-srcsize}'
 
+    def test_31_08_active_download(self, env: Env, vsftpds: VsFTPD):
+        docname = 'data-10k'
+        curl = CurlClient(env=env)
+        srcfile = os.path.join(vsftpds.docs_dir, f'{docname}')
+        count = 1
+        url = f'ftp://{env.ftp_domain}:{vsftpds.port}/{docname}?[0-{count-1}]'
+        r = curl.ftp_ssl_get(urls=[url], with_stats=True, extra_args=[
+            '--ftp-port', '127.0.0.1'
+        ])
+        r.check_stats(count=count, http_status=226)
+        self.check_downloads(curl, srcfile, count)
+
+    def test_31_09_active_upload(self, env: Env, vsftpds: VsFTPD):
+        docname = 'upload-1k'
+        curl = CurlClient(env=env)
+        srcfile = os.path.join(env.gen_dir, docname)
+        dstfile = os.path.join(vsftpds.docs_dir, docname)
+        self._rmf(dstfile)
+        count = 1
+        url = f'ftp://{env.ftp_domain}:{vsftpds.port}/'
+        r = curl.ftp_ssl_upload(urls=[url], fupload=f'{srcfile}', with_stats=True, extra_args=[
+            '--ftp-port', '127.0.0.1'
+        ])
+        r.check_stats(count=count, http_status=226)
+        self.check_upload(env, vsftpds, docname=docname)
+
     def check_downloads(self, client, srcfile: str, count: int,
                         complete: bool = True):
         for i in range(count):