Keep encoded # in ZIM paths when building document URIs - #342
Merged
Merged
Conversation
A `%23` in the path or query of an original URL is decoded to a literal `#` in the ZIM path, as expected. But get_document_uri() then passed that ZIM path to urlsplit(), which treated the `#` as a fragment delimiter and silently dropped everything after it. The rewritten link therefore pointed to another (usually missing) ZIM entry, e.g. `a/b%23c` was rewritten to `a/b`. Tell urlsplit() not to look for a fragment: a ZIM path is never encoded, so the `#` is part of the path. It is then re-encoded to `%23` by the existing quote() call, which matches what the JS side (wombatSetup.js) already produces via encodeURIComponent(). Fix openzim#341
benoit74
approved these changes
Sep 28, 2026
benoit74
left a comment
Collaborator
There was a problem hiding this comment.
Thank you very much, a very good PR, nothing to add besides "let's merge". Looking forward for more contribution on "sibling" issues (or any other one actually) if you see fit.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #342 +/- ##
=========================================
Coverage 100.00% 100.00%
=========================================
Files 42 42
Lines 2694 2694
Branches 398 398
=========================================
Hits 2694 2694 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fix #341
normalize()correctly decodes%23to a literal#in the ZIM path, butget_document_uri()then runs that ZIM path throughurlsplit(), which treats the#as a fragment delimiter and drops everything after it. So the rewritten link points to another entry, usually one that does not exist in the ZIM.The change is one line:
urlsplit(item_path.value, allow_fragments=False). A ZIM path is never encoded, so a#in it is always part of the path or query. The existingquote(relative_path, safe="/=,")then turns it back into%23. A real fragment passed asitem_fragmentis still appended as before.Using @veloman-yunkan's script from openzim/overview#92 (root
https://example.com/):a/b%23dont_drop_me_pleasea/ba/b%23dont_drop_me_pleasea/b%23x#frag(full rewrite)a/b#fraga/b%23x#fraga/b?q=1%232a/b%3Fq=1a/b%3Fq=1%232For comparison,
urlRewriteFunctioninjavascript/src/wombatSetup.jsalready gives.../a/b%23dont_drop_me_pleaseand.../a/b%3Fq=1%232for the same inputs, because it goes throughencodeURIComponent(). With this change the static (Python) rewriting and the dynamic (wombat) rewriting agree on these URLs.Tests: three new cases in
test_relative_url(encoded#in the path, encoded#followed by a real fragment, encoded#in the query). All three fail onmainand pass with the fix. Locally:tests/rewriting/test_url_rewriting.py139 passed;ruff check,ruff format --checkandpyrighton the touched files are clean. (I could not run the libmagic-dependent test modules on my machine; the rest of the suite is 627 passed before, 630 after.)This does not touch the other points from openzim/overview#92 (#340 on
//, #321 on dot segments).CHANGELOG entry added under Unreleased / Fixed.