improved usage of urljoin_rfc function, adding unittests and encoding where needed

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

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)

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

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)

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)

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)

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))

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'

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