GeezerVison is showing github.com — Add an opt-in ZIM path prefix so rewritten URIs are recognizable by pawank925 · Pull Request #397 · openzim/python-scraperlib
Conversation
pawank925
commented
Nov 7, 2026
•
edited
Loading
Uh oh!
There was an error while loading. Please reload this page.
Summary
#396 asks whether a known prefix in every rewritten URL would let wombat tell a rewritten URL from an original one, so that the original no longer has to be recorded everywhere. Answering that needs two halves inside scraperlib, and this adds both behind a single opt-in switch:
ArticleUrlRewriter.zim_path_prefix (a ClassVar, same pattern as the existing additional_rules) plus set_zim_path_prefix(), which validates it. normalize() then prepends it, so entry keys become zimreplay/example.com/a/page.html instead of example.com/a/page.html.
get_document_uri() then walks all the way up to the ZIM root and comes back down through the prefix, instead of stopping at the closest common segment. A link from example.com/dir/page1.html to its sibling ./page2.html becomes ../../../zimreplay/example.com/dir/page2.html rather than today's page2.html.
That second half is the part that actually matters: it is what makes the prefix survive as a literal segment in the URI. Without it the prefix cancels out of every same-host link and a rewritten URI is still indistinguishable from an original one, which is exactly the ambiguity #338 had to resolve by recording __wb_orig_*.
Both are off by default, so existing entry paths and rewritten URIs are byte-identical to today.
The prefix charset is deliberately narrow ([a-zA-Z0-9._-], one segment, ./.. rejected), because a : would make readers parse the URI as a scheme, a / would make it several segments, and anything needing percent-encoding would stop matching the literal string a reader looks for.
What this does not settle
Stating this plainly, since #396 is still an open design question and this PR has no maintainer sign-off on direction:
- Scrapers still have to move. Turning the prefix on changes every entry key, so a scraper must both set the prefix and store its entries under prefixed paths. Existing ZIMs would need rebuilding; this cannot be switched on silently.
- wombat still needs its own change to use the marker (strip everything up to and including the prefix instead of consulting
__wb_orig_* / extractOriginalURL). That is out of scope here, and __wb_orig_* recording is left untouched, so the prefix is purely additive.
- It does not make the URL → ZIM path reversible. The three collisions listed in URL rewriting: can we add a prefix so that we do not need to store original URLs everywhere #396 (
%2F vs /, a literal _. vs an unresolved . segment, + vs %20 vs a raw space) are properties of normalize() itself and are unchanged by this PR. So the prefix answers "has this URL already been rewritten?" — it does not answer "what was the original URL?", which is the second half of @benoit74's remark in the issue.
Treat this as a concrete prototype to react to rather than a finished design: if the API shape is wrong (global ClassVar vs a constructor argument vs a per-scraper config), say so and I'll rework it.
Tests
TestZimPathPrefix (32 cases): default off, prefixed normalize(), the rewritten URI for sibling / cross-host / directory-document / query+fragment / root-article / consecutive-slash articles, non-rewritten URLs (#anchor, mailto:) unaffected, and prefix validation accepting/rejecting.
- A property test that resolves every prefixed URI back the way a reader does (walking
../ up to the ZIM root) and asserts it lands on exactly the zim_path the rewriter reported — so root-anchoring changes the URI's shape, never its target.
- One HTML-level case showing an
<a href> rewritten with the prefix while __wb_orig_href is still recorded.
tests/rewriting/: 652 passed (185 → 217 in test_url_rewriting.py). CI is all green: check-python-qa (ruff + strict pyright), run-python-tests, run-js-tests, build-docs, codecov/patch and codecov/project. The 22 failures in tests/image/test_image.py on my machine are missing gifsicle/jpegoptim binaries (exit 127), unrelated to this change.
Refs #396
Sorry, something went wrong.
Uh oh!
There was an error while loading. Please reload this page.
codecov
Bot
commented
Nov 7, 2026
•
edited
Loading
Uh oh!
There was an error while loading. Please reload this page.
|
✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 100.00%. Comparing base (ca39112) to head (650d70b).
Additional details and impacted files
@@ Coverage Diff @@
## main #397 +/- ##
=========================================
Coverage 100.00% 100.00%
=========================================
Files 42 42
Lines 2937 2950 +13
Branches 450 452 +2
=========================================
+ Hits 2937 2950 +13
☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.
🚀 New features to boost your workflow:
- ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
- 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.
|
Sorry, something went wrong.
Uh oh!
There was an error while loading. Please reload this page.
Sorry, something went wrong.
Uh oh!
There was an error while loading. Please reload this page.
Off by default: entry paths keep their host as first segment and rewritten
URIs keep resolving as-up-as-needed.
pawank925
commented
Nov 8, 2026
•
edited
Loading
Uh oh!
There was an error while loading. Please reload this page.
|
@benoit74 it will be in draft until the discussion comes to an end
|
Sorry, something went wrong.
Uh oh!
There was an error while loading. Please reload this page.
This branch has not been deployed
No deployments
Uh oh!
There was an error while loading. Please reload this page.
Assignees
No one assigned
Milestone
No milestone
Development
Successfully merging this pull request may close these issues.
Uh oh!
There was an error while loading. Please reload this page.
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.
|