Add method StaticFileHandler.get_content_version.

This method is easier for subclasses to override (the base class still
handles caching) and lets us use the post-validation absolute path,
fixing some issues with default_filename support.

Improve StaticFileHandler test coverage.

diff --git a/MANIFEST.in b/MANIFEST.in
index 46e3efc0fa3db282de387511e712229d1e5633a0..b710aac98090ca7e5bdd14941ba73d666bec0ba3 100644 (file)
--- a/MANIFEST.in
+++ b/MANIFEST.in
@@ -6,6 +6,7 @@ include tornado/test/gettext_translations/fr_FR/LC_MESSAGES/tornado_test.mo
 include tornado/test/gettext_translations/fr_FR/LC_MESSAGES/tornado_test.po
 include tornado/test/options_test.cfg
 include tornado/test/static/robots.txt
+include tornado/test/static/dir/index.html
 include tornado/test/templates/utf8.html
 include tornado/test/test.crt
 include tornado/test/test.key

diff --git a/setup.py b/setup.py
index d3849f0d902e5734e919993f62231ca496ec8415..9902eeb8772aeb1a6146d78c68bff924a3d92858 100644 (file)
--- a/setup.py
+++ b/setup.py
@@ -46,6 +46,7 @@ distutils.core.setup(
             "gettext_translations/fr_FR/LC_MESSAGES/tornado_test.po",
             "options_test.cfg",
             "static/robots.txt",
+            "static/dir/index.html",
             "templates/utf8.html",
             "test.crt",
             "test.key",

diff --git a/tornado/test/static/dir/index.html b/tornado/test/static/dir/index.html
new file mode 100644 (file)
index 0000000..e1cd9d8
--- /dev/null
+++ b/tornado/test/static/dir/index.html
@@ -0,0 +1 @@
+this is the index

diff --git a/tornado/test/web_test.py b/tornado/test/web_test.py
index 816b7b68d2f91d9e7907ffe3d82317f7059cb883..a4a089833de7666fdc1228bad1e5f4069bfe3397 100644 (file)
--- a/tornado/test/web_test.py
+++ b/tornado/test/web_test.py
@@ -900,6 +900,60 @@ class StaticFileTest(WebTestCase):
                 'Range': 'asdf'})
         self.assertEqual(response.code, 416)
 
+    def test_static_head(self):
+        response = self.fetch('/static/robots.txt', method='HEAD')
+        self.assertEqual(response.code, 200)
+        # No body was returned, but we did get the right content length.
+        self.assertEqual(response.body, b'')
+        self.assertEqual(response.headers['Content-Length'], '26')
+        self.assertEqual(utf8(response.headers['Etag']),
+                         b'"' + self.robots_txt_hash + b'"')
+
+    def test_static_head_range(self):
+        response = self.fetch('/static/robots.txt', method='HEAD',
+                              headers={'Range': 'bytes=1-4'})
+        self.assertEqual(response.code, 206)
+        self.assertEqual(response.body, b'')
+        self.assertEqual(response.headers['Content-Length'], '4')
+        self.assertEqual(utf8(response.headers['Etag']),
+                         b'"' + self.robots_txt_hash + b'"')
+
+    def test_static_range_if_none_match(self):
+        response = self.fetch('/static/robots.txt', headers={
+            'Range': 'bytes=1-4',
+            'If-None-Match': b'"' + self.robots_txt_hash + b'"'})
+        self.assertEqual(response.code, 304)
+        self.assertEqual(response.body, b'')
+        self.assertTrue('Content-Length' not in response.headers)
+        self.assertEqual(utf8(response.headers['Etag']),
+                         b'"' + self.robots_txt_hash + b'"')
+
+    def test_static_404(self):
+        response = self.fetch('/static/blarg')
+        self.assertEqual(response.code, 404)
+
+
+@wsgi_safe
+class StaticDefaultFilenameTest(WebTestCase):
+    def get_app_kwargs(self):
+        return dict(static_path=os.path.join(os.path.dirname(__file__),
+                                             'static'),
+                    static_handler_args=dict(default_filename='index.html'))
+
+    def get_handlers(self):
+        return []
+
+    def test_static_default_filename(self):
+        response = self.fetch('/static/dir/', follow_redirects=False)
+        self.assertEqual(response.code, 200)
+        self.assertEqual(b'this is the index\n', response.body)
+
+    def test_static_default_redirect(self):
+        response = self.fetch('/static/dir', follow_redirects=False)
+        self.assertEqual(response.code, 301)
+        self.assertTrue(response.headers['Location'].endswith('/static/dir/'))
+
+
 
 @wsgi_safe
 class CustomStaticFileTest(WebTestCase):

