Resolve relative links against page URLs with consecutive slashes

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

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(

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
         )

src/zimscraperlib/rewriting/url_rewriting.py

@@ -43,7 +43,13 @@
 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_relative,
+)
 
 import idna
 
@@ -63,6 +69,72 @@ 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` 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 == "/.":
+            path = "/" + path[3:]
+        elif path.startswith("/../") or path == "/..":
+            path = "/" + path[4:]
+            if output:
+                output.pop()
+        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).
+
+    `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
+
+    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 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 +265,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 +285,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

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

tests/rewriting/test_url_rewriting.py

@@ -7,6 +7,8 @@
     HttpUrl,
     RewriteResult,
     ZimPath,
+    _remove_dot_segments,  # pyright: ignore[reportPrivateUsage]
+    _urljoin,  # pyright: ignore[reportPrivateUsage]
 )
 
 
@@ -648,6 +650,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(
@@ -663,6 +683,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",
         [
@@ -1075,6 +1108,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(
@@ -1265,28 +1309,22 @@ 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)
 
     Unlike `urllib.parse.urljoin`, empty path segments are kept. Returns the host
     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)}"