From d5bd7fb1a63e3ca02688f359c823902e8251419b Mon Sep 17 00:00:00 2001
From: benoit74 <benoit74@users.noreply.github.com>
Date: Tue, 20 Oct 2026 15:26:41 +0000
Subject: [PATCH 1/3] Resolve relative links against page URLs with consecutive
 slashes

urllib.parse.urljoin collapses empty path segments while merging a
relative reference onto the base URL, so a page whose own URL contains
`//` had its relative links resolved to the wrong absolute URL (slashes
collapsed before normalize() even saw them). Replace it with a custom
join that follows RFC 3986 section 5.3 without that deviation, matching
how browsers resolve such references.

The JS (wombat) side already resolves relative URLs correctly via
uri-js's URI.resolve, so only the Python rewriter needed a fix; added a
regression test on the JS side to document that.

Fix #346
---
 CHANGELOG.md                                 |  1 +
 javascript/test/wombatUrlRewriting.js        | 23 ++++++
 src/zimscraperlib/rewriting/url_rewriting.py | 84 ++++++++++++++++++--
 tests/rewriting/test_url_rewriting.py        | 29 +++++++
 4 files changed, 132 insertions(+), 5 deletions(-)

diff --git a/CHANGELOG.md b/CHANGELOG.md
index 0787983..8a7c209 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -29,6 +29,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 - Find the language of an HTML page in a `content-language` `<meta>` that is not the first `<meta>` of the page (#356)
 - Rewrite the `poster` attribute (e.g. of `<video>`) when rewriting HTML, like `src` and `href` (#339)
 - 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)
 
 ## [5.4.1] - 2026-07-31
diff --git a/javascript/test/wombatUrlRewriting.js b/javascript/test/wombatUrlRewriting.js
index 64b397b..1cea943 100644
--- a/javascript/test/wombatUrlRewriting.js
+++ b/javascript/test/wombatUrlRewriting.js
@@ -101,6 +101,29 @@ test('consecutiveSlashesAreKept', (t) => {
   );
 });
 
