Fix crash rewriting relative document URI on ..// in querystring

CHANGELOG.md

@@ -7,6 +7,10 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 
 ## [Unreleased]
 
+### Fixed
+
+- Fix crash in `ArticleUrlRewriter.get_document_uri` when the document or a linked item has a `..` (or `/`) segment in its querystring ; querystrings are part of the ZIM entry name and must not be interpreted as navigable path segments (openzim/warc2zim#380)
+
 ## [5.4.0] - 2026-05-28
 
 ### Added

src/zimscraperlib/rewriting/url_rewriting.py

@@ -311,25 +311,33 @@ def get_document_uri(self, item_path: ZimPath, item_fragment: str) -> str:
 
         """
         item_parts = urlsplit(item_path.value)
-
-        # item_path is both path + querystring, both will be url-encoded in the document
-        # so that readers consider them as a whole and properly pass them to libzim
-        item_url = item_parts.path
-        if item_parts.query:
-            item_url += "?" + item_parts.query
+        article_parts = urlsplit(self.article_path.value)
+
+        # The relative path is computed using only the path components. A querystring is
+        # part of the ZIM entry name (a leaf), it is not a navigable directory, so its
+        # content (which may contain `/` or even `..` segments) must not be interpreted
+        # as path navigation when computing the relative path ; doing so crashes on `..`
+        # segments (see https://github.com/openzim/warc2zim/issues/380). The querystring
+        # is re-appended and url-encoded together with the path below so that readers
+        # consider path + querystring as a single whole entry.
         relative_path = str(
-            PurePosixPath(item_url).relative_to(
+            PurePosixPath(item_parts.path).relative_to(
                 (
-                    PurePosixPath(self.article_path.value)
-                    if self.article_path.value.endswith("/")
-                    else PurePosixPath(self.article_path.value).parent
+                    PurePosixPath(article_parts.path)
+                    if article_parts.path.endswith("/")
+                    else PurePosixPath(article_parts.path).parent
                 ),
                 walk_up=True,
             )
         )
-        # relative_to removes a potential last '/' in the path, we add it back
-        if item_path.value.endswith("/"):
+        # relative_to removes a potential last '/' in the path, we add it back before
+        # appending the querystring
+        if item_parts.path.endswith("/"):
             relative_path += "/"
+        # re-append the querystring (part of the ZIM entry name) now that the relative
+        # path has been computed
+        if item_parts.query:
+            relative_path += "?" + item_parts.query
 
         return (
             f"{quote(relative_path, safe='/')}"

tests/rewriting/test_url_rewriting.py

@@ -532,6 +532,46 @@ def test_relative_url(
             == expected_rewrite_result
         )
 
+    @pytest.mark.parametrize(
+        "article_url, item_url, expected_document_uri",
+        [
+            # `..` in the querystring of the document being rewritten must not be
+            # interpreted as a navigable path segment (see
+            # https://github.com/openzim/warc2zim/issues/380 ; it used to raise
+            # `ValueError: '..' segment ... cannot be walked`)
+            pytest.param(
+                "http://kiwix.org/common/sub/page.html?css=../../prg",
+                "http://kiwix.org/_zim_static/wombat.js",
+                "../../_zim_static/wombat.js",
+                id="dotdot_in_article_querystring",
+            ),
+            # `..` in the querystring of the linked item is kept as-is in the (encoded)
+            # entry name and must not trigger path navigation either
+            pytest.param(
+                "http://kiwix.org/common/page.html",
+                "http://kiwix.org/common/other.html?css=../../prg",
+                "other.html%3Fcss%3D../../prg",
+                id="dotdot_in_item_querystring",
+            ),
+            # a `/` inside the querystring is part of the entry name, not a directory
+            pytest.param(
+                "http://kiwix.org/a/b/page.html",
+                "http://kiwix.org/a/b/img.png?x=1/2",
+                "img.png%3Fx%3D1/2",
+                id="slash_in_item_querystring",
+            ),
+        ],
+    )
+    def test_get_document_uri_querystring_segments(
+        self,
+        article_url: str,
+        item_url: str,
+        expected_document_uri: str,
+    ):
+        rewriter = ArticleUrlRewriter(article_url=HttpUrl(article_url))
+        item_path = ArticleUrlRewriter.normalize(HttpUrl(item_url))
+        assert rewriter.get_document_uri(item_path, "") == expected_document_uri
+
     @pytest.mark.parametrize(
         "article_url, original_content_url, expected_rewrite_result, know_paths, "
         "rewrite_all_url",