From c6326a0425588cd4180e68c3c1d2b6199d1e3b12 Mon Sep 17 00:00:00 2001 From: Daniel Grana Date: Mon, 23 Feb 2009 19:49:37 +0000 Subject: [PATCH] core: add a custom HTTPClientFactory to reuse url parsing, and remove malformed headers monkeypatches. closes #52 --HG-- extra : convert_revision : svn%3Ab85faa78-f9eb-468e-a121-7cced6da292c%40900 --- .../trunk/scrapy/core/downloader/handlers.py | 2 +- .../trunk/scrapy/core/downloader/webclient.py | 73 ++++++++++++ scrapy/trunk/scrapy/patches/monkeypatches.py | 33 +----- scrapy/trunk/scrapy/tests/test_webclient.py | 112 ++++++++++++++++++ 4 files changed, 187 insertions(+), 33 deletions(-) create mode 100644 scrapy/trunk/scrapy/core/downloader/webclient.py create mode 100644 scrapy/trunk/scrapy/tests/test_webclient.py diff --git a/scrapy/trunk/scrapy/core/downloader/handlers.py b/scrapy/trunk/scrapy/core/downloader/handlers.py index 8b90948f6..0e2274b5e 100644 --- a/scrapy/trunk/scrapy/core/downloader/handlers.py +++ b/scrapy/trunk/scrapy/core/downloader/handlers.py @@ -6,7 +6,6 @@ from __future__ import with_statement import os import urlparse -from twisted.web.client import HTTPClientFactory from twisted.internet import defer, reactor from twisted.web import error as web_error @@ -24,6 +23,7 @@ from scrapy.conf import settings from scrapy.core.downloader.dnscache import DNSCache from scrapy.core.downloader.responsetypes import responsetypes +from scrapy.core.downloader.webclient import ScrapyHTTPClientFactory as HTTPClientFactory default_timeout = settings.getint('DOWNLOAD_TIMEOUT') default_agent = settings.get('USER_AGENT') diff --git a/scrapy/trunk/scrapy/core/downloader/webclient.py b/scrapy/trunk/scrapy/core/downloader/webclient.py new file mode 100644 index 000000000..9d0180af4 --- /dev/null +++ b/scrapy/trunk/scrapy/core/downloader/webclient.py @@ -0,0 +1,73 @@ +from urlparse import urlunparse + +from twisted.web.client import HTTPClientFactory +from twisted.internet import reactor + +from scrapy.http import Url + +def _parse(url, defaultPort=None): + url = url.strip() + try: + parsed = url.parsedurl + except AttributeError: + parsed = Url(url).parsedurl + + scheme = parsed[0] + path = urlunparse(('','')+parsed[2:]) + if defaultPort is None: + if scheme == 'https': + defaultPort = 443 + else: + defaultPort = 80 + host, port = parsed[1], defaultPort + if ':' in host: + host, port = host.split(':') + port = int(port) + if path == "": + path = "/" + return scheme, host, port, path + + +class ScrapyHTTPClientFactory(HTTPClientFactory): + """Scrapy implementation of the HTTPClientFactory overwriting the + serUrl method to make use of our Url object that cache the parse + result. Also we override gotHeaders that dies when parsing malformed + cookies. + """ + + def setURL(self, url): + self.url = url + scheme, host, port, path = _parse(url) + if scheme and host: + self.scheme = scheme + self.host = host + self.port = port + self.path = path + + def gotHeaders(self, headers): + """ + HTTPClientFactory.gotHeaders dies when parsing malformed cookies, + and the crawler is getting malformed cookies from this site. + + Cookies format: + http://www.ietf.org/rfc/rfc2109.txt + + I have choosen not to filter based on this, so we don't filter invalid + values that could be managed correctly by twisted. + """ + self.response_headers = headers + if 'set-cookie' in headers: + goodcookies = [] + for cookie in headers['set-cookie']: + cookparts = cookie.split(';') + cook = cookparts[0].lstrip() + t = cook.split('=', 1) + if len(t) == 2: #Good cookie + goodcookies.append(cookie) + k, v = t + self.cookies[k.lstrip()] = v.lstrip() + if goodcookies: + self.response_headers['set-cookie'] = goodcookies + else: + del self.response_headers['set-cookie'] + diff --git a/scrapy/trunk/scrapy/patches/monkeypatches.py b/scrapy/trunk/scrapy/patches/monkeypatches.py index d18c12864..39a88ff95 100644 --- a/scrapy/trunk/scrapy/patches/monkeypatches.py +++ b/scrapy/trunk/scrapy/patches/monkeypatches.py @@ -12,42 +12,11 @@ from twisted.web.client import HTTPClientFactory #sys.setrecursionlimit(7400) def apply_patches(): - patch_HTTPClientFactory_gotHeaders() - if twisted.__version__ < '8.0.0': patch_HTTPPageGetter_handleResponse() -# XXX: Monkeypatch for twisted.web.client-HTTPClientFactory -# HTTPClientFactory.gotHeaders dies when parsing malformed cookies, -# and the crawler is getting malformed cookies from this site. -_old_gotHeaders = HTTPClientFactory.gotHeaders -# Cookies format: http://www.ietf.org/rfc/rfc2109.txt -# I have choosen not to filter based on this, so we don't filter invalid -# values that could be managed correctly by twisted. -#_COOKIES = re.compile(r'^[^=;]+=[^=;]*(;[^=;]+=[^=;])*$') - -def _is_good_cookie(cookie): - """ Check if a given cookie would make gotHeaders raise an exception """ - cookparts = cookie.split(';') - cook = cookparts[0] - return len(cook.split('=', 1)) == 2 - -def _new_gotHeaders(self, headers): - """ Remove cookies that would make twisted raise an exception """ - if headers.has_key('set-cookie'): - cookies = [cookie for cookie in headers['set-cookie'] - if _is_good_cookie(cookie)] - if cookies: - headers['set-cookie'] = cookies - else: - del headers['set-cookie'] - - return _old_gotHeaders(self, headers) - -def patch_HTTPClientFactory_gotHeaders(): - setattr(HTTPClientFactory, 'gotHeaders', _new_gotHeaders) - +# bugfix not present in twisted 2.5 for handling empty response of HEAD requests def patch_HTTPPageGetter_handleResponse(): from twisted.web.client import PartialDownloadError, HTTPPageGetter from twisted.python import failure diff --git a/scrapy/trunk/scrapy/tests/test_webclient.py b/scrapy/trunk/scrapy/tests/test_webclient.py new file mode 100644 index 000000000..c318ee8f9 --- /dev/null +++ b/scrapy/trunk/scrapy/tests/test_webclient.py @@ -0,0 +1,112 @@ +""" +Tests borrowed from the twisted.web.client tests. +""" +from urlparse import urlparse + +from twisted.trial import unittest +from twisted.web import server, static +from twisted.internet import reactor, defer + +from scrapy.core.downloader.webclient import ScrapyHTTPClientFactory +from scrapy.http import Url + +class ParseUrlTestCase(unittest.TestCase): + """Test URL parsing facility and defaults values.""" + + def _parse(self, url): + f = ScrapyHTTPClientFactory(Url(url)) + return (f.scheme, f.host, f.port, f.path) + + def testParse(self): + scheme, host, port, path = self._parse("http://127.0.0.1/?param=value") + self.assertEquals(path, "/?param=value") + self.assertEquals(port, 80) + scheme, host, port, path = self._parse("http://127.0.0.1/") + self.assertEquals(path, "/") + self.assertEquals(port, 80) + scheme, host, port, path = self._parse("https://127.0.0.1/") + self.assertEquals(path, "/") + self.assertEquals(port, 443) + scheme, host, port, path = self._parse("http://spam:12345/") + self.assertEquals(port, 12345) + scheme, host, port, path = self._parse("http://foo ") + self.assertEquals(host, "foo") + self.assertEquals(path, "/") + scheme, host, port, path = self._parse("http://egg:7890") + self.assertEquals(port, 7890) + self.assertEquals(host, "egg") + self.assertEquals(path, "/") + + def test_externalUnicodeInterference(self): + """ + L{client._parse} should return C{str} for the scheme, host, and path + elements of its return tuple, even when passed an URL which has + previously been passed to L{urlparse} as a C{unicode} string. + """ + badInput = u'http://example.com/path' + goodInput = badInput.encode('ascii') + urlparse(badInput) + scheme, host, port, path = self._parse(goodInput) + self.assertTrue(isinstance(scheme, str)) + self.assertTrue(isinstance(host, str)) + self.assertTrue(isinstance(path, str)) + + +class FakeTransport: + disconnecting = False + + def __init__(self): + self.data = [] + + def write(self, stuff): + self.data.append(stuff) + + +class CookieTestCase(unittest.TestCase): + + def _listen(self, site): + return reactor.listenTCP(0, site, interface="127.0.0.1") + + def setUp(self): + root = static.Data('El toro!', 'text/plain') + site = server.Site(root, timeout=None) + self.port = self._listen(site) + self.portno = self.port.getHost().port + + def tearDown(self): + return self.port.stopListening() + + def testMalformedCookieHeaderParsing(self): + d = defer.Deferred() + factory = ScrapyHTTPClientFactory('http://foo.example.com/') + proto = factory.buildProtocol('127.42.42.42') + proto.transport = FakeTransport() + proto.connectionMade() + for line in [ + '200 Ok', + 'Squash: yes', + 'Hands: stolen', + 'Set-Cookie: CUSTOMER=WILE_E_COYOTE; path=/; expires=Wednesday, 09-Nov-99 23:12:40 GMT', + 'Set-Cookie: PART_NUMBER=ROCKET_LAUNCHER_0001; path=/', + 'Set-Cookie: SHIPPING=FEDEX; path=/foo', + 'Set-Cookie: COUNTRY=UY; path=/foo', + 'Set-Cookie: GOOD_CUSTOMER;', + 'Set-Cookie: NO_A_BOT;', + '', + 'body', + 'more body', + ]: + proto.dataReceived(line + '\r\n') + self.assertEquals(proto.transport.data, + ['GET / HTTP/1.0\r\n', + 'Host: foo.example.com\r\n', + 'User-Agent: Twisted PageGetter\r\n', + '\r\n']) + self.assertEquals(factory.cookies, + { + 'CUSTOMER': 'WILE_E_COYOTE', + 'PART_NUMBER': 'ROCKET_LAUNCHER_0001', + 'SHIPPING': 'FEDEX', + 'COUNTRY': 'UY', + }) +