From 90ec4ebf07a9f215b89dbb8b95952d552e17461a Mon Sep 17 00:00:00 2001 From: tejksat Date: Mon, 22 Aug 2016 18:01:12 +0300 Subject: [PATCH] #921 show directory index (from PR #1033) (#1094) * 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 --- aiohttp/web_urldispatcher.py | 71 ++++++++++++++--- docs/web.rst | 6 ++ docs/web_reference.rst | 7 +- tests/test_web_urldispatcher.py | 131 ++++++++++++++++++++++++++++++-- 4 files changed, 199 insertions(+), 16 deletions(-) diff --git a/aiohttp/web_urldispatcher.py b/aiohttp/web_urldispatcher.py index 624093294ed..ff7dea04250 100644 --- 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 = "\n{}\n".format(index_of) + 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( + '
  • {name}
    • '.format(url=file_url, + name=file_name) + ) + ul = "".format('\n'.join(index_list)) + body = "\n{}\n{}\n".format(h1, ul) + + html = "\n{}\n{}\n".format(head, body) + + return html + def __repr__(self): name = "'" + self.name + "' " if self.name is not None else "" return " {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 index 5fa867dbbfc..f448bcc949d 100644 --- 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 index f27a3d24a98..a236023e739 100644 --- 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 index e8aeef25a66..def981d794f 100644 --- 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'\n\nIndex of /\n' + b'\n\n

    Index of /

    \n\n\n')]) @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()