#921 show directory index (from PR #1033) (#1094)

tejksat · asvetlov · commit 90ec4ebf07a9 · 2016-08-22T18:01:12.000+03:00
* Add `show_index` flag to StaticRoute Related: #921 * Accessing static dir, should return 403 by default If we want to access a static file dir, it should return `Forbidden (403)` by default. Related: #921 * WIP: if allowed - return directory's index, otherwise - 403 XXX: need for a response to return proper html Related: #921 * Return directory index list if we allow to show it Also I now return in place, instead of creating variable and returning later, I am not a fan of returning somewehere inside a method, though if we tried to return `ret` at the end as before, but I guess it's the most clean pattern to do this. This is because we have to conditional blocks, either of which can return from the method. If first condtitonal creates `ret` variable, later conditional may just raise `HTTPNotFound` which we do not want. Though, I do not want to check that `ret` is populated either. Thus return in place. Related: #921 * Test if correct statuses are returned if we allow acces index or not Related: #921 * Prettier directory output - Better method name - More human readable output (newlines) - Title is shown as relative path to static directory and not as posix path - Show directories with slash at the end of the name * Remove unnecessary comment * Update docs And fix a minor typo * Check of html content is added for the response page with a directory index Related: #921 * Test of accessing non-existing static resource added Related: #921 * 403 Forbidden returned if the permission error occurred on listing requested folder Related: #921 * show_index parameter usage of app.router.add_static method documented Related: #921 * Import order fixed Related: #921 * Make directory index sorted so stable for tests Related: #921 * Improve tests coverage Related: #921 * Improve tests coverage Related: #921
diff --git a/aiohttp/web_urldispatcher.py b/aiohttp/web_urldispatcher.py
@@ -16,9 +16,10 @@
 from .abc import AbstractMatchInfo, AbstractRouter, AbstractView
 from .file_sender import FileSender
 from .protocol import HttpVersion11
