From 98a2e77a75f0b203ce9841801e342a11f15ad419 Mon Sep 17 00:00:00 2001 From: Leonid Amirov Date: Mon, 2 Nov 2015 15:30:49 +0300 Subject: [PATCH 01/20] issue GH #1550 - fixed error: shell command wasn't accepting files URIs --- scrapy/commands/shell.py | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/scrapy/commands/shell.py b/scrapy/commands/shell.py index 92ebbe605..f10da4370 100644 --- a/scrapy/commands/shell.py +++ b/scrapy/commands/shell.py @@ -5,11 +5,11 @@ See documentation in docs/topics/shell.rst """ from threading import Thread +from w3lib.url import any_to_uri from scrapy.commands import ScrapyCommand from scrapy.shell import Shell from scrapy.http import Request -from scrapy.utils.url import add_http_if_no_scheme from scrapy.utils.spider import spidercls_for_request, DefaultSpider @@ -43,7 +43,8 @@ class Command(ScrapyCommand): def run(self, args, opts): url = args[0] if args else None if url: - url = add_http_if_no_scheme(url) + url = any_to_uri(url) + spider_loader = self.crawler_process.spider_loader spidercls = DefaultSpider From a41c64bfb97a7bba8ec138345d48b94d8734d27e Mon Sep 17 00:00:00 2001 From: Leonid Amirov Date: Mon, 2 Nov 2015 16:06:21 +0300 Subject: [PATCH 02/20] issue GH #1550 - fixed bugs in scrapy.utils.url.add_http_if_no_scheme(): when given URI where scheme is present, but not 'http' the function gave bad result --- scrapy/utils/url.py | 21 ++++++++++++++------- 1 file changed, 14 insertions(+), 7 deletions(-) diff --git a/scrapy/utils/url.py b/scrapy/utils/url.py index 398407a64..3eac5fb3d 100644 --- a/scrapy/utils/url.py +++ b/scrapy/utils/url.py @@ -6,6 +6,7 @@ Some of the functions that used to be imported from this module have been moved to the w3lib.url module. Always import those from there instead. """ import posixpath +from urlparse import urlsplit, urlunsplit from six.moves.urllib.parse import (ParseResult, urlunparse, urldefrag, urlparse, parse_qsl, urlencode, unquote) @@ -114,10 +115,16 @@ def escape_ajax(url): def add_http_if_no_scheme(url): """Add http as the default scheme if it is missing from the url.""" - if url.startswith('//'): - url = 'http:' + url - return url - parser = parse_url(url) - if not parser.scheme or not parser.netloc: - url = 'http://' + url - return url + parts = urlsplit(url) + scheme = parts.scheme or "http" + if parts.netloc: + netloc = parts.netloc + path = parts.path + else: + path_parts = url.split("/", 1) + netloc = path_parts[0] + path = path_parts[1] if len(path_parts) > 1 else "/" + + return urlunsplit(( + scheme, netloc, path, parts.query, parts.fragment + )) From bc9db65358bcae3fc228d4b8099d319cba479ff2 Mon Sep 17 00:00:00 2001 From: Leonid Amirov Date: Mon, 2 Nov 2015 16:08:19 +0300 Subject: [PATCH 03/20] issue GH #1550 - scrapy shell argument fixes: "example.com" requests "http://example.com"; "example" requests "file://example"; "./example.com" requests "file://example.com" --- scrapy/commands/shell.py | 13 ++++++++++++- 1 file changed, 12 insertions(+), 1 deletion(-) diff --git a/scrapy/commands/shell.py b/scrapy/commands/shell.py index f10da4370..cb441bc9d 100644 --- a/scrapy/commands/shell.py +++ b/scrapy/commands/shell.py @@ -5,11 +5,13 @@ See documentation in docs/topics/shell.rst """ from threading import Thread +import urlparse from w3lib.url import any_to_uri from scrapy.commands import ScrapyCommand from scrapy.shell import Shell from scrapy.http import Request +from scrapy.utils.url import add_http_if_no_scheme from scrapy.utils.spider import spidercls_for_request, DefaultSpider @@ -43,7 +45,16 @@ class Command(ScrapyCommand): def run(self, args, opts): url = args[0] if args else None if url: - url = any_to_uri(url) + parts = urlparse.urlsplit(url) + if not parts.scheme: + if "." not in parts.path.split("/", 1)[0]: + url = any_to_uri(url) + + for pattern in ["/", "./", "../"]: + if url.startswith(pattern): + url = any_to_uri(url) + break + url = add_http_if_no_scheme(url) spider_loader = self.crawler_process.spider_loader From dd45b31fe42635ff43df62c667f1ace5f9169737 Mon Sep 17 00:00:00 2001 From: Leonid Amirov Date: Tue, 3 Nov 2015 14:32:30 +0300 Subject: [PATCH 04/20] issue GH #1550 - rewritten add_http_if_no_scheme() --- scrapy/utils/url.py | 20 +++++++------------- 1 file changed, 7 insertions(+), 13 deletions(-) diff --git a/scrapy/utils/url.py b/scrapy/utils/url.py index 3eac5fb3d..0e36003ad 100644 --- a/scrapy/utils/url.py +++ b/scrapy/utils/url.py @@ -6,7 +6,7 @@ Some of the functions that used to be imported from this module have been moved to the w3lib.url module. Always import those from there instead. """ import posixpath -from urlparse import urlsplit, urlunsplit +import re from six.moves.urllib.parse import (ParseResult, urlunparse, urldefrag, urlparse, parse_qsl, urlencode, unquote) @@ -115,16 +115,10 @@ def escape_ajax(url): def add_http_if_no_scheme(url): """Add http as the default scheme if it is missing from the url.""" - parts = urlsplit(url) - scheme = parts.scheme or "http" - if parts.netloc: - netloc = parts.netloc - path = parts.path - else: - path_parts = url.split("/", 1) - netloc = path_parts[0] - path = path_parts[1] if len(path_parts) > 1 else "/" + match = re.match(r"^\w+://", url, flags=re.I) + parts = urlparse(url) + if not match: + scheme = "http:" if parts.netloc else "http://" + url = scheme + url - return urlunsplit(( - scheme, netloc, path, parts.query, parts.fragment - )) + return url From 97b51ea33b6b96241ef87861619e6820ee9e765c Mon Sep 17 00:00:00 2001 From: Leonid Amirov Date: Tue, 3 Nov 2015 14:57:37 +0300 Subject: [PATCH 05/20] issue GH #1550 - six library is used instead of urlparse for python3 compatibility --- scrapy/commands/shell.py | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/scrapy/commands/shell.py b/scrapy/commands/shell.py index cb441bc9d..6f58a2300 100644 --- a/scrapy/commands/shell.py +++ b/scrapy/commands/shell.py @@ -4,8 +4,9 @@ Scrapy Shell See documentation in docs/topics/shell.rst """ +import re +from six.moves.urllib.parse import urlparse, urlunparse from threading import Thread -import urlparse from w3lib.url import any_to_uri from scrapy.commands import ScrapyCommand @@ -45,7 +46,7 @@ class Command(ScrapyCommand): def run(self, args, opts): url = args[0] if args else None if url: - parts = urlparse.urlsplit(url) + parts = urlparse(url) if not parts.scheme: if "." not in parts.path.split("/", 1)[0]: url = any_to_uri(url) From 240ecbf32378a8be9d87c98bec348301fdd05dc4 Mon Sep 17 00:00:00 2001 From: Paul Tremberth Date: Thu, 21 Jan 2016 22:59:48 +0100 Subject: [PATCH 06/20] Add local file tests for scrapy shell command Continuation of #1579 --- tests/test_command_shell.py | 41 +++++++++++++++++++++++++++++++++++++ 1 file changed, 41 insertions(+) diff --git a/tests/test_command_shell.py b/tests/test_command_shell.py index 105202754..7ae685c64 100644 --- a/tests/test_command_shell.py +++ b/tests/test_command_shell.py @@ -1,9 +1,14 @@ +from os.path import join, abspath, dirname, relpath, commonprefix +import os + from twisted.trial import unittest from twisted.internet import defer from scrapy.utils.testsite import SiteTest from scrapy.utils.testproc import ProcessTest +from tests import tests_datadir + class ShellTest(ProcessTest, SiteTest, unittest.TestCase): @@ -51,3 +56,39 @@ class ShellTest(ProcessTest, SiteTest, unittest.TestCase): code = "fetch('{0}') or fetch(response.request.replace(method='POST'))" errcode, out, _ = yield self.execute(['-c', code.format(url)]) self.assertEqual(errcode, 0, out) + + @defer.inlineCallbacks + def test_local_files(self): + test_file_path = join(tests_datadir, 'test_site/index.html') + valid_paths = [ + test_file_path, + relpath(test_file_path), + 'file://'+test_file_path, + './tests/sample_data/test_site/index.html', + 'tests/sample_data/test_site/index.html', + ] + for filepath in valid_paths: + _, out, _ = yield self.execute([filepath, '-c', 'item']) + assert b'{}' in out + + @defer.inlineCallbacks + def test_local_files_invalid(self): + invalid_filepaths = [ + '../nothinghere.html', + './tests/sample_data/test_site/nothinghere.html' + ] + for filepath in invalid_filepaths: + errcode, out, err = yield self.execute([filepath, '-c', 'item'], + check_code=False) + self.assertEqual(errcode, 1, out or err) + self.assertIn(b'No such file or directory', err) + + # currently, this will try to find a host... + invalid_paths = [ + 'nothinghere.html', + ] + for filepath in invalid_paths: + errcode, out, err = yield self.execute([filepath, '-c', 'item'], + check_code=False) + self.assertEqual(errcode, 1, out or err) + self.assertIn(b'DNS lookup failed', err) From 8bd5b60889bef67cb32879562d3cc0e751431ed2 Mon Sep 17 00:00:00 2001 From: Paul Tremberth Date: Thu, 21 Jan 2016 23:23:50 +0100 Subject: [PATCH 07/20] Remove relpath filepath --- tests/test_command_shell.py | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/tests/test_command_shell.py b/tests/test_command_shell.py index 7ae685c64..dd201cff3 100644 --- a/tests/test_command_shell.py +++ b/tests/test_command_shell.py @@ -1,5 +1,4 @@ -from os.path import join, abspath, dirname, relpath, commonprefix -import os +from os.path import join from twisted.trial import unittest from twisted.internet import defer @@ -62,7 +61,7 @@ class ShellTest(ProcessTest, SiteTest, unittest.TestCase): test_file_path = join(tests_datadir, 'test_site/index.html') valid_paths = [ test_file_path, - relpath(test_file_path), + # relpath(test_file_path), 'file://'+test_file_path, './tests/sample_data/test_site/index.html', 'tests/sample_data/test_site/index.html', From 6d73e057b500f5063df63abd856ccd77cbafa1f8 Mon Sep 17 00:00:00 2001 From: Paul Tremberth Date: Fri, 22 Jan 2016 13:07:42 +0100 Subject: [PATCH 08/20] Extract guess_scheme function and refactor tests --- scrapy/commands/shell.py | 30 ++++++---- tests/test_command_shell.py | 113 ++++++++++++++++++++++++++---------- 2 files changed, 102 insertions(+), 41 deletions(-) diff --git a/scrapy/commands/shell.py b/scrapy/commands/shell.py index 14dd0a41a..c975387be 100644 --- a/scrapy/commands/shell.py +++ b/scrapy/commands/shell.py @@ -16,6 +16,24 @@ from scrapy.utils.url import add_http_if_no_scheme from scrapy.utils.spider import spidercls_for_request, DefaultSpider +def guess_scheme(url): + """Given an URL as string, + returns a FileURI if it looks like a file path, + otherwise returns an HTTP URL + """ + parts = urlparse(url) + if not parts.scheme: + if "." not in parts.path.split("/", 1)[0]: + url = any_to_uri(url) + + for pattern in ["/", "./", "../"]: + if url.startswith(pattern): + url = any_to_uri(url) + break + url = add_http_if_no_scheme(url) + return url + + class Command(ScrapyCommand): requires_project = False @@ -50,16 +68,8 @@ class Command(ScrapyCommand): def run(self, args, opts): url = args[0] if args else None if url: - parts = urlparse(url) - if not parts.scheme: - if "." not in parts.path.split("/", 1)[0]: - url = any_to_uri(url) - - for pattern in ["/", "./", "../"]: - if url.startswith(pattern): - url = any_to_uri(url) - break - url = add_http_if_no_scheme(url) + # first argument may be a local file + url = guess_scheme(url) spider_loader = self.crawler_process.spider_loader diff --git a/tests/test_command_shell.py b/tests/test_command_shell.py index dd201cff3..37c6b8c90 100644 --- a/tests/test_command_shell.py +++ b/tests/test_command_shell.py @@ -3,12 +3,77 @@ from os.path import join from twisted.trial import unittest from twisted.internet import defer +from scrapy.commands.shell import guess_scheme from scrapy.utils.testsite import SiteTest from scrapy.utils.testproc import ProcessTest from tests import tests_datadir +class ShellURLTest(unittest.TestCase): + + def test_file_uri_relative001(self): + # FIXME: 'index.html' is interpreted as a domain name + # is this correct? + url = guess_scheme('index.html') + assert url.startswith('http://') + + def test_file_uri_relative002(self): + url = guess_scheme('./index.html') + assert url.startswith('file://') + + def test_file_uri_relative003(self): + url = guess_scheme('../data/index.html') + assert url.startswith('file://') + + def test_file_uri_relative004(self): + url = guess_scheme('subdir/index.html') + assert url.startswith('file://') + + def test_file_uri_absolute001(self): + """Absolute file paths get prepended with "file://" scheme""" + iurl = '/home/user/www/index.html' + url = guess_scheme(iurl) + self.assertEquals(url, 'file://'+iurl) + + def test_file_uri_scheme(self): + """Output File URI does not change if "file://" scheme is set""" + iurl = 'file:///home/user/www/index.html' + url = guess_scheme(iurl) + self.assertEquals(url, iurl) + + def test_file_uri_windows(self): + raise unittest.SkipTest("Windows filepath are not supported for scrapy shell") + url = guess_scheme('C:\absolute\path\to\a\file.html') + assert url.startswith('file://') + + def test_http_url_001(self): + url = guess_scheme('index.html') + assert url.startswith('http://') + + def test_http_url_002(self): + url = guess_scheme('example.com') + assert url.startswith('http://') + + def test_http_url_003(self): + url = guess_scheme('www.example.com') + assert url.startswith('http://') + + def test_http_url_004(self): + url = guess_scheme('www.example.com/index') + assert url.startswith('http://') + + def test_http_url_005(self): + url = guess_scheme('www.example.com/index.html') + assert url.startswith('http://') + + def test_http_url_scheme(self): + """An full HTTP URL is unaltered""" + iurl = 'http://www.example.com/index.html' + url = guess_scheme(iurl) + self.assertEquals(url, iurl) + + class ShellTest(ProcessTest, SiteTest, unittest.TestCase): command = 'shell' @@ -57,37 +122,23 @@ class ShellTest(ProcessTest, SiteTest, unittest.TestCase): self.assertEqual(errcode, 0, out) @defer.inlineCallbacks - def test_local_files(self): - test_file_path = join(tests_datadir, 'test_site/index.html') - valid_paths = [ - test_file_path, - # relpath(test_file_path), - 'file://'+test_file_path, - './tests/sample_data/test_site/index.html', - 'tests/sample_data/test_site/index.html', - ] - for filepath in valid_paths: - _, out, _ = yield self.execute([filepath, '-c', 'item']) - assert b'{}' in out + def test_local_file(self): + filepath = join(tests_datadir, 'test_site/index.html') + _, out, _ = yield self.execute([filepath, '-c', 'item']) + assert b'{}' in out @defer.inlineCallbacks - def test_local_files_invalid(self): - invalid_filepaths = [ - '../nothinghere.html', - './tests/sample_data/test_site/nothinghere.html' - ] - for filepath in invalid_filepaths: - errcode, out, err = yield self.execute([filepath, '-c', 'item'], - check_code=False) - self.assertEqual(errcode, 1, out or err) - self.assertIn(b'No such file or directory', err) + def test_local_nofile(self): + filepath = 'file:///tests/sample_data/test_site/nothinghere.html' + errcode, out, err = yield self.execute([filepath, '-c', 'item'], + check_code=False) + self.assertEqual(errcode, 1, out or err) + self.assertIn(b'No such file or directory', err) - # currently, this will try to find a host... - invalid_paths = [ - 'nothinghere.html', - ] - for filepath in invalid_paths: - errcode, out, err = yield self.execute([filepath, '-c', 'item'], - check_code=False) - self.assertEqual(errcode, 1, out or err) - self.assertIn(b'DNS lookup failed', err) + @defer.inlineCallbacks + def test_dns_failures(self): + url = 'www.somedomainthatdoesntexi.st' + errcode, out, err = yield self.execute([url, '-c', 'item'], + check_code=False) + self.assertEqual(errcode, 1, out or err) + self.assertIn(b'DNS lookup failed', err) From be239f339c4d4d43ff4f7bd5d3248bb4abd32834 Mon Sep 17 00:00:00 2001 From: Paul Tremberth Date: Fri, 22 Jan 2016 13:13:46 +0100 Subject: [PATCH 09/20] Remove unused import --- scrapy/commands/shell.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/scrapy/commands/shell.py b/scrapy/commands/shell.py index c975387be..1e8427753 100644 --- a/scrapy/commands/shell.py +++ b/scrapy/commands/shell.py @@ -5,7 +5,7 @@ See documentation in docs/topics/shell.rst """ import re -from six.moves.urllib.parse import urlparse, urlunparse +from six.moves.urllib.parse import urlparse from threading import Thread from w3lib.url import any_to_uri From 60052b3c68267eca6c0ed2a178378a2a777b3898 Mon Sep 17 00:00:00 2001 From: Paul Tremberth Date: Fri, 22 Jan 2016 13:18:08 +0100 Subject: [PATCH 10/20] Remove unused re import --- scrapy/commands/shell.py | 1 - 1 file changed, 1 deletion(-) diff --git a/scrapy/commands/shell.py b/scrapy/commands/shell.py index 1e8427753..25ceb5f99 100644 --- a/scrapy/commands/shell.py +++ b/scrapy/commands/shell.py @@ -4,7 +4,6 @@ Scrapy Shell See documentation in docs/topics/shell.rst """ -import re from six.moves.urllib.parse import urlparse from threading import Thread from w3lib.url import any_to_uri From 7a51d370f3a59bcfe118f3ba079c5ea6eda5af75 Mon Sep 17 00:00:00 2001 From: Paul Tremberth Date: Fri, 22 Jan 2016 17:16:27 +0100 Subject: [PATCH 11/20] Regex-based guess_scheme() + refactor tests --- scrapy/commands/shell.py | 28 ++++++---- tests/test_command_shell.py | 102 +++++++++++++++++------------------- 2 files changed, 65 insertions(+), 65 deletions(-) diff --git a/scrapy/commands/shell.py b/scrapy/commands/shell.py index 25ceb5f99..5201feb42 100644 --- a/scrapy/commands/shell.py +++ b/scrapy/commands/shell.py @@ -3,7 +3,7 @@ Scrapy Shell See documentation in docs/topics/shell.rst """ - +import re from six.moves.urllib.parse import urlparse from threading import Thread from w3lib.url import any_to_uri @@ -21,16 +21,22 @@ def guess_scheme(url): otherwise returns an HTTP URL """ parts = urlparse(url) - if not parts.scheme: - if "." not in parts.path.split("/", 1)[0]: - url = any_to_uri(url) - - for pattern in ["/", "./", "../"]: - if url.startswith(pattern): - url = any_to_uri(url) - break - url = add_http_if_no_scheme(url) - return url + if parts.scheme: + return url + # Note: this does not match Windows filepath + if re.match(r'''^ # start with... + ( + \. # ...a single dot, + ( + \. | [^/\.]+ # optionally followed by + )? # either a second dot or some characters + )? # optional match of ".", ".." or ".blabla" + / # at least one "/" for a file path, + . # and something after the "/" + ''', parts.path, flags=re.VERBOSE): + return any_to_uri(url) + else: + return add_http_if_no_scheme(url) class Command(ScrapyCommand): diff --git a/tests/test_command_shell.py b/tests/test_command_shell.py index 37c6b8c90..a61d520fa 100644 --- a/tests/test_command_shell.py +++ b/tests/test_command_shell.py @@ -11,67 +11,61 @@ from tests import tests_datadir class ShellURLTest(unittest.TestCase): + pass - def test_file_uri_relative001(self): - # FIXME: 'index.html' is interpreted as a domain name - # is this correct? - url = guess_scheme('index.html') - assert url.startswith('http://') +def create_guess_scheme_t(args): + def do_expected(self): + url = guess_scheme(args[0]) + assert url.startswith(args[1]), \ + 'Wrong scheme guessed: for `%s` got `%s`, expected `%s...`' % ( + args[0], url, args[1]) + return do_expected - def test_file_uri_relative002(self): - url = guess_scheme('./index.html') - assert url.startswith('file://') +def create_skipped_scheme_t(args): + def do_expected(self): + raise unittest.SkipTest(args[2]) + url = guess_scheme(args[0]) + assert url.startswith(args[1]) + return do_expected - def test_file_uri_relative003(self): - url = guess_scheme('../data/index.html') - assert url.startswith('file://') +for k, args in enumerate ([ + ('/index', 'file://'), + ('/index.html', 'file://'), + ('./index.html', 'file://'), + ('../index.html', 'file://'), + ('../../index.html', 'file://'), + ('./data/index.html', 'file://'), + ('.hidden/data/index.html', 'file://'), + ('/home/user/www/index.html', 'file://'), + ('//home/user/www/index.html', 'file://'), + ('file:///home/user/www/index.html', 'file://'), - def test_file_uri_relative004(self): - url = guess_scheme('subdir/index.html') - assert url.startswith('file://') + ('index.html', 'http://'), + ('example.com', 'http://'), + ('www.example.com', 'http://'), + ('www.example.com/index.html', 'http://'), + ('http://example.com', 'http://'), + ('http://example.com/index.html', 'http://'), + ('localhost', 'http://'), + ('localhost/index.html', 'http://'), - def test_file_uri_absolute001(self): - """Absolute file paths get prepended with "file://" scheme""" - iurl = '/home/user/www/index.html' - url = guess_scheme(iurl) - self.assertEquals(url, 'file://'+iurl) + # some corner cases (default to http://) + ('/', 'http://'), + ('.../test', 'http://'), - def test_file_uri_scheme(self): - """Output File URI does not change if "file://" scheme is set""" - iurl = 'file:///home/user/www/index.html' - url = guess_scheme(iurl) - self.assertEquals(url, iurl) + ], start=1): + t_method = create_guess_scheme_t(args) + t_method.__name__ = 'test_uri_%03d' % k + setattr (ShellURLTest, t_method.__name__, t_method) - def test_file_uri_windows(self): - raise unittest.SkipTest("Windows filepath are not supported for scrapy shell") - url = guess_scheme('C:\absolute\path\to\a\file.html') - assert url.startswith('file://') - - def test_http_url_001(self): - url = guess_scheme('index.html') - assert url.startswith('http://') - - def test_http_url_002(self): - url = guess_scheme('example.com') - assert url.startswith('http://') - - def test_http_url_003(self): - url = guess_scheme('www.example.com') - assert url.startswith('http://') - - def test_http_url_004(self): - url = guess_scheme('www.example.com/index') - assert url.startswith('http://') - - def test_http_url_005(self): - url = guess_scheme('www.example.com/index.html') - assert url.startswith('http://') - - def test_http_url_scheme(self): - """An full HTTP URL is unaltered""" - iurl = 'http://www.example.com/index.html' - url = guess_scheme(iurl) - self.assertEquals(url, iurl) +# TODO: the following tests do not pass with current implementation +for k, args in enumerate ([ + ('C:\absolute\path\to\a\file.html', 'file://', + 'Windows filepath are not supported for scrapy shell'), + ], start=1): + t_method = create_skipped_scheme_t(args) + t_method.__name__ = 'test_uri_skipped_%03d' % k + setattr (ShellURLTest, t_method.__name__, t_method) class ShellTest(ProcessTest, SiteTest, unittest.TestCase): From 1a30a7774b7d530394fdd761f2a59403b778fe10 Mon Sep 17 00:00:00 2001 From: Paul Tremberth Date: Fri, 22 Jan 2016 18:22:19 +0100 Subject: [PATCH 12/20] Use pytest.mark.parametrize decorator --- tests/test_command_shell.py | 96 +++++++++++++++---------------------- 1 file changed, 39 insertions(+), 57 deletions(-) diff --git a/tests/test_command_shell.py b/tests/test_command_shell.py index a61d520fa..9032e4124 100644 --- a/tests/test_command_shell.py +++ b/tests/test_command_shell.py @@ -1,5 +1,7 @@ from os.path import join +import pytest + from twisted.trial import unittest from twisted.internet import defer @@ -10,63 +12,6 @@ from scrapy.utils.testproc import ProcessTest from tests import tests_datadir -class ShellURLTest(unittest.TestCase): - pass - -def create_guess_scheme_t(args): - def do_expected(self): - url = guess_scheme(args[0]) - assert url.startswith(args[1]), \ - 'Wrong scheme guessed: for `%s` got `%s`, expected `%s...`' % ( - args[0], url, args[1]) - return do_expected - -def create_skipped_scheme_t(args): - def do_expected(self): - raise unittest.SkipTest(args[2]) - url = guess_scheme(args[0]) - assert url.startswith(args[1]) - return do_expected - -for k, args in enumerate ([ - ('/index', 'file://'), - ('/index.html', 'file://'), - ('./index.html', 'file://'), - ('../index.html', 'file://'), - ('../../index.html', 'file://'), - ('./data/index.html', 'file://'), - ('.hidden/data/index.html', 'file://'), - ('/home/user/www/index.html', 'file://'), - ('//home/user/www/index.html', 'file://'), - ('file:///home/user/www/index.html', 'file://'), - - ('index.html', 'http://'), - ('example.com', 'http://'), - ('www.example.com', 'http://'), - ('www.example.com/index.html', 'http://'), - ('http://example.com', 'http://'), - ('http://example.com/index.html', 'http://'), - ('localhost', 'http://'), - ('localhost/index.html', 'http://'), - - # some corner cases (default to http://) - ('/', 'http://'), - ('.../test', 'http://'), - - ], start=1): - t_method = create_guess_scheme_t(args) - t_method.__name__ = 'test_uri_%03d' % k - setattr (ShellURLTest, t_method.__name__, t_method) - -# TODO: the following tests do not pass with current implementation -for k, args in enumerate ([ - ('C:\absolute\path\to\a\file.html', 'file://', - 'Windows filepath are not supported for scrapy shell'), - ], start=1): - t_method = create_skipped_scheme_t(args) - t_method.__name__ = 'test_uri_skipped_%03d' % k - setattr (ShellURLTest, t_method.__name__, t_method) - class ShellTest(ProcessTest, SiteTest, unittest.TestCase): @@ -136,3 +81,40 @@ class ShellTest(ProcessTest, SiteTest, unittest.TestCase): check_code=False) self.assertEqual(errcode, 1, out or err) self.assertIn(b'DNS lookup failed', err) + + +@pytest.mark.parametrize("url, scheme", [ + ('/index', 'file://'), + ('/index.html', 'file://'), + ('./index.html', 'file://'), + ('../index.html', 'file://'), + ('../../index.html', 'file://'), + ('./data/index.html', 'file://'), + ('.hidden/data/index.html', 'file://'), + ('/home/user/www/index.html', 'file://'), + ('//home/user/www/index.html', 'file://'), + ('file:///home/user/www/index.html', 'file://'), + + ('index.html', 'http://'), + ('example.com', 'http://'), + ('www.example.com', 'http://'), + ('www.example.com/index.html', 'http://'), + ('http://example.com', 'http://'), + ('http://example.com/index.html', 'http://'), + ('localhost', 'http://'), + ('localhost/index.html', 'http://'), + + # some corner cases (default to http://) + ('/', 'http://'), + ('.../test', 'http://'), + + pytest.mark.xfail( + (r'C:\absolute\path\to\a\file.html', 'file://'), + reason = 'Windows filepath are not supported for scrapy shell' + ), +]) +def test_guess_scheme(url, scheme): + guessed_url = guess_scheme(url) + assert guessed_url.startswith(scheme), \ + 'Wrong scheme guessed: for `%s` got `%s`, expected `%s...`' % ( + url, guessed_url, scheme) From 5f09da60c1a360cf0bd55cfcc892e44a13d5c583 Mon Sep 17 00:00:00 2001 From: Paul Tremberth Date: Fri, 22 Jan 2016 23:48:58 +0100 Subject: [PATCH 13/20] Revert "Use pytest.mark.parametrize decorator" This reverts commit 1a30a7774b7d530394fdd761f2a59403b778fe10. --- tests/test_command_shell.py | 96 ++++++++++++++++++++++--------------- 1 file changed, 57 insertions(+), 39 deletions(-) diff --git a/tests/test_command_shell.py b/tests/test_command_shell.py index 9032e4124..a61d520fa 100644 --- a/tests/test_command_shell.py +++ b/tests/test_command_shell.py @@ -1,7 +1,5 @@ from os.path import join -import pytest - from twisted.trial import unittest from twisted.internet import defer @@ -12,6 +10,63 @@ from scrapy.utils.testproc import ProcessTest from tests import tests_datadir +class ShellURLTest(unittest.TestCase): + pass + +def create_guess_scheme_t(args): + def do_expected(self): + url = guess_scheme(args[0]) + assert url.startswith(args[1]), \ + 'Wrong scheme guessed: for `%s` got `%s`, expected `%s...`' % ( + args[0], url, args[1]) + return do_expected + +def create_skipped_scheme_t(args): + def do_expected(self): + raise unittest.SkipTest(args[2]) + url = guess_scheme(args[0]) + assert url.startswith(args[1]) + return do_expected + +for k, args in enumerate ([ + ('/index', 'file://'), + ('/index.html', 'file://'), + ('./index.html', 'file://'), + ('../index.html', 'file://'), + ('../../index.html', 'file://'), + ('./data/index.html', 'file://'), + ('.hidden/data/index.html', 'file://'), + ('/home/user/www/index.html', 'file://'), + ('//home/user/www/index.html', 'file://'), + ('file:///home/user/www/index.html', 'file://'), + + ('index.html', 'http://'), + ('example.com', 'http://'), + ('www.example.com', 'http://'), + ('www.example.com/index.html', 'http://'), + ('http://example.com', 'http://'), + ('http://example.com/index.html', 'http://'), + ('localhost', 'http://'), + ('localhost/index.html', 'http://'), + + # some corner cases (default to http://) + ('/', 'http://'), + ('.../test', 'http://'), + + ], start=1): + t_method = create_guess_scheme_t(args) + t_method.__name__ = 'test_uri_%03d' % k + setattr (ShellURLTest, t_method.__name__, t_method) + +# TODO: the following tests do not pass with current implementation +for k, args in enumerate ([ + ('C:\absolute\path\to\a\file.html', 'file://', + 'Windows filepath are not supported for scrapy shell'), + ], start=1): + t_method = create_skipped_scheme_t(args) + t_method.__name__ = 'test_uri_skipped_%03d' % k + setattr (ShellURLTest, t_method.__name__, t_method) + class ShellTest(ProcessTest, SiteTest, unittest.TestCase): @@ -81,40 +136,3 @@ class ShellTest(ProcessTest, SiteTest, unittest.TestCase): check_code=False) self.assertEqual(errcode, 1, out or err) self.assertIn(b'DNS lookup failed', err) - - -@pytest.mark.parametrize("url, scheme", [ - ('/index', 'file://'), - ('/index.html', 'file://'), - ('./index.html', 'file://'), - ('../index.html', 'file://'), - ('../../index.html', 'file://'), - ('./data/index.html', 'file://'), - ('.hidden/data/index.html', 'file://'), - ('/home/user/www/index.html', 'file://'), - ('//home/user/www/index.html', 'file://'), - ('file:///home/user/www/index.html', 'file://'), - - ('index.html', 'http://'), - ('example.com', 'http://'), - ('www.example.com', 'http://'), - ('www.example.com/index.html', 'http://'), - ('http://example.com', 'http://'), - ('http://example.com/index.html', 'http://'), - ('localhost', 'http://'), - ('localhost/index.html', 'http://'), - - # some corner cases (default to http://) - ('/', 'http://'), - ('.../test', 'http://'), - - pytest.mark.xfail( - (r'C:\absolute\path\to\a\file.html', 'file://'), - reason = 'Windows filepath are not supported for scrapy shell' - ), -]) -def test_guess_scheme(url, scheme): - guessed_url = guess_scheme(url) - assert guessed_url.startswith(scheme), \ - 'Wrong scheme guessed: for `%s` got `%s`, expected `%s...`' % ( - url, guessed_url, scheme) From fb8ab2427bff636375b64fdc459562432c8ac420 Mon Sep 17 00:00:00 2001 From: Paul Tremberth Date: Mon, 25 Jan 2016 13:13:35 +0100 Subject: [PATCH 14/20] Move urlparsing statement in add_http_if_no_scheme() --- scrapy/utils/url.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/scrapy/utils/url.py b/scrapy/utils/url.py index 0e36003ad..0acbbb6ab 100644 --- a/scrapy/utils/url.py +++ b/scrapy/utils/url.py @@ -116,8 +116,8 @@ def escape_ajax(url): def add_http_if_no_scheme(url): """Add http as the default scheme if it is missing from the url.""" match = re.match(r"^\w+://", url, flags=re.I) - parts = urlparse(url) if not match: + parts = urlparse(url) scheme = "http:" if parts.netloc else "http://" url = scheme + url From 713e1eee9b14ef95515b20baacb83daba9c1277a Mon Sep 17 00:00:00 2001 From: Paul Tremberth Date: Tue, 26 Jan 2016 10:44:38 +0100 Subject: [PATCH 15/20] Update docs about local files support for "scrapy shell" --- docs/topics/commands.rst | 4 +++- docs/topics/shell.rst | 30 ++++++++++++++++++++++++++++++ 2 files changed, 33 insertions(+), 1 deletion(-) diff --git a/docs/topics/commands.rst b/docs/topics/commands.rst index 16af52eea..9a40a2c29 100644 --- a/docs/topics/commands.rst +++ b/docs/topics/commands.rst @@ -373,7 +373,9 @@ shell * Requires project: *no* Starts the Scrapy shell for the given URL (if given) or empty if no URL is -given. See :ref:`topics-shell` for more info. +given. Also supports UNIX-style local file paths, either relative with +``./`` or ``../`` prefixes or absolute file paths. +See :ref:`topics-shell` for more info. Usage example:: diff --git a/docs/topics/shell.rst b/docs/topics/shell.rst index 3569cbf37..4af11fbb6 100644 --- a/docs/topics/shell.rst +++ b/docs/topics/shell.rst @@ -53,6 +53,36 @@ this:: Where the ```` is the URL you want to scrape. +:command:`shell` also works for local files. This can be handy if you want +to play around with a local copy of a web page. :command:`shell` understands +the following syntaxes for local files:: + + # UNIX-style + scrapy shell ./path/to/file.html + scrapy shell ../other/path/to/file.html + scrapy shell /absolute/path/to/file.html + + # File URI + scrapy shell file:///absolute/path/to/file.html + +.. warning:: :command:`shell` will interpret ``index.html`` as a domain name, + not as a relative path to a local file, and will trigger a DNS lookup error:: + + $ scrapy shell index.html + [ ... scrapy shell starts ... ] + 2016-01-26 10:29:51 [scrapy] DEBUG: Gave up retrying + (failed 3 times): DNS lookup failed: + address 'index.html' not found: [Errno -5] No address associated with hostname. + [ ... traceback ... ] + twisted.internet.error.DNSLookupError: DNS lookup failed: + address 'index.html' not found: [Errno -5] No address associated with hostname. + + Use ``./`` prefix instead:: + + $ scrapy shell ./index.html + [ ... scrapy shell starts ... ] + + Using the shell =============== From bb1f4013a3883d72bcaa14b06a86ab428b99adfa Mon Sep 17 00:00:00 2001 From: Paul Tremberth Date: Tue, 26 Jan 2016 17:23:28 +0100 Subject: [PATCH 16/20] Rewrite warning about shell with local files as note --- docs/topics/shell.rst | 28 +++++++++++++++------------- 1 file changed, 15 insertions(+), 13 deletions(-) diff --git a/docs/topics/shell.rst b/docs/topics/shell.rst index 4af11fbb6..a6ca036d2 100644 --- a/docs/topics/shell.rst +++ b/docs/topics/shell.rst @@ -65,22 +65,24 @@ the following syntaxes for local files:: # File URI scrapy shell file:///absolute/path/to/file.html -.. warning:: :command:`shell` will interpret ``index.html`` as a domain name, - not as a relative path to a local file, and will trigger a DNS lookup error:: +.. note:: When using relative file paths, be explicit and prepend them + with ``./`` (or ``../`` when relevant). + ``scrapy shell index.html`` will not work as one might expect (and + this is by design, not a bug). - $ scrapy shell index.html - [ ... scrapy shell starts ... ] - 2016-01-26 10:29:51 [scrapy] DEBUG: Gave up retrying - (failed 3 times): DNS lookup failed: - address 'index.html' not found: [Errno -5] No address associated with hostname. - [ ... traceback ... ] - twisted.internet.error.DNSLookupError: DNS lookup failed: - address 'index.html' not found: [Errno -5] No address associated with hostname. + Because :command:`shell` favors HTTP URLs over File URIs, + and ``index.html`` being syntactically similar to ``example.com``, + :command:`shell` will treat ``index.html`` as a domain name and trigger + a DNS lookup error:: - Use ``./`` prefix instead:: + $ scrapy shell index.html + [ ... scrapy shell starts ... ] + [ ... traceback ... ] + twisted.internet.error.DNSLookupError: DNS lookup failed: + address 'index.html' not found: [Errno -5] No address associated with hostname. - $ scrapy shell ./index.html - [ ... scrapy shell starts ... ] + :command:`shell` will not test beforehand if a file called ``index.html`` + exists in the current directory. Again, be explicit. Using the shell From cae268402d13a6d419c56024b3051b1cad3d82e1 Mon Sep 17 00:00:00 2001 From: Paul Tremberth Date: Thu, 28 Jan 2016 13:42:04 +0100 Subject: [PATCH 17/20] Move guess_scheme() to scrapy.utils.url --- scrapy/commands/shell.py | 29 +---------------------------- scrapy/utils/url.py | 24 ++++++++++++++++++++++++ tests/test_command_shell.py | 2 +- 3 files changed, 26 insertions(+), 29 deletions(-) diff --git a/scrapy/commands/shell.py b/scrapy/commands/shell.py index 5201feb42..7be7f7256 100644 --- a/scrapy/commands/shell.py +++ b/scrapy/commands/shell.py @@ -3,40 +3,13 @@ Scrapy Shell See documentation in docs/topics/shell.rst """ -import re -from six.moves.urllib.parse import urlparse from threading import Thread -from w3lib.url import any_to_uri from scrapy.commands import ScrapyCommand from scrapy.shell import Shell from scrapy.http import Request -from scrapy.utils.url import add_http_if_no_scheme from scrapy.utils.spider import spidercls_for_request, DefaultSpider - - -def guess_scheme(url): - """Given an URL as string, - returns a FileURI if it looks like a file path, - otherwise returns an HTTP URL - """ - parts = urlparse(url) - if parts.scheme: - return url - # Note: this does not match Windows filepath - if re.match(r'''^ # start with... - ( - \. # ...a single dot, - ( - \. | [^/\.]+ # optionally followed by - )? # either a second dot or some characters - )? # optional match of ".", ".." or ".blabla" - / # at least one "/" for a file path, - . # and something after the "/" - ''', parts.path, flags=re.VERBOSE): - return any_to_uri(url) - else: - return add_http_if_no_scheme(url) +from scrapy.utils.url import guess_scheme class Command(ScrapyCommand): diff --git a/scrapy/utils/url.py b/scrapy/utils/url.py index 0acbbb6ab..4b47566e5 100644 --- a/scrapy/utils/url.py +++ b/scrapy/utils/url.py @@ -122,3 +122,27 @@ def add_http_if_no_scheme(url): url = scheme + url return url + + +def guess_scheme(url): + """Given an URL as string, + returns a FileURI if it looks like a file path, + otherwise returns an HTTP URL + """ + parts = urlparse(url) + if parts.scheme: + return url + # Note: this does not match Windows filepath + if re.match(r'''^ # start with... + ( + \. # ...a single dot, + ( + \. | [^/\.]+ # optionally followed by + )? # either a second dot or some characters + )? # optional match of ".", ".." or ".blabla" + / # at least one "/" for a file path, + . # and something after the "/" + ''', parts.path, flags=re.VERBOSE): + return any_to_uri(url) + else: + return add_http_if_no_scheme(url) diff --git a/tests/test_command_shell.py b/tests/test_command_shell.py index a61d520fa..35a5fa21a 100644 --- a/tests/test_command_shell.py +++ b/tests/test_command_shell.py @@ -3,9 +3,9 @@ from os.path import join from twisted.trial import unittest from twisted.internet import defer -from scrapy.commands.shell import guess_scheme from scrapy.utils.testsite import SiteTest from scrapy.utils.testproc import ProcessTest +from scrapy.utils.url import guess_scheme from tests import tests_datadir From 481e251775a089a8e82c480a83181146d1bb6847 Mon Sep 17 00:00:00 2001 From: Paul Tremberth Date: Thu, 28 Jan 2016 13:51:50 +0100 Subject: [PATCH 18/20] Move guess_scheme() tests to relevant test module --- tests/test_command_shell.py | 58 --------------------------------- tests/test_utils_url.py | 65 +++++++++++++++++++++++++++++++++++-- 2 files changed, 62 insertions(+), 61 deletions(-) diff --git a/tests/test_command_shell.py b/tests/test_command_shell.py index 35a5fa21a..9d0965902 100644 --- a/tests/test_command_shell.py +++ b/tests/test_command_shell.py @@ -10,64 +10,6 @@ from scrapy.utils.url import guess_scheme from tests import tests_datadir -class ShellURLTest(unittest.TestCase): - pass - -def create_guess_scheme_t(args): - def do_expected(self): - url = guess_scheme(args[0]) - assert url.startswith(args[1]), \ - 'Wrong scheme guessed: for `%s` got `%s`, expected `%s...`' % ( - args[0], url, args[1]) - return do_expected - -def create_skipped_scheme_t(args): - def do_expected(self): - raise unittest.SkipTest(args[2]) - url = guess_scheme(args[0]) - assert url.startswith(args[1]) - return do_expected - -for k, args in enumerate ([ - ('/index', 'file://'), - ('/index.html', 'file://'), - ('./index.html', 'file://'), - ('../index.html', 'file://'), - ('../../index.html', 'file://'), - ('./data/index.html', 'file://'), - ('.hidden/data/index.html', 'file://'), - ('/home/user/www/index.html', 'file://'), - ('//home/user/www/index.html', 'file://'), - ('file:///home/user/www/index.html', 'file://'), - - ('index.html', 'http://'), - ('example.com', 'http://'), - ('www.example.com', 'http://'), - ('www.example.com/index.html', 'http://'), - ('http://example.com', 'http://'), - ('http://example.com/index.html', 'http://'), - ('localhost', 'http://'), - ('localhost/index.html', 'http://'), - - # some corner cases (default to http://) - ('/', 'http://'), - ('.../test', 'http://'), - - ], start=1): - t_method = create_guess_scheme_t(args) - t_method.__name__ = 'test_uri_%03d' % k - setattr (ShellURLTest, t_method.__name__, t_method) - -# TODO: the following tests do not pass with current implementation -for k, args in enumerate ([ - ('C:\absolute\path\to\a\file.html', 'file://', - 'Windows filepath are not supported for scrapy shell'), - ], start=1): - t_method = create_skipped_scheme_t(args) - t_method.__name__ = 'test_uri_skipped_%03d' % k - setattr (ShellURLTest, t_method.__name__, t_method) - - class ShellTest(ProcessTest, SiteTest, unittest.TestCase): command = 'shell' diff --git a/tests/test_utils_url.py b/tests/test_utils_url.py index 314ccd30f..73ad11f8a 100644 --- a/tests/test_utils_url.py +++ b/tests/test_utils_url.py @@ -4,7 +4,8 @@ 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_http_if_no_scheme) + canonicalize_url, add_http_if_no_scheme, + guess_scheme) __doctests__ = ['scrapy.utils.url'] @@ -188,7 +189,7 @@ class CanonicalizeUrlTest(unittest.TestCase): class AddHttpIfNoScheme(unittest.TestCase): - + def test_add_scheme(self): self.assertEqual(add_http_if_no_scheme('www.example.com'), 'http://www.example.com') @@ -216,7 +217,7 @@ class AddHttpIfNoScheme(unittest.TestCase): 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') @@ -294,5 +295,63 @@ class AddHttpIfNoScheme(unittest.TestCase): 'ftp://www.example.com') +class GuessSchemeTest(unittest.TestCase): + pass + +def create_guess_scheme_t(args): + def do_expected(self): + url = guess_scheme(args[0]) + assert url.startswith(args[1]), \ + 'Wrong scheme guessed: for `%s` got `%s`, expected `%s...`' % ( + args[0], url, args[1]) + return do_expected + +def create_skipped_scheme_t(args): + def do_expected(self): + raise unittest.SkipTest(args[2]) + url = guess_scheme(args[0]) + assert url.startswith(args[1]) + return do_expected + +for k, args in enumerate ([ + ('/index', 'file://'), + ('/index.html', 'file://'), + ('./index.html', 'file://'), + ('../index.html', 'file://'), + ('../../index.html', 'file://'), + ('./data/index.html', 'file://'), + ('.hidden/data/index.html', 'file://'), + ('/home/user/www/index.html', 'file://'), + ('//home/user/www/index.html', 'file://'), + ('file:///home/user/www/index.html', 'file://'), + + ('index.html', 'http://'), + ('example.com', 'http://'), + ('www.example.com', 'http://'), + ('www.example.com/index.html', 'http://'), + ('http://example.com', 'http://'), + ('http://example.com/index.html', 'http://'), + ('localhost', 'http://'), + ('localhost/index.html', 'http://'), + + # some corner cases (default to http://) + ('/', 'http://'), + ('.../test', 'http://'), + + ], start=1): + t_method = create_guess_scheme_t(args) + t_method.__name__ = 'test_uri_%03d' % k + setattr (GuessSchemeTest, t_method.__name__, t_method) + +# TODO: the following tests do not pass with current implementation +for k, args in enumerate ([ + ('C:\absolute\path\to\a\file.html', 'file://', + 'Windows filepath are not supported for scrapy shell'), + ], start=1): + t_method = create_skipped_scheme_t(args) + t_method.__name__ = 'test_uri_skipped_%03d' % k + setattr (GuessSchemeTest, t_method.__name__, t_method) + + if __name__ == "__main__": unittest.main() From e9f6b98816c220129fb02e6c09f488ecc8d686bd Mon Sep 17 00:00:00 2001 From: Paul Tremberth Date: Thu, 28 Jan 2016 14:39:19 +0100 Subject: [PATCH 19/20] Amend guess_scheme() docstring --- scrapy/utils/url.py | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/scrapy/utils/url.py b/scrapy/utils/url.py index 4b47566e5..adef4a800 100644 --- a/scrapy/utils/url.py +++ b/scrapy/utils/url.py @@ -125,10 +125,7 @@ def add_http_if_no_scheme(url): def guess_scheme(url): - """Given an URL as string, - returns a FileURI if it looks like a file path, - otherwise returns an HTTP URL - """ + """Add an URL scheme if missing: file:// for filepath-like input or http:// otherwise.""" parts = urlparse(url) if parts.scheme: return url From 78f00401cd284fd7bdcbb525ee94ea0bbedaa7cd Mon Sep 17 00:00:00 2001 From: Paul Tremberth Date: Fri, 29 Jan 2016 16:56:05 +0100 Subject: [PATCH 20/20] Remove unused import in tests --- tests/test_command_shell.py | 1 - 1 file changed, 1 deletion(-) diff --git a/tests/test_command_shell.py b/tests/test_command_shell.py index 9d0965902..c532fc0d8 100644 --- a/tests/test_command_shell.py +++ b/tests/test_command_shell.py @@ -5,7 +5,6 @@ from twisted.internet import defer from scrapy.utils.testsite import SiteTest from scrapy.utils.testproc import ProcessTest -from scrapy.utils.url import guess_scheme from tests import tests_datadir