From 94b65cef55c91c0139c2646f93b9bfc28dddfdf6 Mon Sep 17 00:00:00 2001
From: Eric Pheterson <epheterson@gmail.com>
Date: Wed, 16 Sep 2026 08:03:32 -0700
Subject: [PATCH] Record the original for every rewritten URL attribute

Widens the script-src case to src, href and xlink:href on any tag.

wombat's getAttribute override looks for the recorded original first and
otherwise calls extractOriginalURL, which can only reverse rewrites
wombat itself performed. A value rewritten here is unrecognisable to it,
so it prepends the original scheme to what is already a relative path.

That is openzim/warc2zim#413 as well as #473: same cause, two symptoms.

Needs the matching wombat change, since retrieveWBOSRC is SCRIPT-only
until then and nothing reads the other attributes back.

Measured on en.wikipedia.org/wiki/Whale (1,766 links): 152 attributes
added, 0.9% larger gzipped. Most links are fragments or already relative
and record nothing.

poster is deliberately excluded: wombat rewrites it, this rewriter does
not, so it would record nothing.
---
 src/zimscraperlib/rewriting/html.py    | 94 +++++++++++++-------------
 tests/rewriting/test_html_rewriting.py | 55 +++++++++++----
 2 files changed, 89 insertions(+), 60 deletions(-)

diff --git a/src/zimscraperlib/rewriting/html.py b/src/zimscraperlib/rewriting/html.py
index 001ef84..25594e3 100644
--- a/src/zimscraperlib/rewriting/html.py
+++ b/src/zimscraperlib/rewriting/html.py
@@ -54,55 +54,55 @@ def get_attr_value_from(
     return default
 
 
-# The attribute wombat reads a script's pre-rewrite src back out of.
+# The attributes wombat asks for a pre-rewrite value of.
 #
-# wombat already overrides Element.prototype.getAttribute and, for a <script>,
-# checks this attribute before anything else (retrieveWBOSRC). It exists for
-# exactly this situation and nothing was filling it in.
-WB_ORIG_SRC_ATTRIBUTE = "__wb_orig_src"
+# wombat overrides Element.prototype.getAttribute, and for any attribute it
+# considers a URL it first looks for the original the server-side rewriter
+# recorded (retrieveWBOSRC), falling back to extractOriginalURL. That fallback
+# can only reverse rewrites WOMBAT performed; a value rewritten here, at scrape
+# time, is unrecognisable to it, and it ends up prepending the original scheme
+# to an already-relative path.
+#
+# These are the attributes wombat treats as a URL on every tag, and so the ones
+# it will come asking about.
+WB_ORIG_ATTRIBUTE_PREFIX = "__wb_orig_"
+WB_ORIG_ATTRIBUTES = ("src", "href", "xlink:href")
 
 
-def get_script_original_src(
-    tag: str, attrs: AttrsList, rewritten_attrs: list[AttrNameAndValue]
-) -> str | None:
-    """The src a <script> had before rewriting, when rewriting changed it.
-
-    Bundlers identify their chunks by the raw ``getAttribute("src")``, not by
-    the ``.src`` property, and they expect the string the server sent. Rewriting
-    an absolute path to a relative one therefore breaks them silently: Next.js
-    with Turbopack strips a leading ``/_next/`` to get a chunk's name, the strip
-    no longer matches, every chunk registers under a name nothing is waiting
-    for, and the page never hydrates. Vite is reported to be affected the same
-    way.
-
-    wombat cannot repair this on its own. Its getAttribute override un-rewrites
-    URLs that WOMBAT rewrote, and this one was rewritten here, at scrape time —
-    so extractOriginalURL sees no prefix it knows and hands the value straight
-    back. What it does have is ``__wb_orig_src``, which it checks first for
-    script elements and which only the rewriter can fill in.
-
-    None when the tag is not a script, when it had no src, when rewriting left
-    the src alone, or when the tag already carries the attribute — there is
-    nothing to remember in those cases, and an attribute that merely repeats
-    the src is noise in every captured page.
-
-    The already-carries case is HTML being rewritten a second time. What is
-    recorded then is the src of the FIRST pass, which is already a rewritten
-    value; keeping the attribute that is there preserves the true original, and
-    appending a second one would both duplicate the attribute and let the stale
-    value win, since a browser takes the first.
+def wb_orig_attribute(attr_name: str) -> str:
+    """The attribute wombat reads ``attr_name``'s pre-rewrite value out of."""
+    return WB_ORIG_ATTRIBUTE_PREFIX + attr_name
+
+
+def get_original_attr_values(
+    attrs: AttrsList, rewritten_attrs: list[AttrNameAndValue]
+) -> list[AttrNameAndValue]:
+    """The pre-rewrite values worth recording, as (attribute, value) pairs.
+
+    Two symptoms, one cause. A bundler identifies its chunks by the raw
+    ``getAttribute("src")`` and expects the string the server sent, so a
+    rewritten src stops matching and the page never hydrates. And a rewritten
+    href comes back with the original scheme glued onto a relative path
+    (openzim/warc2zim#413).
+
+    Nothing is recorded for an attribute that had no value, that rewriting left
+    alone, or that already carries its original. That last is HTML rewritten
+    twice: the attribute already present is the true original, and a second
+    would both duplicate it and win, since a browser takes the first.
     """
-    if tag != "script":
-        return None
-    if get_attr_value_from(attrs, WB_ORIG_SRC_ATTRIBUTE) is not None:
-        return None
-    original = get_attr_value_from(attrs, "src")
-    if not original:
-        return None
-    rewritten = get_attr_value_from(rewritten_attrs, "src")
-    if rewritten is None or rewritten == original:
-        return None
-    return original
+    recorded: list[AttrNameAndValue] = []
+    for attr_name in WB_ORIG_ATTRIBUTES:
+        orig_name = wb_orig_attribute(attr_name)
+        if get_attr_value_from(attrs, orig_name) is not None:
+            continue
+        original = get_attr_value_from(attrs, attr_name)
+        if not original:
+            continue
+        rewritten = get_attr_value_from(rewritten_attrs, attr_name)
+        if rewritten is None or rewritten == original:
+            continue
+        recorded.append((orig_name, original))
+    return recorded
 
 
 def format_attr(name: str, value: str | None) -> str:
@@ -263,9 +263,7 @@ def handle_starttag(self, tag: str, attrs: AttrsList, *, auto_close: bool = Fals
                 tag=tag, attr_name=attr_name, attr_value=attr_value, attrs=attrs
             )
         ]
