Close FTP connection and protocol on all paths in FTPDownloadHandler

download_request created an FTPClient per request but never closed the
connection, and the CommandFailed path returned without closing the
ReceivedDataProtocol (leaking a file descriptor / memory buffer). Wrap the
download in try/finally so the protocol is always closed and the connection
is always torn down.

Closes #7602

scrapy/core/downloader/handlers/ftp.py

@@ -108,20 +108,37 @@ class FTPDownloadHandler(BaseDownloadHandler):
         client: FTPClient = await maybe_deferred_to_future(
             creator.connectTCP(parsed_url.hostname, parsed_url.port or 21)
         )
-        filepath = unquote(parsed_url.path)
-        protocol = ReceivedDataProtocol(request.meta.get("ftp_local_filename"))
+        self.client: FTPClient = client
         try:
-            await maybe_deferred_to_future(client.retrieveFile(filepath, protocol))
-        except CommandFailed as e:
-            message = str(e)
-            if m := _CODE_RE.search(message):
-                ftpcode = m.group()
-                httpcode = self.CODE_MAPPING.get(ftpcode, self.CODE_MAPPING["default"])
-                return Response(url=request.url, status=httpcode, body=message.encode())
-            raise
-        protocol.close()
-        headers = {"local filename": protocol.filename or b"", "size": protocol.size}
-        body = protocol.filename or protocol.body.read()
-        respcls = responsetypes.from_args(url=request.url, body=body)
-        # hints for Headers-related types may need to be fixed to not use AnyStr
-        return respcls(url=request.url, status=200, body=body, headers=headers)  # type: ignore[arg-type]
+            filepath = unquote(parsed_url.path)
+            protocol = ReceivedDataProtocol(request.meta.get("ftp_local_filename"))
+            try:
+                await maybe_deferred_to_future(client.retrieveFile(filepath, protocol))
+            except CommandFailed as e:
+                message = str(e)
+                if m := _CODE_RE.search(message):
+                    ftpcode = m.group()
+                    httpcode = self.CODE_MAPPING.get(
+                        ftpcode, self.CODE_MAPPING["default"]
+                    )
+                    return Response(
+                        url=request.url, status=httpcode, body=message.encode()
+                    )
+                raise
+            finally:
+                # Always release the file descriptor / memory buffer held by
+                # the protocol, including on the CommandFailed path above.
+                protocol.close()
+            headers = {
+                "local filename": protocol.filename or b"",
+                "size": protocol.size,
+            }
+            body = protocol.filename or protocol.body.read()
+            respcls = responsetypes.from_args(url=request.url, body=body)
+            # hints for Headers-related types may need to be fixed to not use AnyStr
+            return respcls(url=request.url, status=200, body=body, headers=headers)  # type: ignore[arg-type]
+        finally:
+            # Close the FTP connection instead of leaving it for the garbage
+            # collector to reap at some unspecified later time.
+            if client.transport is not None:
+                client.transport.loseConnection()

tests/test_downloader_handler_twisted_ftp.py

@@ -109,6 +109,29 @@ class TestFTPBase(ABC):
         assert r.status == 404
         assert r.body == b"['550 nonexistent.txt: No such file or directory.']"
 
+    @deferred_f_from_coro_f
+    async def test_ftp_download_closes_connection(
+        self, server_url: str, dh: FTPDownloadHandler
+    ) -> None:
+        # Regression test for #7602: a successful download must close the FTP
+        # connection rather than leaking it until garbage collection.
+        request = Request(url=server_url + "file.txt", meta=self.req_meta)
+        await dh.download_request(request)
+        assert dh.client.transport is not None
+        assert dh.client.transport.disconnecting
+
+    @deferred_f_from_coro_f
+    async def test_ftp_download_closes_connection_on_missing_file(
+        self, server_url: str, dh: FTPDownloadHandler
+    ) -> None:
+        # Regression test for #7602: the connection must be closed even on the
+        # CommandFailed path (e.g. a missing file mapped to a 404 response).
+        request = Request(url=server_url + "nonexistent.txt", meta=self.req_meta)
+        r = await dh.download_request(request)
+        assert r.status == 404
+        assert dh.client.transport is not None
+        assert dh.client.transport.disconnecting
+
     @deferred_f_from_coro_f
     async def test_ftp_local_filename(
         self, server_url: str, dh: FTPDownloadHandler