-from .web_exceptions import (HTTPExpectationFailed, HTTPMethodNotAllowed,
-                             HTTPNotFound)
-from .web_reqrep import StreamResponse
+from .web_exceptions import (HTTPExpectationFailed, HTTPForbidden,
+                             HTTPMethodNotAllowed, HTTPNotFound)
+from .web_reqrep import Response, StreamResponse
+
 
 __all__ = ('UrlDispatcher', 'UrlMappingMatchInfo',
            'AbstractResource', 'Resource', 'PlainResource', 'DynamicResource',
@@ -437,7 +438,8 @@ class StaticRoute(Route):
 
     def __init__(self, name, prefix, directory, *,
                  expect_handler=None, chunk_size=256*1024,
-                 response_factory=StreamResponse):
+                 response_factory=StreamResponse,
+                 show_index=False):
         assert prefix.startswith('/'), prefix
         assert prefix.endswith('/'), prefix
         super().__init__(
@@ -457,6 +459,7 @@ def __init__(self, name, prefix, directory, *,
         self._directory = directory
         self._file_sender = FileSender(resp_factory=response_factory,
                                        chunk_size=chunk_size)
+        self._show_index = show_index
 
     def match(self, path):
         if not path.startswith(self._prefix):
@@ -489,13 +492,59 @@ def handle(self, request):
             request.app.logger.exception(error)
             raise HTTPNotFound() from error
 
-        # Make sure that filepath is a file
-        if not filepath.is_file():
-            raise HTTPNotFound()
+        # on opening a dir, load it's contents if allowed
+        if filepath.is_dir():
+            if self._show_index:
+                try:
+                    ret = Response(text=self._directory_as_html(filepath))
+                except PermissionError:
+                    raise HTTPForbidden()
+            else:
+                raise HTTPForbidden()
+        elif filepath.is_file():
+            ret = yield from self._file_sender.send(request, filepath)
+        else:
+            raise HTTPNotFound
 
-        ret = yield from self._file_sender.send(request, filepath)
         return ret
 
+    def _directory_as_html(self, filepath):
+        "returns directory's index as html"
+        # sanity check
+        assert filepath.is_dir()
+
+        posix_dir_len = len(self._directory.as_posix())
+
+        # remove the beginning of posix path, so it would be relative
+        # to our added static path
+        relative_path_to_dir = filepath.as_posix()[posix_dir_len:]
+        index_of = "Index of /{}".format(relative_path_to_dir)
+        head = "<head>\n<title>{}</title>\n</head>".format(index_of)
+        h1 = "<h1>{}</h1>".format(index_of)
+
+        index_list = []
+        dir_index = filepath.iterdir()
+        for _file in sorted(dir_index):
+            # show file url as relative to static path
+            file_url = _file.as_posix()[posix_dir_len:]
+
+            # if file is a directory, add '/' to the end of the name
+            if _file.is_dir():
+                file_name = "{}/".format(_file.name)
+            else:
+                file_name = _file.name
+
+            index_list.append(
+                '<li><a href="{url}">{name}</a></li>'.format(url=file_url,
+                                                             name=file_name)
+            )
+        ul = "<ul>\n{}\n</ul>".format('\n'.join(index_list))
+        body = "<body>\n{}\n{}\n</body>".format(h1, ul)
+
+        html = "<html>\n{}\n{}\n</html>".format(head, body)
+
+        return html
+
     def __repr__(self):
         name = "'" + self.name + "' " if self.name is not None else ""
         return "<StaticRoute {name}[{method}] {path} -> {directory!r}".format(
@@ -720,7 +769,8 @@ def add_route(self, method, path, handler,
                                   expect_handler=expect_handler)
 
     def add_static(self, prefix, path, *, name=None, expect_handler=None,
-                   chunk_size=256*1024, response_factory=StreamResponse):
+                   chunk_size=256*1024, response_factory=StreamResponse,
+                   show_index=False):
         """
         Adds static files view
         :param prefix - url prefix
@@ -732,7 +782,8 @@ def add_static(self, prefix, path, *, name=None, expect_handler=None,
         route = StaticRoute(name, prefix, path,
                             expect_handler=expect_handler,
                             chunk_size=chunk_size,
-                            response_factory=response_factory)
+                            response_factory=response_factory,
+                            show_index=show_index)
         self.register_route(route)
         return route
 
diff --git a/docs/web.rst b/docs/web.rst
@@ -341,6 +341,12 @@ To do it just register a new static route by
 
    app.router.add_static('/prefix', path_to_static_folder)
 
+When a directory is accessed within a static route then the server responses
+to client with ``HTTP/403 Forbidden`` by default. Displaying folder index
+instead could be enabled with ``show_index`` parameter set to ``True``::
+
+   app.router.add_static('/prefix', path_to_static_folder, show_index=True)
+
 
 Template Rendering
 ------------------
diff --git a/docs/web_reference.rst b/docs/web_reference.rst
@@ -1289,7 +1289,8 @@ Router is any object that implements :class:`AbstractRouter` interface.
       .. versionadded:: 1.0
 
    .. method:: add_static(prefix, path, *, name=None, expect_handler=None, \
-                          chunk_size=256*1024, response_factory=StreamResponse)
+                          chunk_size=256*1024, response_factory=StreamResponse \
+                          show_index=False)
 
       Adds a router and a handler for returning static files.
 
@@ -1341,6 +1342,10 @@ Router is any object that implements :class:`AbstractRouter` interface.
 
                                         .. versionadded:: 0.17
 
+      :param bool show_index: flag for allowing to show indexes of a directory,
+                              by default it's not allowed and HTTP/403 will
+                              be returned on directory access.
+
    :returns: new :class:`StaticRoute` instance.
 
    .. coroutinemethod:: resolve(request)
diff --git a/tests/test_web_urldispatcher.py b/tests/test_web_urldispatcher.py
@@ -4,6 +4,9 @@
 import shutil
 import tempfile
 
+from unittest import mock
+from unittest.mock import MagicMock
+
 import pytest
 
 import aiohttp.web
@@ -29,28 +32,146 @@ def teardown():
     return tmp_dir
 
 
+@pytest.mark.parametrize("show_index,status,data",
+                         [(False, 403, None),
+                          (True, 200,
+                           b'<html>\n<head>\n<title>Index of /</title>\n'
+                           b'</head>\n<body>\n<h1>Index of /</h1>\n<ul>\n'
+                           b'<li><a href="/my_dir">my_dir/</a></li>\n'
+                           b'<li><a href="/my_file">my_file</a></li>\n'
+                           b'</ul>\n</body>\n</html>')])
 @pytest.mark.run_loop
-def test_access_root_of_static_handler(tmp_dir_path, create_app_and_client):
+def test_access_root_of_static_handler(tmp_dir_path, create_app_and_client,
+                                       show_index, status, data):
     """
     Tests the operation of static file server.
     Try to access the root of static file server, and make
-    sure that a proper not found error is returned.
+    sure that correct HTTP statuses are returned depending if we directory
+    index should be shown or not.
     """
     # Put a file inside tmp_dir_path:
     my_file_path = os.path.join(tmp_dir_path, 'my_file')
     with open(my_file_path, 'w') as fw:
         fw.write('hello')
 
+    my_dir_path = os.path.join(tmp_dir_path, 'my_dir')
+    os.mkdir(my_dir_path)
+
+    my_file_path = os.path.join(my_dir_path, 'my_file_in_dir')
+    with open(my_file_path, 'w') as fw:
+        fw.write('world')
+
     app, client = yield from create_app_and_client()
 
     # Register global static route:
-    app.router.add_static('/', tmp_dir_path)
+    app.router.add_static('/', tmp_dir_path, show_index=show_index)
 
     # Request the root of the static directory.
-    # Expect an 404 error page.
     r = yield from client.get('/')
+    assert r.status == status
+
+    if data:
+        read_ = (yield from r.read())
+        assert read_ == data
+    yield from r.release()
+
+
+@pytest.mark.run_loop
+def test_access_non_existing_resource(tmp_dir_path, create_app_and_client):
+    """
+    Tests accessing non-existing resource
+    Try to access a non-exiting resource and make sure that 404 HTTP status
+    returned.
+    """
+    app, client = yield from create_app_and_client()
+
+    # Register global static route:
+    app.router.add_static('/', tmp_dir_path, show_index=True)
+
+    # Request the root of the static directory.
+    r = yield from client.get('/non_existing_resource')
     assert r.status == 404
-    # data = (yield from r.read())
+    yield from r.release()
+
+
+@pytest.mark.run_loop
+def test_unauthorized_folder_access(tmp_dir_path, create_app_and_client):
+    """
+    Tests the unauthorized access to a folder of static file server.
+    Try to list a folder content of static file server when server does not
+    have permissions to do so for the folder.
+    """
+    my_dir_path = os.path.join(tmp_dir_path, 'my_dir')
+    os.mkdir(my_dir_path)
+
+    app, client = yield from create_app_and_client()
+
+    with mock.patch('pathlib.Path.__new__') as path_constructor:
+        path = MagicMock()
+        path.joinpath.return_value = path
+        path.resolve.return_value = path
+        path.iterdir.return_value.__iter__.side_effect = PermissionError()
+        path_constructor.return_value = path
+
+        # Register global static route:
+        app.router.add_static('/', tmp_dir_path, show_index=True)
+
+        # Request the root of the static directory.
+        r = yield from client.get('/my_dir')
+        assert r.status == 403
+
+    yield from r.release()
+
+
+@pytest.mark.run_loop
+def test_access_symlink_loop(tmp_dir_path, create_app_and_client):
+    """
+    Tests the access to a looped symlink, which could not be resolved.
+    """
+    my_dir_path = os.path.join(tmp_dir_path, 'my_symlink')
+    os.symlink(my_dir_path, my_dir_path)
+
+    app, client = yield from create_app_and_client()
+
+    # Register global static route:
+    app.router.add_static('/', tmp_dir_path, show_index=True)
+
+    # Request the root of the static directory.
+    r = yield from client.get('/my_symlink')
+    assert r.status == 404
+
+    yield from r.release()
+
+
+@pytest.mark.run_loop
+def test_access_special_resource(tmp_dir_path, create_app_and_client):
+    """
+    Tests the access to a resource that is neither a file nor a directory.
+    Checks that if a special resource is accessed (f.e. named pipe or UNIX
+    domain socket) then 404 HTTP status returned.
+    """
+    app, client = yield from create_app_and_client()
+
+    with mock.patch('pathlib.Path.__new__') as path_constructor:
+        special = MagicMock()
+        special.is_dir.return_value = False
+        special.is_file.return_value = False
+
+        path = MagicMock()
+        path.joinpath.side_effect = lambda p: (special if p == 'special'
+                                               else path)
+        path.resolve.return_value = path
+        special.resolve.return_value = special
+
+        path_constructor.return_value = path
+
+        # Register global static route:
+        app.router.add_static('/', tmp_dir_path, show_index=True)
+
+        # Request the root of the static directory.
+        r = yield from client.get('/special')
+        assert r.status == 404
+
     yield from r.release()
 
 
