From c517951a484c25346e96651c34e88105dc0908ef Mon Sep 17 00:00:00 2001 From: preetwinder Date: Wed, 16 Sep 2015 14:05:05 +0530 Subject: [PATCH 1/4] add_scheme_if_missing for scrapy shell command --- scrapy/commands/shell.py | 3 +++ scrapy/utils/url.py | 8 ++++++++ tests/test_utils_url.py | 18 +++++++++++++++++- 3 files changed, 28 insertions(+), 1 deletion(-) diff --git a/scrapy/commands/shell.py b/scrapy/commands/shell.py index 95af8586b..e94e339de 100644 --- a/scrapy/commands/shell.py +++ b/scrapy/commands/shell.py @@ -9,6 +9,7 @@ from threading import Thread from scrapy.commands import ScrapyCommand from scrapy.shell import Shell from scrapy.http import Request +from scrapy.utils.url import add_scheme_if_missing from scrapy.utils.spider import spidercls_for_request, DefaultSpider @@ -41,6 +42,8 @@ class Command(ScrapyCommand): def run(self, args, opts): url = args[0] if args else None + if url: + url = add_scheme_if_missing(url) spider_loader = self.crawler_process.spider_loader spidercls = DefaultSpider diff --git a/scrapy/utils/url.py b/scrapy/utils/url.py index 99f350361..94ec4de1b 100644 --- a/scrapy/utils/url.py +++ b/scrapy/utils/url.py @@ -110,3 +110,11 @@ def escape_ajax(url): if not frag.startswith('!'): return url return add_or_replace_parameter(defrag, '_escaped_fragment_', frag[1:]) + +def add_scheme_if_missing(url): + parser = parse_url(url) + if not parser.scheme: + if not parser.netloc: + parser = parser._replace(netloc=parser.path, path='') + parser = parser._replace(scheme='http') + return parser.geturl() diff --git a/tests/test_utils_url.py b/tests/test_utils_url.py index 7bf0e5b4a..fae4c988b 100644 --- a/tests/test_utils_url.py +++ b/tests/test_utils_url.py @@ -4,7 +4,7 @@ import unittest import six from scrapy.spiders import Spider from scrapy.utils.url import (url_is_from_any_domain, url_is_from_spider, - canonicalize_url) + canonicalize_url, add_scheme_if_missing) __doctests__ = ['scrapy.utils.url'] @@ -73,6 +73,22 @@ class UrlUtilsTest(unittest.TestCase): self.assertTrue(url_is_from_spider('http://www.example.net/some/page.html', MySpider)) self.assertFalse(url_is_from_spider('http://www.example.us/some/page.html', MySpider)) + def test_add_scheme_if_missing(self): + self.assertEqual(add_scheme_if_missing('http://www.example.com'), + 'http://www.example.com') + self.assertEqual(add_scheme_if_missing('http://www.example.com/some/page.html'), + 'http://www.example.com/some/page.html') + self.assertEqual(add_scheme_if_missing('http://example.com'), + 'http://example.com') + self.assertEqual(add_scheme_if_missing('www.example.com'), + 'http://www.example.com') + self.assertEqual(add_scheme_if_missing('example.com'), + 'http://example.com') + self.assertEqual(add_scheme_if_missing('//example.com'), + 'http://example.com') + self.assertEqual(add_scheme_if_missing('https://www.example.com'), + 'https://www.example.com') + class CanonicalizeUrlTest(unittest.TestCase): From 8c629eee3e41a4d40f620e3a3f594391735b5a9f Mon Sep 17 00:00:00 2001 From: preetwinder Date: Fri, 18 Sep 2015 16:31:37 +0530 Subject: [PATCH 2/4] adds docstring, tests and correction --- scrapy/commands/shell.py | 4 ++-- scrapy/utils/url.py | 13 ++++++------ tests/test_utils_url.py | 44 ++++++++++++++++++++++++++++++++-------- 3 files changed, 44 insertions(+), 17 deletions(-) diff --git a/scrapy/commands/shell.py b/scrapy/commands/shell.py index e94e339de..92ebbe605 100644 --- a/scrapy/commands/shell.py +++ b/scrapy/commands/shell.py @@ -9,7 +9,7 @@ from threading import Thread from scrapy.commands import ScrapyCommand from scrapy.shell import Shell from scrapy.http import Request -from scrapy.utils.url import add_scheme_if_missing +from scrapy.utils.url import add_http_if_no_scheme from scrapy.utils.spider import spidercls_for_request, DefaultSpider @@ -43,7 +43,7 @@ class Command(ScrapyCommand): def run(self, args, opts): url = args[0] if args else None if url: - url = add_scheme_if_missing(url) + url = add_http_if_no_scheme(url) spider_loader = self.crawler_process.spider_loader spidercls = DefaultSpider diff --git a/scrapy/utils/url.py b/scrapy/utils/url.py index 94ec4de1b..c0934ddcf 100644 --- a/scrapy/utils/url.py +++ b/scrapy/utils/url.py @@ -111,10 +111,11 @@ def escape_ajax(url): return url return add_or_replace_parameter(defrag, '_escaped_fragment_', frag[1:]) -def add_scheme_if_missing(url): +def add_http_if_no_scheme(url): + """Adds http as the default scheme if it is missing from the url""" parser = parse_url(url) - if not parser.scheme: - if not parser.netloc: - parser = parser._replace(netloc=parser.path, path='') - parser = parser._replace(scheme='http') - return parser.geturl() + if url.startswith('//'): + url = 'http:' + url + elif not parser.scheme or not parser.netloc: + url = 'http://' + url + return url diff --git a/tests/test_utils_url.py b/tests/test_utils_url.py index fae4c988b..7ccf68c7a 100644 --- a/tests/test_utils_url.py +++ b/tests/test_utils_url.py @@ -4,7 +4,7 @@ import unittest import six from scrapy.spiders import Spider from scrapy.utils.url import (url_is_from_any_domain, url_is_from_spider, - canonicalize_url, add_scheme_if_missing) + canonicalize_url, add_http_if_no_scheme) __doctests__ = ['scrapy.utils.url'] @@ -73,21 +73,47 @@ class UrlUtilsTest(unittest.TestCase): self.assertTrue(url_is_from_spider('http://www.example.net/some/page.html', MySpider)) self.assertFalse(url_is_from_spider('http://www.example.us/some/page.html', MySpider)) - def test_add_scheme_if_missing(self): - self.assertEqual(add_scheme_if_missing('http://www.example.com'), + def test_add_http_if_no_scheme(self): + self.assertEqual(add_http_if_no_scheme('http://www.example.com'), 'http://www.example.com') - self.assertEqual(add_scheme_if_missing('http://www.example.com/some/page.html'), + self.assertEqual(add_http_if_no_scheme('http://www.example.com/some/page.html'), 'http://www.example.com/some/page.html') - self.assertEqual(add_scheme_if_missing('http://example.com'), + self.assertEqual(add_http_if_no_scheme('http://example.com'), 'http://example.com') - self.assertEqual(add_scheme_if_missing('www.example.com'), + self.assertEqual(add_http_if_no_scheme('www.example.com'), 'http://www.example.com') - self.assertEqual(add_scheme_if_missing('example.com'), + self.assertEqual(add_http_if_no_scheme('example.com'), 'http://example.com') - self.assertEqual(add_scheme_if_missing('//example.com'), + self.assertEqual(add_http_if_no_scheme('//example.com'), 'http://example.com') - self.assertEqual(add_scheme_if_missing('https://www.example.com'), + self.assertEqual(add_http_if_no_scheme('//www.example.com/some/page.html'), + 'http://www.example.com/some/page.html') + self.assertEqual(add_http_if_no_scheme('www.example.com:80'), + 'http://www.example.com:80') + self.assertEqual(add_http_if_no_scheme('www.example.com:80/some/page.html'), + 'http://www.example.com:80/some/page.html') + self.assertEqual(add_http_if_no_scheme('http://www.example.com:80/some/page.html'), + 'http://www.example.com:80/some/page.html') + self.assertEqual(add_http_if_no_scheme('www.example.com/some/page#frag'), + 'http://www.example.com/some/page#frag') + self.assertEqual(add_http_if_no_scheme('http://www.example.com/some/page#frag'), + 'http://www.example.com/some/page#frag') + self.assertEqual(add_http_if_no_scheme('www.example.com/do?a=1&b=2&c=3'), + 'http://www.example.com/do?a=1&b=2&c=3') + self.assertEqual(add_http_if_no_scheme('http://www.example.com/do?a=1&b=2&c=3'), + 'http://www.example.com/do?a=1&b=2&c=3') + self.assertEqual(add_http_if_no_scheme('username:password@example.com/some/page.html'), + 'http://username:password@example.com/some/page.html') + self.assertEqual(add_http_if_no_scheme('http://username:password@example.com/some/page.html'), + 'http://username:password@example.com/some/page.html') + self.assertEqual(add_http_if_no_scheme('username:password@example.com:80/some/part?a=1&b=2&c=3#frag'), + 'http://username:password@example.com:80/some/part?a=1&b=2&c=3#frag') + self.assertEqual(add_http_if_no_scheme('http://username:password@example.com:80/some/part?a=1&b=2&c=3#frag'), + 'http://username:password@example.com:80/some/part?a=1&b=2&c=3#frag') + self.assertEqual(add_http_if_no_scheme('https://www.example.com'), 'https://www.example.com') + self.assertEqual(add_http_if_no_scheme('ftp://www.example.com'), + 'ftp://www.example.com') class CanonicalizeUrlTest(unittest.TestCase): From 9d96e767a1baf3b7737440eae0f1d2c5d433f798 Mon Sep 17 00:00:00 2001 From: preetwinder Date: Thu, 24 Sep 2015 17:31:30 +0000 Subject: [PATCH 3/4] Minor changes to add_http_if_no_scheme --- scrapy/utils/url.py | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/scrapy/utils/url.py b/scrapy/utils/url.py index c0934ddcf..398407a64 100644 --- a/scrapy/utils/url.py +++ b/scrapy/utils/url.py @@ -111,11 +111,13 @@ def escape_ajax(url): return url return add_or_replace_parameter(defrag, '_escaped_fragment_', frag[1:]) + def add_http_if_no_scheme(url): - """Adds http as the default scheme if it is missing from the url""" - parser = parse_url(url) + """Add http as the default scheme if it is missing from the url.""" if url.startswith('//'): url = 'http:' + url - elif not parser.scheme or not parser.netloc: + return url + parser = parse_url(url) + if not parser.scheme or not parser.netloc: url = 'http://' + url return url From 47c8e2ba781e351868305f65b989d4b18f54279f Mon Sep 17 00:00:00 2001 From: preetwinder Date: Thu, 24 Sep 2015 17:57:25 +0000 Subject: [PATCH 4/4] Restructure tests for add_http_if_no_scheme function --- tests/test_utils_url.py | 149 +++++++++++++++++++++++++++++----------- 1 file changed, 107 insertions(+), 42 deletions(-) diff --git a/tests/test_utils_url.py b/tests/test_utils_url.py index 7ccf68c7a..314ccd30f 100644 --- a/tests/test_utils_url.py +++ b/tests/test_utils_url.py @@ -73,48 +73,6 @@ class UrlUtilsTest(unittest.TestCase): self.assertTrue(url_is_from_spider('http://www.example.net/some/page.html', MySpider)) self.assertFalse(url_is_from_spider('http://www.example.us/some/page.html', MySpider)) - def test_add_http_if_no_scheme(self): - self.assertEqual(add_http_if_no_scheme('http://www.example.com'), - 'http://www.example.com') - self.assertEqual(add_http_if_no_scheme('http://www.example.com/some/page.html'), - 'http://www.example.com/some/page.html') - self.assertEqual(add_http_if_no_scheme('http://example.com'), - 'http://example.com') - self.assertEqual(add_http_if_no_scheme('www.example.com'), - 'http://www.example.com') - self.assertEqual(add_http_if_no_scheme('example.com'), - 'http://example.com') - self.assertEqual(add_http_if_no_scheme('//example.com'), - 'http://example.com') - self.assertEqual(add_http_if_no_scheme('//www.example.com/some/page.html'), - 'http://www.example.com/some/page.html') - self.assertEqual(add_http_if_no_scheme('www.example.com:80'), - 'http://www.example.com:80') - self.assertEqual(add_http_if_no_scheme('www.example.com:80/some/page.html'), - 'http://www.example.com:80/some/page.html') - self.assertEqual(add_http_if_no_scheme('http://www.example.com:80/some/page.html'), - 'http://www.example.com:80/some/page.html') - self.assertEqual(add_http_if_no_scheme('www.example.com/some/page#frag'), - 'http://www.example.com/some/page#frag') - self.assertEqual(add_http_if_no_scheme('http://www.example.com/some/page#frag'), - 'http://www.example.com/some/page#frag') - self.assertEqual(add_http_if_no_scheme('www.example.com/do?a=1&b=2&c=3'), - 'http://www.example.com/do?a=1&b=2&c=3') - self.assertEqual(add_http_if_no_scheme('http://www.example.com/do?a=1&b=2&c=3'), - 'http://www.example.com/do?a=1&b=2&c=3') - self.assertEqual(add_http_if_no_scheme('username:password@example.com/some/page.html'), - 'http://username:password@example.com/some/page.html') - self.assertEqual(add_http_if_no_scheme('http://username:password@example.com/some/page.html'), - 'http://username:password@example.com/some/page.html') - self.assertEqual(add_http_if_no_scheme('username:password@example.com:80/some/part?a=1&b=2&c=3#frag'), - 'http://username:password@example.com:80/some/part?a=1&b=2&c=3#frag') - self.assertEqual(add_http_if_no_scheme('http://username:password@example.com:80/some/part?a=1&b=2&c=3#frag'), - 'http://username:password@example.com:80/some/part?a=1&b=2&c=3#frag') - self.assertEqual(add_http_if_no_scheme('https://www.example.com'), - 'https://www.example.com') - self.assertEqual(add_http_if_no_scheme('ftp://www.example.com'), - 'ftp://www.example.com') - class CanonicalizeUrlTest(unittest.TestCase): @@ -229,5 +187,112 @@ class CanonicalizeUrlTest(unittest.TestCase): "http://foo.com/AC%2FDC/") +class AddHttpIfNoScheme(unittest.TestCase): + + def test_add_scheme(self): + self.assertEqual(add_http_if_no_scheme('www.example.com'), + 'http://www.example.com') + + def test_without_subdomain(self): + self.assertEqual(add_http_if_no_scheme('example.com'), + 'http://example.com') + + def test_path(self): + self.assertEqual(add_http_if_no_scheme('www.example.com/some/page.html'), + 'http://www.example.com/some/page.html') + + def test_port(self): + self.assertEqual(add_http_if_no_scheme('www.example.com:80'), + 'http://www.example.com:80') + + def test_fragment(self): + self.assertEqual(add_http_if_no_scheme('www.example.com/some/page#frag'), + 'http://www.example.com/some/page#frag') + + def test_query(self): + self.assertEqual(add_http_if_no_scheme('www.example.com/do?a=1&b=2&c=3'), + 'http://www.example.com/do?a=1&b=2&c=3') + + def test_username_password(self): + self.assertEqual(add_http_if_no_scheme('username:password@www.example.com'), + 'http://username:password@www.example.com') + + def test_complete_url(self): + self.assertEqual(add_http_if_no_scheme('username:password@www.example.com:80/some/page/do?a=1&b=2&c=3#frag'), + 'http://username:password@www.example.com:80/some/page/do?a=1&b=2&c=3#frag') + + def test_preserve_http(self): + self.assertEqual(add_http_if_no_scheme('http://www.example.com'), + 'http://www.example.com') + + def test_preserve_http_without_subdomain(self): + self.assertEqual(add_http_if_no_scheme('http://example.com'), + 'http://example.com') + + def test_preserve_http_path(self): + self.assertEqual(add_http_if_no_scheme('http://www.example.com/some/page.html'), + 'http://www.example.com/some/page.html') + + def test_preserve_http_port(self): + self.assertEqual(add_http_if_no_scheme('http://www.example.com:80'), + 'http://www.example.com:80') + + def test_preserve_http_fragment(self): + self.assertEqual(add_http_if_no_scheme('http://www.example.com/some/page#frag'), + 'http://www.example.com/some/page#frag') + + def test_preserve_http_query(self): + self.assertEqual(add_http_if_no_scheme('http://www.example.com/do?a=1&b=2&c=3'), + 'http://www.example.com/do?a=1&b=2&c=3') + + def test_preserve_http_username_password(self): + self.assertEqual(add_http_if_no_scheme('http://username:password@www.example.com'), + 'http://username:password@www.example.com') + + def test_preserve_http_complete_url(self): + self.assertEqual(add_http_if_no_scheme('http://username:password@www.example.com:80/some/page/do?a=1&b=2&c=3#frag'), + 'http://username:password@www.example.com:80/some/page/do?a=1&b=2&c=3#frag') + + def test_protocol_relative(self): + self.assertEqual(add_http_if_no_scheme('//www.example.com'), + 'http://www.example.com') + + def test_protocol_relative_without_subdomain(self): + self.assertEqual(add_http_if_no_scheme('//example.com'), + 'http://example.com') + + def test_protocol_relative_path(self): + self.assertEqual(add_http_if_no_scheme('//www.example.com/some/page.html'), + 'http://www.example.com/some/page.html') + + def test_protocol_relative_port(self): + self.assertEqual(add_http_if_no_scheme('//www.example.com:80'), + 'http://www.example.com:80') + + def test_protocol_relative_fragment(self): + self.assertEqual(add_http_if_no_scheme('//www.example.com/some/page#frag'), + 'http://www.example.com/some/page#frag') + + def test_protocol_relative_query(self): + self.assertEqual(add_http_if_no_scheme('//www.example.com/do?a=1&b=2&c=3'), + 'http://www.example.com/do?a=1&b=2&c=3') + + def test_protocol_relative_username_password(self): + self.assertEqual(add_http_if_no_scheme('//username:password@www.example.com'), + 'http://username:password@www.example.com') + + def test_protocol_relative_complete_url(self): + self.assertEqual(add_http_if_no_scheme('//username:password@www.example.com:80/some/page/do?a=1&b=2&c=3#frag'), + 'http://username:password@www.example.com:80/some/page/do?a=1&b=2&c=3#frag') + + def test_preserve_https(self): + self.assertEqual(add_http_if_no_scheme('https://www.example.com'), + 'https://www.example.com') + + def test_preserve_ftp(self): + self.assertEqual(add_http_if_no_scheme('ftp://www.example.com'), + 'ftp://www.example.com') + + if __name__ == "__main__": unittest.main()