Repository: encukou/freezeyt (language: python)

Issue:
Handle extra_files in config
Handle chars below in `url_part`

- [x] `:`
- [x] `.`
- [x] `..`
- [x] `https:` resp. `http:`
- [x] quoted comment below


Discussion / hints:

Fix patch:
diff --git a/freezeyt/extra_files.py b/freezeyt/extra_files.py
index f2122adb..f1b05b75 100644
--- a/freezeyt/extra_files.py
+++ b/freezeyt/extra_files.py
@@ -4,6 +4,9 @@
 from typing import Mapping, Iterator, Tuple, Union
 import sys
 
+from freezeyt.util import get_url_part
+
+
 if sys.version_info > (3, 8):
     from typing import Literal
     literal_content = Literal["content"]
@@ -31,11 +34,8 @@ def get_extra_files(
     """
     extra_files_config = config.get('extra_files')
     if extra_files_config is not None:
-        for url_part, content in extra_files_config.items():
-            backslash = "\\"
-            url_part = url_part.replace(backslash, "/")
-            while "//" in url_part:
-                url_part = url_part.replace("//", "/")
+        for text, content in extra_files_config.items():
+            url_part = get_url_part(text)
             if isinstance(content, str):
                 yield url_part, "content", content.encode()
             elif isinstance(content, bytes):
diff --git a/freezeyt/freezer.py b/freezeyt/freezer.py
index 2505e2e2..3c9d0c0e 100644
--- a/freezeyt/freezer.py
+++ b/freezeyt/freezer.py
@@ -409,6 +409,10 @@ async def prepare(self):
         self.add_task(self.prefix, reason='site root (homepage)')
         for url_part, kind, content_or_path in get_extra_files(self.config):
             if kind == 'content':
+                # join part with path, otherwise filename 'http:' overwrite prefix
+                assert self.prefix.path.endswith('/')
+                assert not url_part.startswith('/')
+                url_part = self.prefix.path + url_part
                 self.add_task(
                     self.prefix.join(url_part),
                     reason="from extra_files",
@@ -417,6 +421,10 @@ async def prepare(self):
                 for part in get_url_parts_from_directory(
                     url_part, content_or_path
                 ):
+                # join part with path, otherwise filename 'http:' overwrite prefix
+                    assert self.prefix.path.endswith('/')
+                    assert not url_part.startswith('/')
+                    part = self.prefix.path + part
                     self.add_task(
                         self.prefix.join(part),
                         reason="from extra_files",
diff --git a/freezeyt/util.py b/freezeyt/util.py
index 16bc0ceb..355e28ba 100644
--- a/freezeyt/util.py
+++ b/freezeyt/util.py
@@ -4,6 +4,7 @@
 from werkzeug.urls import url_parse
 
 from freezeyt.compat import _MultiErrorBase, HAVE_EXCEPTION_GROUP
+from freezeyt.encoding import decode_input_path
 
 
 process_pool_executor = concurrent.futures.ProcessPoolExecutor()
@@ -167,3 +168,34 @@ def import_variable_from_module(
         result = getattr(result, attribute_name)
 
     return result
+
+
+def get_url_part(part: str) -> str:
+    """Get normalized url_part from string.
+
+    Normalizing rules are:
+        - decode to unicode
+        - any backslash replace by forward slash, except encoded backslash
+        - simple dot as filesystem hardlink is removed
+        - filesystem hardlink '..' is not allowed
+        - multiple slashes reduce to only one
+        - relative path does not start with slash
+    """
+
+    backslash = "\\"
+    part = part.replace(backslash, "/")
+
+    part = decode_input_path(part)
+
+    items = ["" if p == "." else p for p in part.split("/")]
+
+    if ".." in items:
+        raise ValueError("'..' component not allowed in URL part")
+
+    part = "/".join(items)
+
+    while "//" in part:
+        part = part.replace("//", "/")
+
+    return part.lstrip("/")
+


Test patch:
diff --git a/tests/test_extra_files.py b/tests/test_extra_files.py
index 6a8fa4fa..d3ce7381 100644
--- a/tests/test_extra_files.py
+++ b/tests/test_extra_files.py
@@ -17,7 +17,7 @@ def test_simple():
     assert list(get_extra_files(config)) == [('foo', 'content', b'abc')]
 
 
-EXTRA_FILES = {
+EXTRA_FILE_SLASH = {
     'url_part': {'url_part': b'a'},
     '/url_part': {'url_part': b'a'},
     'url_part/': {'url_part': {'index.html': b'a'}},
@@ -30,10 +30,10 @@ def test_simple():
     '/http/https/': {'http': {'https': {'index.html': b'a'}}},
     r'/http\https/': {'http': {'https': {'index.html': b'a'}}},
 }
-@pytest.mark.parametrize('test_case', EXTRA_FILES)
+@pytest.mark.parametrize('test_case', EXTRA_FILE_SLASH)
 def test_slashes(test_case):
     extra_file = {test_case: 'a'}
-    expected = EXTRA_FILES[test_case]
+    expected = EXTRA_FILE_SLASH[test_case]
     config = {
         'extra_files': extra_file,
         'output': {'type': 'dict'},
@@ -48,6 +48,147 @@ def test_slashes(test_case):
 
     assert result == expected
 
+EXTRA_FILE_DOT = {
+    ':': {':': b'a'},
+    ':/': {':': {'index.html': b'a'}},
+    '.nojekyll': {'.nojekyll': b'a'},
+    '/.nojekyll': {'.nojekyll': b'a'},
+    '/info.txt': {'info.txt': b'a'},
+    '/./a/./a.b.': {'a': {'a.b.': b'a'}},
+    '/./a/./a.b./.': {'a': {'a.b.': {'index.html': b'a'}}},
+    './a/./a.b./.': {'a': {'a.b.': {'index.html': b'a'}}},
+    './a/./a.b././': {'a': {'a.b.': {'index.html': b'a'}}},
+    '/./././a.b./.': {'a.b.': {'index.html': b'a'}},
+    'a..b/a../b..': {'a..b': {'a..': {'b..': b'a'}}},
+    'a..b/a../..b/': {'a..b': {'a..': {'..b': {'index.html': b'a'}}}},
+    '/..b../': {'..b..': {'index.html': b'a'}},
+    '/..b..': {'..b..': b'a'},
+    # http: without leading forward slash become absolute url instead of url part
+    'http:': {'http:': b'a'},
+    '/http:': {'http:': b'a'},
+    '/http:/': {'http:': {'index.html': b'a'}},
+    '/https:/': {'https:': {'index.html': b'a'}},
+    '/https://': {'https:': {'index.html': b'a'}},
+    '/http:/https:/': {'http:': {'https:': {'index.html': b'a'}}},
+    '/http:/foo/': {'http:': {'foo': {'index.html': b'a'}}},
+}
+@pytest.mark.parametrize('test_case', EXTRA_FILE_DOT)
+def test_dots(test_case):
+    extra_file = {test_case: 'a'}
+    expected = EXTRA_FILE_DOT[test_case]
+    config = {
+        'extra_files': extra_file,
+        'output': {'type': 'dict'},
+    }
+
+    with context_for_test('app_simple') as module:
+        result = freeze(module.app, config)
+
+    # pop to simplify syntax of expected dict
+    # index.html is root page for app_simple, not useful for this test
+    result.pop('index.html')
+
+    assert result == expected
+
+
+EXTRA_FILE_QUOTED = {
+    '%C3%A1/':  {'á': {'index.html': b'a'}},
+    'á/':       {'á': {'index.html': b'a'}},
+    '/a%5cb/':  {'a\\b': {'index.html': b'a'}},
+}
+@pytest.mark.parametrize('test_case', EXTRA_FILE_QUOTED)
+def test_quoted_url_path(test_case):
+    extra_file = {test_case: 'a'}
+    expected = EXTRA_FILE_QUOTED[test_case]
+    config = {
+        'extra_files': extra_file,
+        'output': {'type': 'dict'},
+    }
+
+    with context_for_test('app_simple') as module:
+        result = freeze(module.app, config)
+
+    # pop to simplify syntax of expected dict
+    # index.html is root page for app_simple, not useful for this test
+    result.pop('index.html')
+
+    assert result == expected
+
+EXTRA_FILE_WITH_PREFIX = {
+    # url_part - from configuration: expected url_part - after clean
+    ':':                ':',
+    ':/':               ':/',
+    '.nojekyll':        '.nojekyll',
+    '/.nojekyll':       '.nojekyll',
+    'http:/info.txt':   'http:/info.txt',
+    '/./a/./a.b.':      'a/a.b.',
+    '/./a/./a.b./.':    'a/a.b./',
+    './a/./a.b./.':     'a/a.b./',
+    './a/./a.b././':    'a/a.b./',
+    '/./././a.b./.':    'a.b./',
+    'a..b/a../b..':     'a..b/a../b..',
+    'a..b/a../..b/':    'a..b/a../..b/',
+    '/..b../':          '..b../',
+    '/..b..':           '..b..',
+    'http':             'http',
+    '/a%5cb/':          'a\\b/',
+    '%C3%A1/':          'á/',
+    'á/':               'á/',
+    # http: without leading forward slash alter prefix by werkzeug.url_parse(<prefix>).join("http:")
+    # http: become the protocol of prefix instead url path
+    'http:':            'http:',
+    '/http:':           'http:',
+    '/http:/':          'http:/',
+    '/https:/':         'https:/',
+    '/https://':        'https:/',
+    '/http:/https/':    'http:/https/',
+    r'/http\https/':    'http/https/',
+}
+@pytest.mark.parametrize('url_part', EXTRA_FILE_WITH_PREFIX)
+@pytest.mark.parametrize('prefix', ('https://example.com:443/freezeyt/', 'https://example.com:443/'))
+def test_join_with_prefix(url_part, prefix):
+    recorded_tasks = {}
+
+    def record_page(task_info):
+        recorded_tasks[task_info.get_a_url()] = task_info
+
+    extra_file = {url_part: 'a'}
+
+    config = {
+        'prefix': prefix,
+        'extra_files': extra_file,
+        'output': {'type': 'dict'},
+        'hooks': {'page_frozen': [record_page]},
+    }
+
+    with context_for_test('app_simple') as module:
+        freeze(module.app, config)
+
+    expected = [
+        config['prefix'],
+        config['prefix'] + EXTRA_FILE_WITH_PREFIX[url_part],
+    ]
+
+    assert sorted(recorded_tasks) == expected
+
+
+EXTRA_FILE_INVALID = (
+    '../a/b.txt',
+    '/a/../b.txt',
+    '/a/b/..',
+)
+@pytest.mark.parametrize('url_path', EXTRA_FILE_INVALID)
+def test_invalid_url_part(url_path):
+    extra_file = {url_path: 'a'}
+    config = {
+        'extra_files': extra_file,
+        'output': {'type': 'dict'},
+    }
+
+    with context_for_test('app_simple') as module:
+        with pytest.raises(ValueError):
+            freeze(module.app, config)
+
 
 def test_content():
     config = {
diff --git a/tests/test_hooks.py b/tests/test_hooks.py
index 7e3da0b4..c4884c29 100644
--- a/tests/test_hooks.py
+++ b/tests/test_hooks.py
@@ -167,7 +167,7 @@ def add_output_to_output_paths(dict_to_add, path_so_far):
 
     assert recorded_tasks == output_paths
 
-    
+
 def test_taskinfo_has_freezeinfo():
     freezeinfos = []