diff --git a/CHANGELOG.md b/CHANGELOG.md index 8a7c209..e3036b4 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -31,6 +31,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - Do not collapse consecutive slashes `//` into a single `/` when normalizing URLs into ZIM paths, in both Python and JS rewriting (#340) - Resolve relative links against a page URL containing consecutive slashes `//` the way browsers do, instead of collapsing them like `urllib.parse.urljoin` does (#346) - `Creator.finish()` no longer silently discards errors raised by libzim while finalizing the ZIM, so a failure during finalization is no longer reported as a successful run (openzim/warc2zim#462) +- Escape `.` and `..` path segments (e.g. folded into the path from a querystring) with a leading `_` when normalizing URLs into ZIM paths, so they are kept as literal entry names instead of being resolved away as navigation, in both Python and JS rewriting (#321) ## [5.4.1] - 2026-07-31 diff --git a/javascript/src/wombatSetup.js b/javascript/src/wombatSetup.js index 7c48eae..c3266d5 100644 --- a/javascript/src/wombatSetup.js +++ b/javascript/src/wombatSetup.js @@ -18,6 +18,20 @@ export function applyFuzzyRules(path) { return path; } +export function escapeDotSegments(path) { + // Escape '.' and '..' path segments so they are not resolved as navigation. + // Per RFC 3986, a '.' or '..' path segment is a dot-segment which any URI consumer + // removes/resolves while normalizing a reference. A ZIM path must keep such a + // segment as a literal entry name instead (e.g. one folded in from a querystring), + // so it is prefixed with a '_', which is not itself special in a ZIM path. + // Segments which only look like a dot-segment (e.g. 'foo..', '..foo', '...') are + // not dot-segments and are left untouched. + return path + .split('/') + .map((part) => (part === '.' || part === '..' ? '_' + part : part)) + .join('/'); +} + export function hasAlreadyBeenRewritten( original_absolute_url, orig_url, @@ -186,10 +200,14 @@ export function urlRewriteFunction( // apply the fuzzy rules to the ZIM path const fuzzifiedPath = applyFuzzyRules(zimPath); + // escape '.' and '..' path segments so they are not resolved as navigation by the + // reader once this ZIM path is turned into a URI + const escapedPath = escapeDotSegments(fuzzifiedPath); + // Reencode everything but '/' (we decode it afterwards for simplicity) const finalUrl = prefix + - encodeURIComponent(fuzzifiedPath) + encodeURIComponent(escapedPath) .replaceAll('%2F', '/') .replaceAll('%3D', '=') .replaceAll('%2C', ','); @@ -310,6 +328,7 @@ export function getWombatInfo( export default { applyFuzzyRules: applyFuzzyRules, + escapeDotSegments: escapeDotSegments, urlRewriteFunction: urlRewriteFunction, getWombatInfo: getWombatInfo, }; diff --git a/javascript/test/wombatSetup.js b/javascript/test/wombatSetup.js index 3fc889d..376c8fb 100644 --- a/javascript/test/wombatSetup.js +++ b/javascript/test/wombatSetup.js @@ -8,6 +8,29 @@ test.beforeEach((t) => { t.context.originalScheme = 'https'; }); +test('escapeDotSegmentsDot', (t) => { + t.is(utils.escapeDotSegments('a/./b'), 'a/_./b'); +}); + +test('escapeDotSegmentsDotDot', (t) => { + t.is(utils.escapeDotSegments('a/../b'), 'a/_../b'); +}); + +test('escapeDotSegmentsTrailingDotDot', (t) => { + t.is(utils.escapeDotSegments('a/..'), 'a/_..'); +}); + +test('escapeDotSegmentsConsecutive', (t) => { + t.is(utils.escapeDotSegments('a/../../b'), 'a/_../_../b'); +}); + +test('escapeDotSegmentsLookAlikesUntouched', (t) => { + t.is( + utils.escapeDotSegments('a/.foo/foo../..foo/.../b'), + 'a/.foo/foo../..foo/.../b', + ); +}); + test('nominalWbInfo', (t) => { const path = 'path1/resource1.js'; const originalUrl = diff --git a/javascript/test/wombatUrlRewriting.js b/javascript/test/wombatUrlRewriting.js index 1cea943..f9db7a9 100644 --- a/javascript/test/wombatUrlRewriting.js +++ b/javascript/test/wombatUrlRewriting.js @@ -498,6 +498,25 @@ test('contentWithSimpleQueryString', (t) => { ); }); +test('contentWithQueryStringDotDotSegment', (t) => { + // a `..` dot-segment folded into the path from a querystring must be escaped so + // it is not resolved as navigation once this ZIM path is turned into a URI + t.is( + urlRewriteFunction( + t.context.currentUrl, + t.context.originalHost, + t.context.originalScheme, + t.context.originalUrl, + t.context.prefix, + 'https://www.example.com/javascript/content.txt?css=../../prg', + undefined, + undefined, + undefined, + ), + 'http://library.kiwix.org/content/myzim_yyyy-mm/www.example.com/javascript/content.txt%3Fcss=../_../prg', + ); +}); + test('contentWithSimplePathWithEqualAndComma', (t) => { t.is( urlRewriteFunction( diff --git a/src/zimscraperlib/rewriting/url_rewriting.py b/src/zimscraperlib/rewriting/url_rewriting.py index 47540bd..cf1bfb3 100644 --- a/src/zimscraperlib/rewriting/url_rewriting.py +++ b/src/zimscraperlib/rewriting/url_rewriting.py @@ -23,6 +23,16 @@ . This is valid : "foo/part/file?question=Is there any + here?" . This is NOT valid : "foo/part/file?question%3DIs%20there%20any%20%2B%20here%3F" +A ZIM path must never contain a `.` or `..` path segment: per RFC 3986 these +"dot-segments" are resolved away by any URI consumer (including the reader turning a +relative link back into an absolute one), so a literal `.` or `..` coming from the +original URL (typically folded into the path from a querystring, see below) would +either disappear or be misinterpreted as a navigation instruction instead of being +kept as a literal entry name. Such segments are therefore escaped by prefixing them +with a `_`, e.g. path `a/../b` becomes `a/_../b`. This only applies to segments that +are exactly `.` or `..`; look-alikes such as `a/foo../b` or `a/..foo/b` are left +untouched since they are not dot-segments. + On top of that, fuzzy rules are applied on the ZIM path: For instance a path "https://www.youtube.com/youtubei/v1/foo/baz/things?key=value &other_key=other_value&videoId=xxxx&yet_another_key=yet_another_value" @@ -437,6 +447,22 @@ def _relative_path(cls, path: str, start: str) -> str: relative_path = "./" + relative_path return relative_path + @classmethod + def escape_dot_segments(cls, path: str) -> str: + """Escape '.' and '..' path segments so they are not resolved as navigation + + Per RFC 3986, a `.` or `..` path segment is a dot-segment which any URI + consumer removes/resolves while normalizing a reference. A ZIM path must + keep such a segment as a literal entry name instead, so it is prefixed with + a `_`, which is not itself special in a ZIM path. + + Segments which only look like a dot-segment (e.g. `foo..`, `..foo`, `...`) + are not dot-segments and are left untouched. + """ + return "/".join( + f"_{part}" if part in (".", "..") else part for part in path.split("/") + ) + @classmethod def apply_additional_rules(cls, uri: HttpUrl | str) -> str: """Apply additional rules on a URL or relative path @@ -532,4 +558,4 @@ def normalize(cls, url: HttpUrl) -> ZimPath: f"{hostname}{path}{query}" ) - return ZimPath(fuzzified_url) + return ZimPath(ArticleUrlRewriter.escape_dot_segments(fuzzified_url)) diff --git a/tests/rewriting/test_url_rewriting.py b/tests/rewriting/test_url_rewriting.py index 5ff4b77..7d2197d 100644 --- a/tests/rewriting/test_url_rewriting.py +++ b/tests/rewriting/test_url_rewriting.py @@ -113,6 +113,22 @@ class TestNormalize: "Solidarité-Numérique_1@300x.png" ), ), + # `.` and `..` dot-segments are escaped with a leading `_`, wherever + # they appear (including a querystring folded into the path), since they + # would otherwise be resolved away as RFC 3986 navigation instructions + ( + "https://example.com/abc/de?f=12/../34", + "example.com/abc/de?f=12/_../34", + ), + ("https://example.com/a/./b", "example.com/a/_./b"), + ("https://example.com/a/..", "example.com/a/_.."), + ("https://example.com/a/.", "example.com/a/_."), + ("https://example.com/a/../../b", "example.com/a/_../_../b"), + # look-alikes are not dot-segments and must be left untouched + ("https://example.com/.foo", "example.com/.foo"), + ("https://example.com/foo../bar", "example.com/foo../bar"), + ("https://example.com/..foo/bar", "example.com/..foo/bar"), + ("https://example.com/...", "example.com/..."), ], ) def test_normalize(self, url: str, zim_path: str): @@ -121,6 +137,28 @@ def test_normalize(self, url: str, zim_path: str): ) +class TestEscapeDotSegments: + @pytest.mark.parametrize( + "path,expected", + [ + ("a/../b", "a/_../b"), + ("a/./b", "a/_./b"), + ("a/..", "a/_.."), + ("a/.", "a/_."), + ("a/../../b", "a/_../_../b"), + (".", "_."), + ("..", "_.."), + ("", ""), + ("a/.foo/b", "a/.foo/b"), + ("a/foo../b", "a/foo../b"), + ("a/..foo/b", "a/..foo/b"), + ("a/.../b", "a/.../b"), + ], + ) + def test_escape_dot_segments(self, path: str, expected: str): + assert ArticleUrlRewriter.escape_dot_segments(path) == expected + + _empty_zimpath_set: set[ZimPath] = set() @@ -719,6 +757,18 @@ def test_relative_url_query_only_reference(self): None, ), ("https://example.com/x//y/page.html", "https://example.com/a.html", None), + # a `..` dot-segment folded into the path (e.g. from a querystring) must + # not be resolved as navigation when the link is followed back + ( + "https://kiwix.org/common/sub/page.html", + "https://kiwix.org/common/sub/xtree.html?css=../../prg", + "xtree.html%3Fcss=../_../prg", + ), + ( + "https://example.com/index.html", + "https://example.com/a/../b", + "a/_../b", + ), ], ) def test_relative_url_round_trip(