From 60715a3a5c5301ee44aaeedd151248171a002dd1 Mon Sep 17 00:00:00 2001 From: "Brian S. Stephan" Date: Sat, 17 Apr 2021 10:30:01 -0500 Subject: [PATCH] make request -> instance conversion support symlink dirs I think this also clarifies the code, a bit --- incorporealcms/pages.py | 60 +++++++++++++++----------- tests/functional_tests.py | 14 ++++++ tests/instance/pages/symlink-to-subdir | 1 + tests/test_pages.py | 7 +++ 4 files changed, 57 insertions(+), 25 deletions(-) create mode 120000 tests/instance/pages/symlink-to-subdir diff --git a/incorporealcms/pages.py b/incorporealcms/pages.py index 7b41627..7ea9dd2 100644 --- a/incorporealcms/pages.py +++ b/incorporealcms/pages.py @@ -34,7 +34,10 @@ def display_page(path): if render_type == 'file': return send_from_directory(app.instance_path, resolved_path) elif render_type == 'symlink': - return redirect(instance_resource_path_to_request_path(resolved_path), code=301) + logger.debug("attempting to redirect path '%s' to reverse of resource '%s'", path, resolved_path) + redirect_path = f'/{instance_resource_path_to_request_path(resolved_path)}' + logger.debug("redirect path: '%s'", redirect_path) + return redirect(redirect_path, code=301) elif render_type == 'markdown': try: with app.open_instance_resource(resolved_path, 'r') as entry_file: @@ -72,45 +75,52 @@ def request_path_to_instance_resource_path(path): """ # check if the path is allowed base_dir = os.path.realpath(f'{app.instance_path}/pages/') - resolved_path = os.path.realpath(os.path.join(base_dir, path)) - logger.debug("base_dir: %s, constructed resolved_path: %s", base_dir, resolved_path) + verbatim_path = os.path.abspath(os.path.join(base_dir, path)) + resolved_path = os.path.realpath(verbatim_path) + logger.debug("base_dir '%s', constructed resolved_path '%s' for path '%s'", base_dir, resolved_path, path) # bail if the requested real path isn't inside the base directory if base_dir != os.path.commonpath((base_dir, resolved_path)): logger.warning("client tried to request a path '%s' outside of the base_dir!", path) raise PermissionError - # if this is a (valid) symlink, find what it's pointed to and redirect the user - if os.path.islink(os.path.join(base_dir, path)): - logger.info("client requested a path '%s' that is actually a symlink to file '%s'", path, resolved_path) - return resolved_path.replace(f'{app.instance_path}{os.path.sep}', ''), 'symlink' - elif os.path.islink(os.path.join(base_dir, f'{path}.md')): - resolved_path = os.path.realpath(os.path.join(base_dir, f'{path}.md')) - logger.info("client requested a path '%s' that is actually a symlink to file '%s'", path, resolved_path) - return resolved_path.replace(f'{app.instance_path}{os.path.sep}', ''), 'symlink' + # see if we have a real file or if we should infer markdown rendering + if os.path.exists(resolved_path): + # if this is a file-like request but actually a directory, redirect the user + if os.path.isdir(resolved_path) and not path.endswith('/'): + logger.info("client requested a path '%s' that is actually a directory", path) + raise IsADirectoryError - # if this is a file-like requset but actually a directory, redirect the user - if os.path.isdir(resolved_path) and not path.endswith('/'): - logger.info("client requested a path '%s' that is actually a directory", path) - raise IsADirectoryError + # if the requested path contains a symlink, redirect the user + if verbatim_path != resolved_path: + logger.info("client requested a path '%s' that is actually a symlink to file '%s'", path, resolved_path) + return resolved_path.replace(f'{app.instance_path}{os.path.sep}', ''), 'symlink' + + # derive the proper markdown or actual file depending on if this is a dir or file + if os.path.isdir(resolved_path): + resolved_path = os.path.join(resolved_path, 'index.md') + return resolved_path.replace(f'{app.instance_path}{os.path.sep}', ''), 'markdown' - # derive the proper markdown or actual file depending on if this is a dir or file - if os.path.isdir(resolved_path): - absolute_resource = os.path.join(resolved_path, 'index.md') - elif os.path.exists(resolved_path): logger.info("final DIRECT path = '%s' for request '%s'", resolved_path, path) return resolved_path.replace(f'{app.instance_path}{os.path.sep}', ''), 'file' - else: - absolute_resource = f'{resolved_path}.md' + + # if we're here, this isn't direct file access, so try markdown inference + verbatim_path = os.path.abspath(os.path.join(base_dir, f'{path}.md')) + resolved_path = os.path.realpath(verbatim_path) # does the final file actually exist? - if not os.path.exists(absolute_resource): - logger.warning("requested final path '%s' does not exist!", absolute_resource) + if not os.path.exists(resolved_path): + logger.warning("requested final path '%s' does not exist!", resolved_path) raise FileNotFoundError - logger.info("final path = '%s' for request '%s'", absolute_resource, path) + # check for symlinks + if verbatim_path != resolved_path: + logger.info("client requested a path '%s' that is actually a symlink to file '%s'", path, resolved_path) + return resolved_path.replace(f'{app.instance_path}{os.path.sep}', ''), 'symlink' + + logger.info("final path = '%s' for request '%s'", resolved_path, path) # we checked that the file exists via absolute path, but now we need to give the path relative to instance dir - return absolute_resource.replace(f'{app.instance_path}{os.path.sep}', ''), 'markdown' + return resolved_path.replace(f'{app.instance_path}{os.path.sep}', ''), 'markdown' def instance_resource_path_to_request_path(path): diff --git a/tests/functional_tests.py b/tests/functional_tests.py index e76fdb3..a47916f 100644 --- a/tests/functional_tests.py +++ b/tests/functional_tests.py @@ -125,6 +125,20 @@ def test_that_request_to_symlink_redirects_file(client): assert response.location == 'http://localhost/foo.txt' +def test_that_request_to_symlink_redirects_directory(client): + """Test that a request to /foo/ redirects to /what-foo-points-at/.""" + response = client.get('/symlink-to-subdir/') + assert response.status_code == 301 + assert response.location == 'http://localhost/subdir' + # sadly, this location also redirects + response = client.get('/subdir') + assert response.status_code == 301 + assert response.location == 'http://localhost/subdir/' + # but we do get there + response = client.get('/subdir/') + assert response.status_code == 200 + + def test_that_dir_request_does_not_redirect(client): """Test that a request to /foo/ serves the index page, if foo is a directory.""" response = client.get('/subdir/') diff --git a/tests/instance/pages/symlink-to-subdir b/tests/instance/pages/symlink-to-subdir new file mode 120000 index 0000000..8bbe8a5 --- /dev/null +++ b/tests/instance/pages/symlink-to-subdir @@ -0,0 +1 @@ +subdir \ No newline at end of file diff --git a/tests/test_pages.py b/tests/test_pages.py index c7c0172..a1d6f55 100644 --- a/tests/test_pages.py +++ b/tests/test_pages.py @@ -146,6 +146,13 @@ def test_request_path_to_instance_resource_path_file_symlink(app): ('pages/foo.txt', 'symlink')) +def test_request_path_to_instance_resource_path_dir_symlink(app): + """Test that a request for e.g. '/foo' when /foo is a symlink to /bar redirects.""" + with app.test_request_context(): + assert (request_path_to_instance_resource_path('symlink-to-subdir/') == + ('pages/subdir', 'symlink')) + + def test_request_path_to_instance_resource_path_nonexistant_file_errors(app): """Test that a request for something not on disk errors.""" with app.test_request_context():