-        original_src = get_script_original_src(tag, attrs, rewritten_attrs)
-        if original_src is not None:
-            rewritten_attrs.append((WB_ORIG_SRC_ATTRIBUTE, original_src))
+        rewritten_attrs.extend(get_original_attr_values(attrs, rewritten_attrs))
         self.send(" ".join(format_attr(*attr) for attr in rewritten_attrs))
 
         if auto_close:
diff --git a/tests/rewriting/test_html_rewriting.py b/tests/rewriting/test_html_rewriting.py
index 38b8432..695d50f 100644
--- a/tests/rewriting/test_html_rewriting.py
+++ b/tests/rewriting/test_html_rewriting.py
@@ -229,7 +229,8 @@ def test_escaped_content(escaped_content: ContentForTests):
             '<link rel="preload" src="https://cdnjs.cloudflare.com/jquery.min.js"'
             ' integrity="sha512-3gJwYpMe3QewGELv8k/BX9vcqhryRdzRMxVfq6ngyWXwo03GFEzjsUm'
             '8Q7RZcHPHksttq7/GFoxjCVUjkjvPdw=="></link>',
-            '<link rel="preload" src="../cdnjs.cloudflare.com/jquery.min.js"></link>',
+            '<link rel="preload" src="../cdnjs.cloudflare.com/jquery.min.js"'
+            ' __wb_orig_src="https://cdnjs.cloudflare.com/jquery.min.js"></link>',
         ),
         ContentForTests(
             '<link rel="preload" as="script"'
@@ -237,7 +238,8 @@ def test_escaped_content(escaped_content: ContentForTests):
             ' integrity="sha512-3gJwYpMe3QewGELv8k/BX9vcqhryRdzRMxVfq6ngyWXwo03GFEzjsUm'
             '8Q7RZcHPHksttq7/GFoxjCVUjkjvPdw=="></link>',
             '<link rel="preload" as="script" '
-            'src="../cdnjs.cloudflare.com/jquery.min.js"></link>',
+            'src="../cdnjs.cloudflare.com/jquery.min.js"'
+            ' __wb_orig_src="https://cdnjs.cloudflare.com/jquery.min.js"></link>',
         ),
     ]
 )
@@ -261,8 +263,16 @@ def test_js_rewrites(js_rewrites: ContentForTests):
     assert transformed == js_rewrites.expected_str
 
 
