diff --git a/CHANGELOG.md b/CHANGELOG.md
index a1b3c4c..856cc47 100644
--- a/CHANGELOG.md
+++ b/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
diff --git a/docs/technical_architecture.md b/docs/technical_architecture.md
index d3b9a39..a6244c9 100644
--- a/docs/technical_architecture.md
+++ b/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
diff --git a/javascript/src/wombatSetup.js b/javascript/src/wombatSetup.js
index d14761e..9aa84ca 100644
--- a/javascript/src/wombatSetup.js
+++ b/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: ' +
diff --git a/javascript/test/wombatUrlRewriting.js b/javascript/test/wombatUrlRewriting.js
index e047f84..65a9248 100644
--- a/javascript/test/wombatUrlRewriting.js
+++ b/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(
diff --git a/src/zimscraperlib/rewriting/url_rewriting.py b/src/zimscraperlib/rewriting/url_rewriting.py
index 337fc2c..fffe5e0 100644
--- a/src/zimscraperlib/rewriting/url_rewriting.py
+++ b/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 ''}"
)
diff --git a/src/zimscraperlib/zim/items.py b/src/zimscraperlib/zim/items.py
index 767747e..ba18dd1 100644
--- a/src/zimscraperlib/zim/items.py
+++ b/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(
diff --git a/tests/conftest.py b/tests/conftest.py
index 510ff3d..57ec280 100644
--- a/tests/conftest.py
+++ b/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:
diff --git a/tests/rewriting/test_css_rewriting.py b/tests/rewriting/test_css_rewriting.py
index a43ce84..3d7e094 100644
--- a/tests/rewriting/test_css_rewriting.py
+++ b/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);
diff --git a/tests/rewriting/test_html_rewriting.py b/tests/rewriting/test_html_rewriting.py
index 2650492..32cb877 100644
--- a/tests/rewriting/test_html_rewriting.py
+++ b/tests/rewriting/test_html_rewriting.py
@@ -787,7 +787,7 @@ def test_rewrite_base_href(rewrite_base_href_content: ContentForTests):
),
pytest.param(
"""
""",
- """
""",
+ """
""",
id="badly_escaped_src",
),
],
diff --git a/tests/rewriting/test_url_rewriting.py b/tests/rewriting/test_url_rewriting.py
index 8375f27..2d9e2e7 100644
--- a/tests/rewriting/test_url_rewriting.py
+++ b/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"],