From 3ffa2c5759d068e298d94be406fc26faffac6034 Mon Sep 17 00:00:00 2001 From: Paul Tremberth Date: Thu, 20 Oct 2016 18:01:50 +0200 Subject: [PATCH 1/7] Support Anonymous FTP Fixes GH-2342 --- scrapy/core/downloader/handlers/ftp.py | 20 ++++++---- scrapy/settings/default_settings.py | 3 ++ tests/test_downloader_handlers.py | 52 +++++++++++++++++++++++--- 3 files changed, 62 insertions(+), 13 deletions(-) diff --git a/scrapy/core/downloader/handlers/ftp.py b/scrapy/core/downloader/handlers/ftp.py index 93fa0a3a7..603c1e2e3 100644 --- a/scrapy/core/downloader/handlers/ftp.py +++ b/scrapy/core/downloader/handlers/ftp.py @@ -30,7 +30,7 @@ In case of status 200 request, response.headers will come with two keys: import re from io import BytesIO -from six.moves.urllib.parse import urlparse, unquote +from six.moves.urllib.parse import unquote from twisted.internet import reactor from twisted.protocols.ftp import FTPClient, CommandFailed @@ -38,6 +38,8 @@ from twisted.internet.protocol import Protocol, ClientCreator from scrapy.http import Response from scrapy.responsetypes import responsetypes +from scrapy.utils.httpobj import urlparse_cached + class ReceivedDataProtocol(Protocol): def __init__(self, filename=None): @@ -64,14 +66,18 @@ class FTPDownloadHandler(object): "default": 503, } - def __init__(self, setting): - pass + def __init__(self, settings): + self.anonymous_user = settings['FTP_ANONYMOUS_USER'] + self.anonymous_password = settings['FTP_ANONYMOUS_PASSWORD'] def download_request(self, request, spider): - parsed_url = urlparse(request.url) - creator = ClientCreator(reactor, FTPClient, request.meta["ftp_user"], - request.meta["ftp_password"], - passive=request.meta.get("ftp_passive", 1)) + parsed_url = urlparse_cached(request) + user = request.meta.get("ftp_user", self.anonymous_user) + password = request.meta.get("ftp_password") + if user == self.anonymous_user and password is None: + password = self.anonymous_password + creator = ClientCreator(reactor, FTPClient, user, password, + passive=request.meta.get("ftp_passive", 1)) return creator.connectTCP(parsed_url.hostname, parsed_url.port or 21).addCallback(self.gotClient, request, unquote(parsed_url.path)) diff --git a/scrapy/settings/default_settings.py b/scrapy/settings/default_settings.py index cb88bc2bf..a21c2584a 100644 --- a/scrapy/settings/default_settings.py +++ b/scrapy/settings/default_settings.py @@ -161,6 +161,9 @@ FEED_EXPORTERS_BASE = { FILES_STORE_S3_ACL = 'private' +FTP_ANONYMOUS_USER = 'anonymous' +FTP_ANONYMOUS_PASSWORD = 'anonymous@example.com' + HTTPCACHE_ENABLED = False HTTPCACHE_DIR = 'httpcache' HTTPCACHE_IGNORE_MISSING = False diff --git a/tests/test_downloader_handlers.py b/tests/test_downloader_handlers.py index 6333efceb..2d97573b3 100644 --- a/tests/test_downloader_handlers.py +++ b/tests/test_downloader_handlers.py @@ -676,10 +676,11 @@ class S3TestCase(unittest.TestCase): b'AWS 0PN5J17HBGZHT7JJ3X82:+CfvG8EZ3YccOrRVMXNaK2eKZmM=') -class FTPTestCase(unittest.TestCase): +class BaseFTPTestCase(unittest.TestCase): username = "scrapy" password = "passwd" + req_meta = {"ftp_user": username, "ftp_password": password} if six.PY3: skip = "Twisted missing ftp support for PY3" @@ -722,7 +723,7 @@ class FTPTestCase(unittest.TestCase): def test_ftp_download_success(self): request = Request(url="ftp://127.0.0.1:%s/file.txt" % self.portNum, - meta={"ftp_user": self.username, "ftp_password": self.password}) + meta=self.req_meta) d = self.download_handler.download_request(request, None) def _test(r): @@ -734,7 +735,7 @@ class FTPTestCase(unittest.TestCase): def test_ftp_download_path_with_spaces(self): request = Request( url="ftp://127.0.0.1:%s/file with spaces.txt" % self.portNum, - meta={"ftp_user": self.username, "ftp_password": self.password} + meta=self.req_meta ) d = self.download_handler.download_request(request, None) @@ -746,7 +747,7 @@ class FTPTestCase(unittest.TestCase): def test_ftp_download_notexist(self): request = Request(url="ftp://127.0.0.1:%s/notexist.txt" % self.portNum, - meta={"ftp_user": self.username, "ftp_password": self.password}) + meta=self.req_meta) d = self.download_handler.download_request(request, None) def _test(r): @@ -755,8 +756,10 @@ class FTPTestCase(unittest.TestCase): def test_ftp_local_filename(self): local_fname = "/tmp/file.txt" + meta = {"ftp_local_filename": local_fname} + meta.update(self.req_meta) request = Request(url="ftp://127.0.0.1:%s/file.txt" % self.portNum, - meta={"ftp_user": self.username, "ftp_password": self.password, "ftp_local_filename": local_fname}) + meta=meta) d = self.download_handler.download_request(request, None) def _test(r): @@ -768,13 +771,50 @@ class FTPTestCase(unittest.TestCase): os.remove(local_fname) return self._add_test_callbacks(d, _test) + +class FTPTestCase(BaseFTPTestCase): + def test_invalid_credentials(self): from twisted.protocols.ftp import ConnectionLost + meta = dict(self.req_meta) + meta.update({"ftp_password": 'invalid'}) request = Request(url="ftp://127.0.0.1:%s/file.txt" % self.portNum, - meta={"ftp_user": self.username, "ftp_password": 'invalid'}) + meta=meta) d = self.download_handler.download_request(request, None) def _test(r): self.assertEqual(r.type, ConnectionLost) return self._add_test_callbacks(d, errback=_test) + + +class AnonymousFTPTestCase(BaseFTPTestCase): + + username = "anonymous" + req_meta = {} + + def setUp(self): + from twisted.protocols.ftp import FTPRealm, FTPFactory + from scrapy.core.downloader.handlers.ftp import FTPDownloadHandler + + # setup dir and test file + self.directory = self.mktemp() + os.mkdir(self.directory) + + fp = FilePath(self.directory) + fp.child('file.txt').setContent("I have the power!") + fp.child('file with spaces.txt').setContent("Moooooooooo power!") + + # setup server for anonymous access + realm = FTPRealm(anonymousRoot=self.directory) + p = portal.Portal(realm) + p.registerChecker(checkers.AllowAnonymousAccess(), + credentials.IAnonymous) + + self.factory = FTPFactory(portal=p, + userAnonymous=self.username) + print("self.factory.allowAnonymous=%r" % self.factory.allowAnonymous) + self.port = reactor.listenTCP(0, self.factory, interface="127.0.0.1") + self.portNum = self.port.getHost().port + self.download_handler = FTPDownloadHandler(Settings()) + self.addCleanup(self.port.stopListening) From 26b7c039a9ba3a975b94472dbd54d6fa7bbc358a Mon Sep 17 00:00:00 2001 From: Paul Tremberth Date: Thu, 20 Oct 2016 18:06:59 +0200 Subject: [PATCH 2/7] Remove debug print statement --- tests/test_downloader_handlers.py | 1 - 1 file changed, 1 deletion(-) diff --git a/tests/test_downloader_handlers.py b/tests/test_downloader_handlers.py index 2d97573b3..519a3052e 100644 --- a/tests/test_downloader_handlers.py +++ b/tests/test_downloader_handlers.py @@ -813,7 +813,6 @@ class AnonymousFTPTestCase(BaseFTPTestCase): self.factory = FTPFactory(portal=p, userAnonymous=self.username) - print("self.factory.allowAnonymous=%r" % self.factory.allowAnonymous) self.port = reactor.listenTCP(0, self.factory, interface="127.0.0.1") self.portNum = self.port.getHost().port self.download_handler = FTPDownloadHandler(Settings()) From 565baae1e2b5f4b805d8f5354d164d61423f59ad Mon Sep 17 00:00:00 2001 From: Paul Tremberth Date: Mon, 24 Oct 2016 15:07:55 +0200 Subject: [PATCH 3/7] Change FTP_* settings names --- scrapy/core/downloader/handlers/ftp.py | 15 ++++++++------- scrapy/settings/default_settings.py | 5 +++-- 2 files changed, 11 insertions(+), 9 deletions(-) diff --git a/scrapy/core/downloader/handlers/ftp.py b/scrapy/core/downloader/handlers/ftp.py index 603c1e2e3..1398140b4 100644 --- a/scrapy/core/downloader/handlers/ftp.py +++ b/scrapy/core/downloader/handlers/ftp.py @@ -67,17 +67,18 @@ class FTPDownloadHandler(object): } def __init__(self, settings): - self.anonymous_user = settings['FTP_ANONYMOUS_USER'] - self.anonymous_password = settings['FTP_ANONYMOUS_PASSWORD'] + self.default_user = settings['FTP_USER'] + self.default_password = settings['FTP_PASSWORD'] + self.passive_mode = settings['FTP_PASSIVE_MODE'] def download_request(self, request, spider): parsed_url = urlparse_cached(request) - user = request.meta.get("ftp_user", self.anonymous_user) - password = request.meta.get("ftp_password") - if user == self.anonymous_user and password is None: - password = self.anonymous_password + user = request.meta.get("ftp_user", self.default_user) + password = request.meta.get("ftp_password", self.default_password) + passive_mode = 1 if bool(request.meta.get("ftp_passive", + self.passive_mode)) else 0 creator = ClientCreator(reactor, FTPClient, user, password, - passive=request.meta.get("ftp_passive", 1)) + passive=passive_mode) return creator.connectTCP(parsed_url.hostname, parsed_url.port or 21).addCallback(self.gotClient, request, unquote(parsed_url.path)) diff --git a/scrapy/settings/default_settings.py b/scrapy/settings/default_settings.py index a21c2584a..9ecefbbb5 100644 --- a/scrapy/settings/default_settings.py +++ b/scrapy/settings/default_settings.py @@ -161,8 +161,9 @@ FEED_EXPORTERS_BASE = { FILES_STORE_S3_ACL = 'private' -FTP_ANONYMOUS_USER = 'anonymous' -FTP_ANONYMOUS_PASSWORD = 'anonymous@example.com' +FTP_USER = 'anonymous' +FTP_PASSWORD = 'anonymous@example.com' +FTP_PASSIVE_MODE = True HTTPCACHE_ENABLED = False HTTPCACHE_DIR = 'httpcache' From b80e1bb6c5605406cfff45ae8058cd0f142ef3f4 Mon Sep 17 00:00:00 2001 From: Paul Tremberth Date: Mon, 24 Oct 2016 15:08:24 +0200 Subject: [PATCH 4/7] Document new FTP_* settings --- docs/topics/settings.rst | 28 ++++++++++++++++++++++++++++ 1 file changed, 28 insertions(+) diff --git a/docs/topics/settings.rst b/docs/topics/settings.rst index 0515a9e0d..78fbc79fe 100644 --- a/docs/topics/settings.rst +++ b/docs/topics/settings.rst @@ -686,6 +686,34 @@ The Feed Temp dir allows you to set a custom folder to save crawler temporary files before uploading with :ref:`FTP feed storage ` and :ref:`Amazon S3 `. +.. setting:: FTP_PASSIVE_MODE + +FTP_PASSIVE_MODE +---------------- + +Default: ``True`` + +Whether or not to use passive mode when initiating FTP transfers. + +.. setting:: FTP_PASSWORD + +FTP_PASSWORD +------------ + +Default: ``"anonymous@example.com"`` + +The password to use for FTP connections when there is no ``"ftp_password"`` +in ``Request`` meta. + +.. setting:: FTP_USER + +FTP_USER +-------- + +Default: ``"anonymous"`` + +The username to use for FTP connections when there is no ``"ftp_user"`` +in ``Request`` meta. .. setting:: ITEM_PIPELINES From d35a01a1036c94b13a3828b34bd1752000b5c0fc Mon Sep 17 00:00:00 2001 From: Paul Tremberth Date: Mon, 20 Feb 2017 14:23:23 +0100 Subject: [PATCH 5/7] Update default password --- docs/topics/settings.rst | 2 +- scrapy/settings/default_settings.py | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/docs/topics/settings.rst b/docs/topics/settings.rst index 78fbc79fe..72e7a881f 100644 --- a/docs/topics/settings.rst +++ b/docs/topics/settings.rst @@ -700,7 +700,7 @@ Whether or not to use passive mode when initiating FTP transfers. FTP_PASSWORD ------------ -Default: ``"anonymous@example.com"`` +Default: ``"guest"`` The password to use for FTP connections when there is no ``"ftp_password"`` in ``Request`` meta. diff --git a/scrapy/settings/default_settings.py b/scrapy/settings/default_settings.py index 9ecefbbb5..6ae0fc288 100644 --- a/scrapy/settings/default_settings.py +++ b/scrapy/settings/default_settings.py @@ -162,7 +162,7 @@ FEED_EXPORTERS_BASE = { FILES_STORE_S3_ACL = 'private' FTP_USER = 'anonymous' -FTP_PASSWORD = 'anonymous@example.com' +FTP_PASSWORD = 'guest' FTP_PASSIVE_MODE = True HTTPCACHE_ENABLED = False From f2e200120bc20b6d4792ade2b295395296340ee0 Mon Sep 17 00:00:00 2001 From: Paul Tremberth Date: Mon, 20 Feb 2017 14:50:05 +0100 Subject: [PATCH 6/7] Explicitly remove test directories in FTP tests --- tests/test_downloader_handlers.py | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/tests/test_downloader_handlers.py b/tests/test_downloader_handlers.py index 519a3052e..459c1a179 100644 --- a/tests/test_downloader_handlers.py +++ b/tests/test_downloader_handlers.py @@ -5,6 +5,7 @@ try: from unittest import mock except ImportError: import mock +import shutil from twisted.trial import unittest from twisted.protocols.policies import WrappingFactory @@ -710,6 +711,9 @@ class BaseFTPTestCase(unittest.TestCase): self.download_handler = FTPDownloadHandler(Settings()) self.addCleanup(self.port.stopListening) + def tearDown(self): + shutil.rmtree(self.directory) + def _add_test_callbacks(self, deferred, callback=None, errback=None): def _clean(data): self.download_handler.client.transport.loseConnection() @@ -817,3 +821,6 @@ class AnonymousFTPTestCase(BaseFTPTestCase): self.portNum = self.port.getHost().port self.download_handler = FTPDownloadHandler(Settings()) self.addCleanup(self.port.stopListening) + + def tearDown(self): + shutil.rmtree(self.directory) From f3a7567443d9e52add6224291e98999740d4c33d Mon Sep 17 00:00:00 2001 From: Paul Tremberth Date: Mon, 20 Feb 2017 17:15:05 +0100 Subject: [PATCH 7/7] Add note on FTP_PASSWORD default value --- docs/topics/settings.rst | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/docs/topics/settings.rst b/docs/topics/settings.rst index 72e7a881f..522843fee 100644 --- a/docs/topics/settings.rst +++ b/docs/topics/settings.rst @@ -705,6 +705,14 @@ Default: ``"guest"`` The password to use for FTP connections when there is no ``"ftp_password"`` in ``Request`` meta. +.. note:: + Paraphrasing `RFC 1635`_, although it is common to use either the password + "guest" or one's e-mail address for anonymous FTP, + some FTP servers explicitly ask for the user's e-mail address + and will not allow login with the "guest" password. + +.. _RFC 1635: https://tools.ietf.org/html/rfc1635 + .. setting:: FTP_USER FTP_USER