+LONG_PATH = "http://exemple.com/a/long/path"
+
+
 def long_path_replace_test_content(input_: str, rewriten_url: str, article_url: str):
-    expected = input_.replace("http://exemple.com/a/long/path", rewriten_url)
+    expected = input_.replace(LONG_PATH, rewriten_url)
+    if rewriten_url != LONG_PATH:
+        # A rewritten href records the value the server sent, for wombat.
+        expected = expected.replace(
+            '">A link', f'" __wb_orig_href="{LONG_PATH}">A link'
+        )
     return ContentForTests(input_, expected, article_url)
 
 
@@ -394,12 +404,12 @@ def test_rewrite_attributes():
 
     assert (
         rewriter.rewrite("<a href='https://kiwix.org/foo'>A link</a>").content
-        == '<a href="foo">A link</a>'
+        == '<a href="foo" __wb_orig_href="https://kiwix.org/foo">A link</a>'
     )
 
     assert (
         rewriter.rewrite("<img src='https://kiwix.org/foo'></img>").content
-        == '<img src="foo"></img>'
+        == '<img src="foo" __wb_orig_src="https://kiwix.org/foo"></img>'
     )
 
     assert (
@@ -676,7 +686,8 @@ def test_extract_base_href(html_content: str, expected_base_href: str):
         ContentForTests(
             '<html><head><base href="../"></head>'
             '<body><a href="foo.html"></a></body></html>',
-            '<html><head></head><body><a href="../foo.html"></a></body></html>',
+            "<html><head></head><body>"
+            '<a href="../foo.html" __wb_orig_href="foo.html"></a></body></html>',
             "kiwix.org/a/index.html",
         ),
         ContentForTests(
@@ -727,8 +738,8 @@ def test_extract_base_href(html_content: str, expected_base_href: str):
         ContentForTests(
             '<html><head> <link rel="shortcut icon" href="favicon.ico">'
             '<base href="../"></head><body></body></html>',
-            '<html><head> <link rel="shortcut icon" href="../favicon.ico">'
-            "</head><body></body></html>",
+            '<html><head> <link rel="shortcut icon" href="../favicon.ico"'
+            ' __wb_orig_href="favicon.ico"></head><body></body></html>',
             "kiwix.org/a/index.html",
         ),
     ]
@@ -790,7 +801,8 @@ def test_rewrite_base_href(rewrite_base_href_content: ContentForTests):
         ),
         pytest.param(
             """<img src="image.png?param1=value1&param2=value2">""",
-            """<img src="image.png%3Fparam1=value1%26param2=value2">""",
+            '<img src="image.png%3Fparam1=value1%26param2=value2"'
+            ' __wb_orig_src="image.png?param1=value1&amp;param2=value2">',
             id="badly_escaped_src",
         ),
     ],
@@ -1621,8 +1633,25 @@ def test_rewrite_meta_http_equiv_redirect_rule(
         ),
         pytest.param(
             '<img src="/img/a.png">',
-            '<img src="../img/a.png">',
-            id="only_scripts_get_it",
+            '<img src="../img/a.png" __wb_orig_src="/img/a.png">',
+            id="an_image_gets_one_too",
+        ),
+        pytest.param(
+            '<a href="/page?ref=noted.lol">z</a>',
+            '<a href="https://kiwix.org/page?ref=noted.lol"'
+            ' __wb_orig_href="/page?ref=noted.lol">z</a>',
+            id="a_link_gets_one_too",
+        ),
+        pytest.param(
+            '<a href="#anchor">x</a>',
+            '<a href="#anchor">x</a>',
+            id="an_unchanged_href_adds_nothing",
+        ),
+        pytest.param(
+            '<a href="/b.html" __wb_orig_href="/original/b.html">x</a>',
+            '<a href="https://kiwix.org/b.html"'
+            ' __wb_orig_href="/original/b.html">x</a>',
+            id="rewriting_twice_keeps_the_first_original_for_any_attribute",
         ),
         pytest.param(
             '<script src="/_next/a.js" __wb_orig_src="/original/a.js"></script>',
@@ -1631,7 +1660,9 @@ def test_rewrite_meta_http_equiv_redirect_rule(
         ),
     ],
 )
-def test_script_keeps_its_original_src(input_str: str, expected_str: str):
+def test_rewritten_url_attributes_keep_their_original(
+    input_str: str, expected_str: str
+):
     """A rewritten <script> remembers the src the server sent, for wombat.
 
     Bundlers identify a chunk by the raw ``getAttribute("src")`` rather than by