From 92708108409ccddbacef4fd2e52a08ec65a6184a Mon Sep 17 00:00:00 2001 From: Pablo Hoffman Date: Thu, 9 Jul 2009 18:45:40 -0300 Subject: [PATCH] improved usage of urljoin_rfc function, adding unittests and encoding where needed --- .../contrib/downloadermiddleware/redirect.py | 8 +++---- scrapy/contrib/linkextractors/htmlparser.py | 4 ++-- scrapy/contrib/linkextractors/image.py | 4 ++-- scrapy/contrib/linkextractors/lxmlparser.py | 4 ++-- scrapy/contrib/linkextractors/regex.py | 4 ++-- scrapy/contrib/linkextractors/sgml.py | 4 ++-- scrapy/contrib_exp/adaptors/extraction.py | 10 +++----- scrapy/tests/test_utils_url.py | 20 +++++++++++++++- scrapy/utils/url.py | 24 ++++++++++--------- 9 files changed, 49 insertions(+), 33 deletions(-) diff --git a/scrapy/contrib/downloadermiddleware/redirect.py b/scrapy/contrib/downloadermiddleware/redirect.py index b5af78324..acf109cdc 100644 --- a/scrapy/contrib/downloadermiddleware/redirect.py +++ b/scrapy/contrib/downloadermiddleware/redirect.py @@ -1,5 +1,5 @@ from scrapy import log -from scrapy.utils.url import urljoin_rfc as urljoin +from scrapy.utils.url import urljoin_rfc from scrapy.utils.response import get_meta_refresh from scrapy.conf import settings @@ -16,20 +16,20 @@ class RedirectMiddleware(object): domain = spider.domain_name if response.status in [302, 303] and 'Location' in response.headers: - redirected_url = urljoin(request.url, response.headers['location']) + redirected_url = urljoin_rfc(request.url, response.headers['location']) redirected = request.replace(url=redirected_url, method='GET', body='') redirected.headers.pop('Content-Type', None) redirected.headers.pop('Content-Length', None) return self._redirect(redirected, request, spider, response.status) if response.status in [301, 307] and 'Location' in response.headers: - redirected_url = urljoin(request.url, response.headers['location']) + redirected_url = urljoin_rfc(request.url, response.headers['location']) redirected = request.replace(url=redirected_url) return self._redirect(redirected, request, spider, response.status) interval, url = get_meta_refresh(response) if url and int(interval) < self.max_metarefresh_delay: - redirected = request.replace(url=urljoin(request.url, url)) + redirected = request.replace(url=urljoin_rfc(request.url, url)) return self._redirect(redirected, request, spider, 'meta refresh') return response diff --git a/scrapy/contrib/linkextractors/htmlparser.py b/scrapy/contrib/linkextractors/htmlparser.py index 7026e6481..2714fb562 100644 --- a/scrapy/contrib/linkextractors/htmlparser.py +++ b/scrapy/contrib/linkextractors/htmlparser.py @@ -6,7 +6,7 @@ from HTMLParser import HTMLParser from scrapy.link import Link from scrapy.utils.python import unique as unique_list -from scrapy.utils.url import safe_url_string, urljoin_rfc as urljoin +from scrapy.utils.url import safe_url_string, urljoin_rfc class HtmlParserLinkExtractor(HTMLParser): @@ -28,7 +28,7 @@ class HtmlParserLinkExtractor(HTMLParser): ret = [] base_url = self.base_url if self.base_url else response_url for link in links: - link.url = urljoin(base_url, link.url) + link.url = urljoin_rfc(base_url, link.url, response_encoding) link.url = safe_url_string(link.url, response_encoding) link.text = link.text.decode(response_encoding) ret.append(link) diff --git a/scrapy/contrib/linkextractors/image.py b/scrapy/contrib/linkextractors/image.py index dc6245f32..463989ffd 100644 --- a/scrapy/contrib/linkextractors/image.py +++ b/scrapy/contrib/linkextractors/image.py @@ -6,7 +6,7 @@ image links only. import urlparse from scrapy.link import Link -from scrapy.utils.url import canonicalize_url +from scrapy.utils.url import canonicalize_url, urljoin_rfc from scrapy.utils.python import unicode_to_str, flatten from scrapy.xpath.selector import XPathSelectorList, HtmlXPathSelector @@ -67,7 +67,7 @@ class HTMLImageLinkExtractor(object): seen, ret = set(), [] for link in links: - link.url = urlparse.urljoin(base_url, link.url) + link.url = urljoin_rfc(base_url, link.url, response.encoding) if self.unique: if link.url in seen: continue diff --git a/scrapy/contrib/linkextractors/lxmlparser.py b/scrapy/contrib/linkextractors/lxmlparser.py index d74e5bc19..8d350a349 100644 --- a/scrapy/contrib/linkextractors/lxmlparser.py +++ b/scrapy/contrib/linkextractors/lxmlparser.py @@ -10,7 +10,7 @@ import lxml.html from scrapy.link import Link from scrapy.utils.python import unique as unique_list -from scrapy.utils.url import safe_url_string, urljoin_rfc as urljoin +from scrapy.utils.url import safe_url_string, urljoin_rfc class LxmlLinkExtractor(object): def __init__(self, tag="a", attr="href", process=None, unique=False): @@ -31,7 +31,7 @@ class LxmlLinkExtractor(object): ret = [] base_url = self.base_url if self.base_url else response_url for link in links: - link.url = urljoin(base_url, link.url) + link.url = urljoin_rfc(base_url, link.url, response_encoding) link.url = safe_url_string(link.url, response_encoding) link.text = link.text.decode(response_encoding) ret.append(link) diff --git a/scrapy/contrib/linkextractors/regex.py b/scrapy/contrib/linkextractors/regex.py index ece1a84bf..08e1e4526 100644 --- a/scrapy/contrib/linkextractors/regex.py +++ b/scrapy/contrib/linkextractors/regex.py @@ -1,6 +1,6 @@ import re -from scrapy.utils.url import urljoin_rfc as urljoin +from scrapy.utils.url import urljoin_rfc from scrapy.utils.markup import remove_tags, remove_entities, replace_escape_chars from scrapy.link import Link @@ -19,7 +19,7 @@ class RegexLinkExtractor(SgmlLinkExtractor): def _extract_links(self, response_text, response_url, response_encoding): base_url = self.base_url if self.base_url else response_url - clean_url = lambda u: urljoin(base_url, remove_entities(clean_link(u.decode(response_encoding)))) + clean_url = lambda u: urljoin_rfc(base_url, remove_entities(clean_link(u.decode(response_encoding)))) clean_text = lambda t: replace_escape_chars(remove_tags(t.decode(response_encoding))).strip() links_text = linkre.findall(response_text) diff --git a/scrapy/contrib/linkextractors/sgml.py b/scrapy/contrib/linkextractors/sgml.py index 8740ce776..67632a7af 100644 --- a/scrapy/contrib/linkextractors/sgml.py +++ b/scrapy/contrib/linkextractors/sgml.py @@ -8,7 +8,7 @@ from scrapy.xpath import HtmlXPathSelector from scrapy.link import Link from scrapy.utils.misc import arg_to_iter from scrapy.utils.python import FixedSGMLParser, unique as unique_list, str_to_unicode -from scrapy.utils.url import safe_url_string, urljoin_rfc as urljoin, canonicalize_url, url_is_from_any_domain +from scrapy.utils.url import safe_url_string, urljoin_rfc, canonicalize_url, url_is_from_any_domain class BaseSgmlLinkExtractor(FixedSGMLParser): @@ -30,7 +30,7 @@ class BaseSgmlLinkExtractor(FixedSGMLParser): ret = [] base_url = self.base_url if self.base_url else response_url for link in links: - link.url = urljoin(base_url, link.url) + link.url = urljoin_rfc(base_url, link.url, response_encoding) link.url = safe_url_string(link.url, response_encoding) link.text = str_to_unicode(link.text, response_encoding) ret.append(link) diff --git a/scrapy/contrib_exp/adaptors/extraction.py b/scrapy/contrib_exp/adaptors/extraction.py index 37525c4e0..2d71b70a1 100644 --- a/scrapy/contrib_exp/adaptors/extraction.py +++ b/scrapy/contrib_exp/adaptors/extraction.py @@ -2,13 +2,9 @@ Adaptors related with extraction of data """ -import urlparse -import re -from scrapy import log -from scrapy.http import Response -from scrapy.utils.url import is_url +from scrapy.utils.url import urljoin_rfc from scrapy.utils.response import get_base_url -from scrapy.utils.python import flatten, unicode_to_str +from scrapy.utils.python import unicode_to_str from scrapy.xpath.selector import XPathSelector, XPathSelectorList from scrapy.contrib.linkextractors.image import HTMLImageLinkExtractor @@ -70,7 +66,7 @@ class ExtractImageLinks(object): if raw_links: base_url = get_base_url(self.response) - raw_links = [urlparse.urljoin(base_url, unicode_to_str(rel_url)) for rel_url in raw_links] + raw_links = [urljoin_rfc(base_url, unicode_to_str(rel_url), self.response.encoding) for rel_url in raw_links] lx = HTMLImageLinkExtractor(locations=selectors, canonicalize=self.canonicalize) urls = map(lambda link: link.url, lx.extract_links(self.response)) diff --git a/scrapy/tests/test_utils_url.py b/scrapy/tests/test_utils_url.py index cd4c87ff5..745f748fb 100644 --- a/scrapy/tests/test_utils_url.py +++ b/scrapy/tests/test_utils_url.py @@ -1,5 +1,8 @@ import unittest -from scrapy.utils.url import url_is_from_any_domain, safe_url_string, safe_download_url, url_query_parameter, add_or_replace_parameter, url_query_cleaner, canonicalize_url +from scrapy.utils.url import url_is_from_any_domain, safe_url_string, safe_download_url, \ + url_query_parameter, add_or_replace_parameter, url_query_cleaner, canonicalize_url, \ + urljoin_rfc + class UrlUtilsTest(unittest.TestCase): @@ -16,6 +19,21 @@ class UrlUtilsTest(unittest.TestCase): self.assertFalse(url_is_from_any_domain(url, ['testdomain.com'])) self.assertFalse(url_is_from_any_domain(url+'.testdomain.com', ['testdomain.com'])) + def test_urljoin_rfc(self): + self.assertEqual(urljoin_rfc('http://example.com/some/path', 'newpath/test'), + 'http://example.com/some/newpath/test') + self.assertEqual(urljoin_rfc('http://example.com/some/path/a.jpg', '../key/other'), + 'http://example.com/some/key/other') + u = urljoin_rfc(u'http://example.com/lolo/\xa3/lele', u'lala/\xa3') + self.assertEqual(u, 'http://example.com/lolo/\xc2\xa3/lala/\xc2\xa3') + assert isinstance(u, str) + u = urljoin_rfc(u'http://example.com/lolo/\xa3/lele', 'lala/\xa3', encoding='latin-1') + self.assertEqual(u, 'http://example.com/lolo/\xa3/lala/\xa3') + assert isinstance(u, str) + u = urljoin_rfc('http://example.com/lolo/\xa3/lele', 'lala/\xa3') + self.assertEqual(u, 'http://example.com/lolo/\xa3/lala/\xa3') + assert isinstance(u, str) + def test_safe_url_string(self): # Motoko Kusanagi (Cyborg from Ghost in the Shell) motoko = u'\u8349\u8599 \u7d20\u5b50' diff --git a/scrapy/utils/url.py b/scrapy/utils/url.py index e8265d8ec..91dcf0b14 100644 --- a/scrapy/utils/url.py +++ b/scrapy/utils/url.py @@ -26,18 +26,20 @@ def url_is_from_spider(url, spider): domains.extend(spider.extra_domain_names) return url_is_from_any_domain(url, domains) -def urljoin_rfc(base, ref): - """ - Fixed urlparse.urljoin version that handles - relative query string as RFC states. - """ - if ref.startswith('?'): - fpart = urlparse.urlsplit(str(base))[2].rsplit('/', 1)[-1] - ref = ''.join([fpart, ref]) - # convert ref to a string. This should already - # be the case, however, many spiders do not convert. - return urlparse.urljoin(base, str(ref)) +def urljoin_rfc(base, ref, encoding='utf-8'): + """Same as urlparse.urljoin but supports unicode values in base and ref + parameters (in which case they will be converted to str using the given + encoding). + Always returns a str. + """ + # XXX: this code was commented out because its purpose is unknown and + # there's no test or documentation that specifies its behaviour. please + # don't restore this code without adding unittests for it + #if ref.startswith('?'): + # fpart = urlparse.urlsplit(unicode_to_str(base, encoding))[2].rsplit('/', 1)[-1] + # ref = ''.join([fpart, ref]) + return urlparse.urljoin(unicode_to_str(base, encoding), unicode_to_str(ref, encoding)) _reserved = ';/?:@&=+$|,#' # RFC 2396 (Generic Syntax) _unreserved_marks = "-_.!~*'()" #RFC 2396 sec 2.3