Consider equal and comma are safe in URLs for proper Youtube JS player operation

Author: benoit74

CHANGELOG.md

@@ -7,6 +7,10 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 
 ## [Unreleased]
 
+### Fixed
+
+- Do not URL-encode `=` and `,` characters when rewriting document URIs, fixing YouTube JS player breakage (#316)
+
 ## [5.4.0] - 2026-05-28
 
 ### Added

docs/technical_architecture.md

@@ -44,7 +44,7 @@ Below is an example case of the rewrite operation on an image URL found in an HT
 - Document original URL: `https://kiwix.org/a/article/document.html`
 - Document ZIM path: `kiwix.org/a/article/document.html`
 - Image original URL: `//xn--exmple-cva.com/a/resource/image.png?foo=bar`
-- Image rewritten URL: `../../../ex%C3%A9mple.com/a/resource/image.png%3Ffoo%3Dbar`
+- Image rewritten URL: `../../../ex%C3%A9mple.com/a/resource/image.png%3Ffoo=bar`
 - Image ZIM Path: `exémple.com/a/resource/image.png?foo=bar`
 
 ## JS Rewriting

javascript/src/wombatSetup.js

@@ -194,7 +194,11 @@ export function urlRewriteFunction(
 
   // Reencode everything but '/' (we decode it afterwards for simplicity)
   const finalUrl =
-    prefix + encodeURIComponent(fuzzifiedPath).replaceAll('%2F', '/');
+    prefix +
+    encodeURIComponent(fuzzifiedPath)
+      .replaceAll('%2F', '/')
+      .replaceAll('%3D', '=')
+      .replaceAll('%2C', ',');
 
   console.debug(
     'urlRewriten:\n\t- current_url: ' +

javascript/test/wombatUrlRewriting.js

@@ -335,7 +335,7 @@ test('contentWithEncodedQuestionMarkAndQueryParam', (t) => {
       undefined,
       undefined,
     ),
-    'http://library.kiwix.org/content/myzim_yyyy-mm/www.example.com/javascript/cont%3Fnt.txt%3Fquery%3Dvalue',
+    'http://library.kiwix.org/content/myzim_yyyy-mm/www.example.com/javascript/cont%3Fnt.txt%3Fquery=value',
   );
 });
 
@@ -454,7 +454,24 @@ test('contentWithSimpleQueryString', (t) => {
       undefined,
       undefined,
     ),
-    'http://library.kiwix.org/content/myzim_yyyy-mm/www.example.com/javascript/content.txt%3Fquery%3Dvalue',
+    'http://library.kiwix.org/content/myzim_yyyy-mm/www.example.com/javascript/content.txt%3Fquery=value',
+  );
+});
+
+test('contentWithSimplePathWithEqualAndComma', (t) => {
+  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/query1=value1,param2=value2',
+      undefined,
+      undefined,
+      undefined,
+    ),
+    'http://library.kiwix.org/content/myzim_yyyy-mm/www.example.com/javascript/content.txt/query1=value1,param2=value2',
   );
 });
 
@@ -471,7 +488,7 @@ test('contentWithQueryValueEqualSign', (t) => {
       undefined,
       undefined,
     ),
-    'http://library.kiwix.org/content/myzim_yyyy-mm/www.example.com/javascript/content.txt%3Fquery%3Dval%3Deue',
+    'http://library.kiwix.org/content/myzim_yyyy-mm/www.example.com/javascript/content.txt%3Fquery=val=eue',
   );
 });
 
@@ -488,7 +505,7 @@ test('contentWithQueryValuePercentSign', (t) => {
       undefined,
       undefined,
     ),
-    'http://library.kiwix.org/content/myzim_yyyy-mm/www.example.com/javascript/content.txt%3Fquery%3Dval%25eue',
+    'http://library.kiwix.org/content/myzim_yyyy-mm/www.example.com/javascript/content.txt%3Fquery=val%25eue',
   );
 });
 
@@ -505,7 +522,7 @@ test('contentWithQueryParamPercentSign', (t) => {
       undefined,
       undefined,
     ),
-    'http://library.kiwix.org/content/myzim_yyyy-mm/www.example.com/javascript/content.txt%3Fque%25ry%3Dvaleue',
+    'http://library.kiwix.org/content/myzim_yyyy-mm/www.example.com/javascript/content.txt%3Fque%25ry=valeue',
   );
 });
 