diff --git a/tornado/web.py b/tornado/web.py
index 1292fb44e4148a9ace0c4e365d931542b67df99e..998380753e73a49b72dd0204a31c65507c385b79 100644 (file)
--- a/tornado/web.py
+++ b/tornado/web.py
@@ -1773,6 +1773,8 @@ class StaticFileHandler(RequestHandler):
         del path  # make sure we don't refer to path instead of self.path again
         absolute_path = self.get_absolute_path(self.settings, self.path)
         self.absolute_path = self.validate_absolute_path(absolute_path)
+        if self.absolute_path is None:
+            return
 
         self.modified = self.get_modified_time()
         self.set_headers()
@@ -1812,7 +1814,7 @@ class StaticFileHandler(RequestHandler):
         versions, and sends the correct ``Etag`` for a partial response
         (i.e. the same ``Etag`` as the full file).
         """
-        version_hash = self.get_version(self.settings, self.path_args[0])
+        version_hash = self._get_cached_version(self.absolute_path)
         if not version_hash:
             return None
         return '"%s"' %(version_hash, )
@@ -1893,7 +1895,7 @@ class StaticFileHandler(RequestHandler):
             # but there is some prefix to the path that was already
             # trimmed by the routing
             if not self.request.path.endswith("/"):
-                self.redirect(self.request.path + "/")
+                self.redirect(self.request.path + "/", permanent=True)
                 return
             absolute_path = os.path.join(absolute_path, self.default_filename)
         if not os.path.exists(absolute_path):
@@ -1915,6 +1917,16 @@ class StaticFileHandler(RequestHandler):
         with open(abspath, "rb") as file:
             return file.read()
 
+    @classmethod
+    def get_content_version(cls, abspath):
+        """Returns a version string for the resource at the given path.
+
+        This class method may be overridden by subclasses.  The
+        default implementation is a hash of the file's contents.
+        """
+        data = cls.get_content(abspath)
+        return hashlib.md5(data).hexdigest()
+
     def get_modified_time(self):
         """Returns the time that ``self.absolute_path`` was last modified.
 
@@ -1981,24 +1993,27 @@ class StaticFileHandler(RequestHandler):
     def get_version(cls, settings, path):
         """Generate the version string to be used in static URLs.
 
-        This method may be overridden in subclasses (but note that it
-        is a class method rather than a static method).  The default
-        implementation uses a hash of the file's contents.
-
         ``settings`` is the `Application.settings` dictionary and ``path``
         is the relative location of the requested asset on the filesystem.
         The returned value should be a string, or ``None`` if no version
         could be determined.
+
+        This method was previously recommended for subclasses to override;
+        `get_content_version` is now preferred as it allows the base
+        class to handle caching of the result.
         """
         abs_path = cls.get_absolute_path(settings, path)
+        return cls._get_cached_version(abs_path)
+
+    @classmethod
+    def _get_cached_version(cls, abs_path):
         with cls._lock:
             hashes = cls._static_hashes
             if abs_path not in hashes:
                 try:
-                    data = cls.get_content(abs_path)
-                    hashes[abs_path] = hashlib.md5(data).hexdigest()
+                    hashes[abs_path] = cls.get_content_version(abs_path)
                 except Exception:
-                    gen_log.error("Could not open static file %r", path)
+                    gen_log.error("Could not open static file %r", abs_path)
                     hashes[abs_path] = None
             hsh = hashes.get(abs_path)
             if hsh: