Repository: scrapy/scrapy
Pull request #1710 (2016-01-21 22:04:51)

[MRG+1] shell command's ability to open local files + tests
This is a continuation of @side2k's #1579 


Patch:
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..a6ca036d2 100644
--- a/docs/topics/shell.rst
+++ b/docs/topics/shell.rst
@@ -53,6 +53,38 @@ this::
 
 Where the ``<url>`` 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
+
+.. 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).
+
+    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::
+
+        $ 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.
+
+    :command:`shell` will not test beforehand if a file called ``index.html``
+    exists in the current directory. Again, be explicit.
+
+
 Using the shell
 ===============
 
diff --git a/scrapy/commands/shell.py b/scrapy/commands/shell.py
index 77ae1358b..7be7f7256 100644
--- a/scrapy/commands/shell.py
+++ b/scrapy/commands/shell.py
@@ -3,14 +3,13 @@ Scrapy Shell
 
 See documentation in docs/topics/shell.rst
 """
-
 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_http_if_no_scheme
 from scrapy.utils.spider import spidercls_for_request, DefaultSpider
+from scrapy.utils.url import guess_scheme
 
 
 class Command(ScrapyCommand):
@@ -47,7 +46,9 @@ class Command(ScrapyCommand):
     def run(self, args, opts):
         url = args[0] if args else None
         if url:
-            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
 
         spidercls = DefaultSpider
diff --git a/scrapy/utils/url.py b/scrapy/utils/url.py
index 398407a64..adef4a800 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
+import re
 from six.moves.urllib.parse import (ParseResult, urlunparse, urldefrag,
                                     urlparse, parse_qsl, urlencode,
                                     unquote)
@@ -114,10 +115,31 @@ 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
+    match = re.match(r"^\w+://", url, flags=re.I)
+    if not match:
+        parts = urlparse(url)
+        scheme = "http:" if parts.netloc else "http://"
+        url = scheme + url
+
     return url
+
+
+def guess_scheme(url):
+    """Add an URL scheme if missing: file:// for filepath-like input or http:// otherwise."""
+    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)


Test patch:
diff --git a/tests/test_command_shell.py b/tests/test_command_shell.py
index 105202754..c532fc0d8 100644
--- a/tests/test_command_shell.py
+++ b/tests/test_command_shell.py
@@ -1,9 +1,13 @@
+from os.path import join
+
 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 +55,25 @@ 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_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_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)
+
+    @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)
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()