@@ -522,7 +539,7 @@ test('contentWithQueryParamPlusSign', (t) => {
       undefined,
       undefined,
     ),
-    'http://library.kiwix.org/content/myzim_yyyy-mm/www.example.com/javascript/content.txt%3Fparam%3Dval%20ue',
+    'http://library.kiwix.org/content/myzim_yyyy-mm/www.example.com/javascript/content.txt%3Fparam=val%20ue',
   );
 });
 
@@ -811,7 +828,7 @@ test('youtubeFuzzyNotEncoded', (t) => {
       undefined,
       undefined,
     ),
-    'http://library.kiwix.org/content/myzim_yyyy-mm/www.example.com/path1/youtube.fuzzy.replayweb.page/get_video_info%3Fvideo_id%3D123ah',
+    'http://library.kiwix.org/content/myzim_yyyy-mm/www.example.com/path1/youtube.fuzzy.replayweb.page/get_video_info%3Fvideo_id=123ah',
   );
 });
 
@@ -828,7 +845,7 @@ test('youtubeFuzzyEncoded', (t) => {
       undefined,
       undefined,
     ),
-    'http://library.kiwix.org/content/myzim_yyyy-mm/www.example.com/path1/youtube.fuzzy.replayweb.page/get_video_info%3Fvideo_id%3D12%3D3ah',
+    'http://library.kiwix.org/content/myzim_yyyy-mm/www.example.com/path1/youtube.fuzzy.replayweb.page/get_video_info%3Fvideo_id=12=3ah',
   );
 });
 
@@ -931,7 +948,7 @@ test('relAlreadyEncoded', (t) => {
       undefined,
       undefined,
     ),
-    'http://library.kiwix.org/content/myzim_yyyy-mm/www.example.com/javascript/content.txt%3Fquery%3Dvalue',
+    'http://library.kiwix.org/content/myzim_yyyy-mm/www.example.com/javascript/content.txt%3Fquery=value',
   );
 });
 
@@ -1001,7 +1018,8 @@ test('relAnotherHostAlreadyRewrittenEmptyPath', (t) => {
 });
 
 // this is an edge case where the URL has already been statically rewritten and is located
-// on a different fuzzified domain name => we do not touch it at all
+// on a different fuzzified domain name => we do not touch it at all (for instance it keeps
+// %3D even if it should not have them)
 test('relAnotherFuzzifiedHostAlreadyRewritten', (t) => {
   t.is(
     urlRewriteFunction(

src/zimscraperlib/rewriting/url_rewriting.py

@@ -34,8 +34,10 @@
 start by resolving it into an absolute URL (based on the containing document absolute
 URI), applying the transformation to compute the corresponding ZIM path and we
 url-encode the whole ZIM path, so that readers will have one single blob to process,
-url-decode and find corresponding ZIM entry. Only '/' separators are considered safe
-and not url-encoded.
+url-decode and find corresponding ZIM entry. Only '/', '=' and ',' are considered safe
+and not url-encoded. '=' and ',' are kept unencoded because some sites (e.g. YouTube)
+embed scripts that parse their own script URL and expect these characters to appear
+literally in it.
 """
 
 import re
@@ -332,7 +334,7 @@ def get_document_uri(self, item_path: ZimPath, item_fragment: str) -> str:
             relative_path += "/"
 
         return (
-            f"{quote(relative_path, safe='/')}"
+            f"{quote(relative_path, safe='/=,')}"
             f"{'#' + item_fragment if item_fragment else ''}"
         )
 

src/zimscraperlib/zim/items.py

@@ -270,7 +270,7 @@ def __init__(
             if self.headers.get("Content-Encoding", "identity") != "identity":
                 raise ValueError("Can't trust Content-Length for size")
             # non-html, non-compressed data.
-            self.size = int(self.headers["Content-Length"])
+            self.size = int(self.headers["Content-Length"])  # pragma: no cover
         except Exception:
             # we couldn't retrieve size so we have to download resource to
             target, self.size = self.download_for_size(

tests/conftest.py

@@ -74,7 +74,7 @@ def gzip_html_url() -> str:
 
 @pytest.fixture(scope="module")
 def gzip_nonhtml_url() -> str:
-    return "http://mirror.download.kiwix.org/robots.txt"
+    return "http://lb.download.kiwix.org/robots.txt"
 
 
 def file_src(fname: str) -> pathlib.Path:

tests/rewriting/test_css_rewriting.py

@@ -190,7 +190,7 @@ def test_rewrite():
 
     expected = """
     /* A comment with a link : http://foo.com */
-    @import url(../fonts.googleapis.com/icon%3Ffamily%3DMaterial%20Icons);
+    @import url(../fonts.googleapis.com/icon%3Ffamily=Material%20Icons);
 
     p, input {
         color: rbg(1, 2, 3);

tests/rewriting/test_html_rewriting.py

@@ -787,7 +787,7 @@ 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%3Dvalue1%26param2%3Dvalue2">""",
+            """<img src="image.png%3Fparam1=value1%26param2=value2">""",
             id="badly_escaped_src",
         ),
     ],

tests/rewriting/test_url_rewriting.py

@@ -170,7 +170,7 @@ def test_missing_zim_paths(
                 "foo.html?foo=bar",
                 RewriteResult(
                     "https://kiwix.org/a/article/foo.html?foo=bar",
-                    "foo.html%3Ffoo%3Dbar",
+                    "foo.html%3Ffoo=bar",
                     ZimPath("kiwix.org/a/article/foo.html?foo=bar"),
                 ),
                 ["kiwix.org/a/article/foo.html?foo=bar"],
@@ -181,7 +181,7 @@ def test_missing_zim_paths(
                 "foo.html?foo=b%24ar",
                 RewriteResult(
                     "https://kiwix.org/a/article/foo.html?foo=b%24ar",
-                    "foo.html%3Ffoo%3Db%24ar",
+                    "foo.html%3Ffoo=b%24ar",
                     ZimPath("kiwix.org/a/article/foo.html?foo=b$ar"),
                 ),
                 ["kiwix.org/a/article/foo.html?foo=b$ar"],
@@ -192,7 +192,7 @@ def test_missing_zim_paths(
                 "foo.html?foo=b%3Far",  # a query string with an encoded ? char in value
                 RewriteResult(
                     "https://kiwix.org/a/article/foo.html?foo=b%3Far",
-                    "foo.html%3Ffoo%3Db%3Far",
+                    "foo.html%3Ffoo=b%3Far",
                     ZimPath("kiwix.org/a/article/foo.html?foo=b?ar"),
                 ),
                 ["kiwix.org/a/article/foo.html?foo=b?ar"],
@@ -316,7 +316,7 @@ def test_missing_zim_paths(
                 "foo?param=value",
                 RewriteResult(
                     "https://kiwix.org/a/article/foo?param=value",
-                    "foo%3Fparam%3Dvalue",
+                    "foo%3Fparam=value",
                     ZimPath("kiwix.org/a/article/foo?param=value"),
                 ),
                 ["kiwix.org/a/article/foo?param=value"],
@@ -327,7 +327,7 @@ def test_missing_zim_paths(
                 "foo?param=value%2F",
                 RewriteResult(
                     "https://kiwix.org/a/article/foo?param=value%2F",
-                    "foo%3Fparam%3Dvalue/",
+                    "foo%3Fparam=value/",
                     ZimPath("kiwix.org/a/article/foo?param=value/"),
                 ),
                 ["kiwix.org/a/article/foo?param=value/"],
@@ -338,12 +338,26 @@ def test_missing_zim_paths(
                 "foo?param=value%2Fend",
                 RewriteResult(
                     "https://kiwix.org/a/article/foo?param=value%2Fend",
-                    "foo%3Fparam%3Dvalue/end",
+                    "foo%3Fparam=value/end",
                     ZimPath("kiwix.org/a/article/foo?param=value/end"),
                 ),
                 ["kiwix.org/a/article/foo?param=value/end"],
                 False,
             ),
+            (
+                # regression test for #316: '=' and ',' must not be encoded so that
+                # e.g. the YouTube JS player can parse its own script URL properly,
+                # while other reserved characters (e.g. '?') are still encoded
+                "https://kiwix.org/a/article/document.html",
+                "foo?k=ytembeds.base.en_US,foo.bar",
+                RewriteResult(
+                    "https://kiwix.org/a/article/foo?k=ytembeds.base.en_US,foo.bar",
+                    "foo%3Fk=ytembeds.base.en_US,foo.bar",
+                    ZimPath("kiwix.org/a/article/foo?k=ytembeds.base.en_US,foo.bar"),
+                ),
+                ["kiwix.org/a/article/foo?k=ytembeds.base.en_US,foo.bar"],
+                False,
+            ),
             (
                 "https://kiwix.org/a/article/document.html",
                 "foo/",
@@ -393,7 +407,7 @@ def test_missing_zim_paths(
                 "/foo.html?param=val+ue",
                 RewriteResult(
                     "https://kiwix.org/foo.html?param=val+ue",
-                    "../../foo.html%3Fparam%3Dval%20ue",
+                    "../../foo.html%3Fparam=val%20ue",
                     ZimPath("kiwix.org/foo.html?param=val ue"),
                 ),
                 ["kiwix.org/foo.html?param=val ue"],