+test('relativeLinkOnPageWithConsecutiveSlashesKeepsThem', (t) => {
+  // #346: a relative link must be resolved against the page's own URL the way a
+  // browser would, keeping consecutive slashes in that URL instead of collapsing
+  // them (uri-js's URI.resolve, unlike Python's urllib.parse.urljoin, already does
+  // this correctly)
+  const originalUrl = 'https://www.example.com/path1//resource1.html';
+  const currentUrl = t.context.prefix + 'www.example.com/path1//resource1.html';
+  t.is(
+    urlRewriteFunction(
+      currentUrl,
+      t.context.originalHost,
+      t.context.originalScheme,
+      originalUrl,
+      t.context.prefix,
+      'other.html',
+      undefined,
+      undefined,
+      undefined,
+    ),
+    t.context.prefix + 'www.example.com/path1//other.html',
+  );
+});
+
 test('simpleContentEmptyPath', (t) => {
   t.is(
     urlRewriteFunction(
diff --git a/src/zimscraperlib/rewriting/url_rewriting.py b/src/zimscraperlib/rewriting/url_rewriting.py
index 426f046..105ba74 100644
--- a/src/zimscraperlib/rewriting/url_rewriting.py
+++ b/src/zimscraperlib/rewriting/url_rewriting.py
@@ -43,7 +43,14 @@
 import re
 from dataclasses import dataclass
 from typing import ClassVar
-from urllib.parse import quote, unquote, urljoin, urlsplit
+from urllib.parse import (
+    quote,
+    unquote,
+    urlsplit,
+    urlunsplit,
+    uses_netloc,
+    uses_relative,
+)
 
 import idna
 
@@ -63,6 +70,73 @@ class AdditionalRule:
 ]
 
 
+def _remove_dot_segments(path: str) -> str:
+    """Remove '.' and '..' segments from a path, per RFC 3986 section 5.2.4
+
+    Unlike a naive implementation based on splitting the path on '/', this keeps
+    empty segments (i.e. consecutive slashes), since those are part of the path.
+
+    `path` is expected to always start with a '/'.
+    """
+    output: list[str] = []
+    while path:
+        if path.startswith("../"):
+            path = path[3:]
+        elif path.startswith("./"):
+            path = path[2:]
+        elif path.startswith("/./") or path == "/.":
+            path = "/" + path[3:]
+        elif path.startswith("/../") or path == "/..":
+            path = "/" + path[4:]
+            if output:
+                output.pop()
+        elif path in (".", ".."):
+            path = ""
+        else:
+            end = path.find("/", 1)
+            segment, path = (path, "") if end == -1 else (path[:end], path[end:])
+            output.append(segment)
+    return "".join(output)
+
+
+def _urljoin(base: str, url: str | None) -> str:
+    """Join a base URL and a possibly relative URL, like `urllib.parse.urljoin`
+
+    Unlike `urllib.parse.urljoin`, empty path segments (i.e. consecutive slashes)
+    are not collapsed while merging a relative-path reference onto the base URL's
+    path, matching how browsers resolve such references (RFC 3986 section 5.3).
+    """
+    if not base:
+        return url or ""
+    if not url:
+        return base
+
+    bscheme, bnetloc, bpath, bquery, _bfragment = urlsplit(base)
+    scheme, netloc, path, query, fragment = urlsplit(url, bscheme)
+
+    if scheme != bscheme or scheme not in uses_relative:
+        return url
+
+    if scheme in uses_netloc:
+        if netloc:
+            return urlunsplit((scheme, netloc, path, query, fragment))
+        netloc = bnetloc
+
+    if not path:
+        path = bpath
+        if not query:
+            query = bquery
+        return urlunsplit((scheme, netloc, path, query, fragment))
+
+    merged_path = (
+        path if path.startswith("/") else bpath.rpartition("/")[0] + "/" + path
+    )
+
+    return urlunsplit(
+        (scheme, netloc, _remove_dot_segments(merged_path), query, fragment)
+    )
+
+
 class HttpUrl:
     """A utility class representing an HTTP url, usefull to pass this data around
 
@@ -193,8 +267,8 @@ def __init__(
     def get_item_path(self, item_url: str, base_href: str | None) -> ZimPath:
         """Utility to transform an item URL into a ZimPath"""
 
-        item_absolute_url = urljoin(
-            urljoin(self.article_url.value, base_href), item_url
+        item_absolute_url = _urljoin(
+            _urljoin(self.article_url.value, base_href), item_url
         )
         return ArticleUrlRewriter.normalize(HttpUrl(item_absolute_url))
 
@@ -213,8 +287,8 @@ def __call__(
         try:
             item_url = item_url.strip()
 
-            item_absolute_url = urljoin(
-                urljoin(self.article_url.value, base_href), item_url
+            item_absolute_url = _urljoin(
+                _urljoin(self.article_url.value, base_href), item_url
             )
 
             # Make case of standalone fragments more straightforward
diff --git a/tests/rewriting/test_url_rewriting.py b/tests/rewriting/test_url_rewriting.py
index b626157..78abe6d 100644
--- a/tests/rewriting/test_url_rewriting.py
+++ b/tests/rewriting/test_url_rewriting.py
@@ -648,6 +648,24 @@ def test_relative_url(
                     ZimPath("example.com/a.html"),
                 ),
             ),
+            (
+                "https://example.com/x//y/page.html",
+                "other.html",
+                RewriteResult(
+                    "https://example.com/x//y/other.html",
+                    "other.html",
+                    ZimPath("example.com/x//y/other.html"),
+                ),
+            ),
+            (
+                "https://example.com/x//y/page.html",
+                "../z.html",
+                RewriteResult(
+                    "https://example.com/x//z.html",
+                    "../z.html",
+                    ZimPath("example.com/x//z.html"),
+                ),
+            ),
         ],
     )
     def test_relative_url_keeps_consecutive_slashes(
@@ -1075,6 +1093,17 @@ def test_no_rewrite_other_schemes(
                 ),
                 id="base_on_absolute_url",
             ),
+            pytest.param(
+                "foo.html",
+                "kiwix.org/a/a//bar/foo.html",
+                "../a//bar/",
+                RewriteResult(
+                    "https://kiwix.org/a/a//bar/foo.html",
+                    "../a//bar/foo.html",
+                    ZimPath("kiwix.org/a/a//bar/foo.html"),
+                ),
+                id="base_with_consecutive_slashes",
+            ),
         ],
     )
     def test_base_href(

From f03c199d9d0bfad44b4a69a97c8633c2c652260f Mon Sep 17 00:00:00 2001
From: benoit74 <benoit74@users.noreply.github.com>
Date: Thu, 22 Oct 2026 07:30:22 +0000
Subject: [PATCH 2/3] Drop unreachable branches and add a test, per review

_remove_dot_segments and _urljoin had a few branches that can never be
hit given the merged path always starts with '/' and base is always a
non-empty absolute URL, so drop them instead of leaving dead code
around. Add a test for a query-only reference (e.g. "?page=2"), the
one gap in branch coverage that was actually reachable. Also have the
test helper _resolve_relative_reference reuse _remove_dot_segments
instead of duplicating it.
---
 src/zimscraperlib/rewriting/url_rewriting.py | 23 +++++--------
 tests/rewriting/test_url_rewriting.py        | 34 +++++++++-----------
 2 files changed, 24 insertions(+), 33 deletions(-)

diff --git a/src/zimscraperlib/rewriting/url_rewriting.py b/src/zimscraperlib/rewriting/url_rewriting.py
index 105ba74..e93a283 100644
--- a/src/zimscraperlib/rewriting/url_rewriting.py
+++ b/src/zimscraperlib/rewriting/url_rewriting.py
@@ -48,7 +48,6 @@
     unquote,
     urlsplit,
     urlunsplit,
-    uses_netloc,
     uses_relative,
 )
 
@@ -76,22 +75,17 @@ def _remove_dot_segments(path: str) -> str:
     Unlike a naive implementation based on splitting the path on '/', this keeps
     empty segments (i.e. consecutive slashes), since those are part of the path.
 
-    `path` is expected to always start with a '/'.
+    `path` must always start with a '/' (which the only caller, `_urljoin`,
+    guarantees).
     """
     output: list[str] = []
     while path:
-        if path.startswith("../"):
-            path = path[3:]
-        elif path.startswith("./"):
-            path = path[2:]
-        elif path.startswith("/./") or path == "/.":
+        if path.startswith("/./") or path == "/.":
             path = "/" + path[3:]
         elif path.startswith("/../") or path == "/..":
             path = "/" + path[4:]
             if output:
                 output.pop()
-        elif path in (".", ".."):
-            path = ""
         else:
             end = path.find("/", 1)
             segment, path = (path, "") if end == -1 else (path[:end], path[end:])
@@ -105,9 +99,9 @@ def _urljoin(base: str, url: str | None) -> str:
     Unlike `urllib.parse.urljoin`, empty path segments (i.e. consecutive slashes)
     are not collapsed while merging a relative-path reference onto the base URL's
     path, matching how browsers resolve such references (RFC 3986 section 5.3).
+
+    `base` must always be a non-empty absolute URL.
     """
-    if not base:
-        return url or ""
     if not url:
         return base
 
@@ -117,10 +111,9 @@ def _urljoin(base: str, url: str | None) -> str:
     if scheme != bscheme or scheme not in uses_relative:
         return url
 
-    if scheme in uses_netloc:
-        if netloc:
-            return urlunsplit((scheme, netloc, path, query, fragment))
-        netloc = bnetloc
+    if netloc:
+        return urlunsplit((scheme, netloc, path, query, fragment))
+    netloc = bnetloc
 
     if not path:
         path = bpath
diff --git a/tests/rewriting/test_url_rewriting.py b/tests/rewriting/test_url_rewriting.py
index 78abe6d..649d8cb 100644
--- a/tests/rewriting/test_url_rewriting.py
+++ b/tests/rewriting/test_url_rewriting.py
@@ -7,6 +7,7 @@
     HttpUrl,
     RewriteResult,
     ZimPath,
+    _remove_dot_segments,  # pyright: ignore[reportPrivateUsage]
 )
 
 
@@ -681,6 +682,19 @@ def test_relative_url_keeps_consecutive_slashes(
             == expected_rewrite_result
         )
 
+    def test_relative_url_query_only_reference(self):
+        """A query-only reference keeps the article's path but its own query"""
+        rewriter = ArticleUrlRewriter(
+            article_url=HttpUrl("https://example.com/index.html?old=1")
+        )
+        assert rewriter(
+            "?page=2", base_href=None, rewrite_all_url=True
+        ) == RewriteResult(
+            "https://example.com/index.html?page=2",
+            "index.html%3Fpage=2",
+            ZimPath("example.com/index.html?page=2"),
+        )
+
     @pytest.mark.parametrize(
         "article_url, original_content_url, expected_rewritten_url",
         [
@@ -1301,21 +1315,5 @@ def _resolve_relative_reference(base_url: str, reference: str) -> str:
     and path of the resolved URL, i.e. the form of a ZIM path.
     """
     base = urlsplit(base_url)
-    path = base.path.rpartition("/")[0] + "/" + reference.partition("#")[0]
-    output: list[str] = []
-    while path:
-        if path.startswith(("../", "./")):
-            path = path.partition("/")[2]
-        elif path.startswith("/./") or path == "/.":
-            path = "/" + path[3:]
-        elif path.startswith("/../") or path == "/..":
-            path = "/" + path[4:]
-            if output:
-                output.pop()
-        elif path in (".", ".."):
-            path = ""
-        else:
-            end = path.find("/", 1)
-            segment, path = (path, "") if end == -1 else (path[:end], path[end:])
-            output.append(segment)
-    return f"{base.netloc}{''.join(output)}"
+    merged_path = base.path.rpartition("/")[0] + "/" + reference.partition("#")[0]
+    return f"{base.netloc}{_remove_dot_segments(merged_path)}"

From a576f69c68a4a85d8d45d9936dcedb6d3c927992 Mon Sep 17 00:00:00 2001
From: benoit74 <benoit74@users.noreply.github.com>
Date: Thu, 22 Oct 2026 07:48:44 +0000
Subject: [PATCH 3/3] Guard internal invariants explicitly instead of with
 assert

Bare assert statements are stripped when Python runs with -O, so an
internal contract violation would silently misbehave instead of
failing loudly in that mode. Replace them with explicit
raise AssertionError, in url_rewriting.py's _urljoin/_remove_dot_segments
and in the pre-existing rx_replacer.py one, so the check always runs.
Add tests covering each.
---
 src/zimscraperlib/rewriting/rx_replacer.py   |  3 ++-
 src/zimscraperlib/rewriting/url_rewriting.py |  5 +++++
 tests/rewriting/test_rx_replacer.py          |  8 ++++++++
 tests/rewriting/test_url_rewriting.py        | 11 +++++++++++
 4 files changed, 26 insertions(+), 1 deletion(-)

diff --git a/src/zimscraperlib/rewriting/rx_replacer.py b/src/zimscraperlib/rewriting/rx_replacer.py
index adc0e82..0c7f187 100644
--- a/src/zimscraperlib/rewriting/rx_replacer.py
+++ b/src/zimscraperlib/rewriting/rx_replacer.py
@@ -153,7 +153,8 @@ def rewrite(
         if isinstance(text, bytes):
             text = text.decode()
 
-        assert self.compiled_rule is not None  # noqa
+        if self.compiled_rule is None:
+            raise AssertionError("compiled_rule must be set before calling rewrite()")
         return self.compiled_rule.sub(
             partial(self._replace_match, opts=opts, text=text), text
         )
diff --git a/src/zimscraperlib/rewriting/url_rewriting.py b/src/zimscraperlib/rewriting/url_rewriting.py
index e93a283..47540bd 100644
--- a/src/zimscraperlib/rewriting/url_rewriting.py
+++ b/src/zimscraperlib/rewriting/url_rewriting.py
@@ -78,6 +78,9 @@ def _remove_dot_segments(path: str) -> str:
     `path` must always start with a '/' (which the only caller, `_urljoin`,
     guarantees).
     """
+    if not path.startswith("/"):
+        raise AssertionError("path must start with '/'")
+
     output: list[str] = []
     while path:
         if path.startswith("/./") or path == "/.":
@@ -102,6 +105,8 @@ def _urljoin(base: str, url: str | None) -> str:
 
     `base` must always be a non-empty absolute URL.
     """
+    if not base:
+        raise AssertionError("base must be a non-empty absolute URL")
     if not url:
         return base
 
diff --git a/tests/rewriting/test_rx_replacer.py b/tests/rewriting/test_rx_replacer.py
index 13e3544..fc7cdf8 100644
--- a/tests/rewriting/test_rx_replacer.py
+++ b/tests/rewriting/test_rx_replacer.py
@@ -105,3 +105,11 @@ def test_rx_rewriter_no_rules():
         ]
     )
     assert rewriter.rewrite("pre<replaced>post") == "pre<re123ced>post"
+
+
+def test_rx_rewriter_without_compiled_rule_raises():
+    rewriter = RxRewriter()
+    with pytest.raises(
+        AssertionError, match="compiled_rule must be set before calling rewrite"
+    ):
+        rewriter.rewrite("pre<replaced>post")
diff --git a/tests/rewriting/test_url_rewriting.py b/tests/rewriting/test_url_rewriting.py
index 649d8cb..5ff4b77 100644
--- a/tests/rewriting/test_url_rewriting.py
+++ b/tests/rewriting/test_url_rewriting.py
@@ -8,6 +8,7 @@
     RewriteResult,
     ZimPath,
     _remove_dot_segments,  # pyright: ignore[reportPrivateUsage]
+    _urljoin,  # pyright: ignore[reportPrivateUsage]
 )
 
 
@@ -1308,6 +1309,16 @@ def test_zim_path_str(self):
         assert f"{ZimPath('content/index.html')}" == "ZimPath(content/index.html)"
 
 
+def test_urljoin_rejects_empty_base():
+    with pytest.raises(AssertionError, match="base must be a non-empty absolute URL"):
+        _urljoin("", "foo.html")
+
+
+def test_remove_dot_segments_rejects_path_without_leading_slash():
+    with pytest.raises(AssertionError, match="path must start with '/'"):
+        _remove_dot_segments("foo/bar")
+
+
 def _resolve_relative_reference(base_url: str, reference: str) -> str:
     """Resolve a relative-path reference as browsers do (RFC 3986 section 5.2)