make request -> instance conversion support symlink dirs

I think this also clarifies the code, a bit

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):

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/')

tests/instance/pages/symlink-to-subdir

@@ -0,0 +1 @@
+subdir

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():