Skip to content

Commit f08bb5a

Browse files
goosfrabbaclaude
andcommitted
Fix crash rewriting relative document URI on ..// in querystring
`ArticleUrlRewriter.get_document_uri` computed the relative path between two ZIM entries by feeding `path + "?" + querystring` to `PurePosixPath`, which splits on `/` and interprets `..` segments as directory navigation. When the document being rewritten (or a linked item) had a querystring containing `..` (e.g. `xtree.html?css=../../prg`), `PurePosixPath.relative_to(..., walk_up=True)` raised `ValueError: '..' segment ... cannot be walked`, aborting the scrape. A querystring is part of the ZIM entry name (a leaf), not a navigable directory, so its content must not take part in the relative-path walk. Compute the relative path from the path components only, then re-append the querystring (url-encoded together with the path) once the relative path is known. Existing outputs are unchanged for querystrings without `/` or `..` segments. Fixes openzim/warc2zim#380 Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
1 parent 295fefc commit f08bb5a

3 files changed

Lines changed: 64 additions & 12 deletions

File tree

CHANGELOG.md

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,10 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
77

88
## [Unreleased]
99

10+
### Fixed
11+
12+
- Fix crash in `ArticleUrlRewriter.get_document_uri` when the document or a linked item has a `..` (or `/`) segment in its querystring ; querystrings are part of the ZIM entry name and must not be interpreted as navigable path segments (openzim/warc2zim#380)
13+
1014
## [5.4.0] - 2026-05-28
1115

1216
### Added

src/zimscraperlib/rewriting/url_rewriting.py

Lines changed: 20 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -311,25 +311,33 @@ def get_document_uri(self, item_path: ZimPath, item_fragment: str) -> str:
311311
312312
"""
313313
item_parts = urlsplit(item_path.value)
314-
315-
# item_path is both path + querystring, both will be url-encoded in the document
316-
# so that readers consider them as a whole and properly pass them to libzim
317-
item_url = item_parts.path
318-
if item_parts.query:
319-
item_url += "?" + item_parts.query
314+
article_parts = urlsplit(self.article_path.value)
315+
316+
# The relative path is computed using only the path components. A querystring is
317+
# part of the ZIM entry name (a leaf), it is not a navigable directory, so its
318+
# content (which may contain `/` or even `..` segments) must not be interpreted
319+
# as path navigation when computing the relative path ; doing so crashes on `..`
320+
# segments (see https://github.com/openzim/warc2zim/issues/380). The querystring
321+
# is re-appended and url-encoded together with the path below so that readers
322+
# consider path + querystring as a single whole entry.
320323
relative_path = str(
321-
PurePosixPath(item_url).relative_to(
324+
PurePosixPath(item_parts.path).relative_to(
322325
(
323-
PurePosixPath(self.article_path.value)
324-
if self.article_path.value.endswith("/")
325-
else PurePosixPath(self.article_path.value).parent
326+
PurePosixPath(article_parts.path)
327+
if article_parts.path.endswith("/")
328+
else PurePosixPath(article_parts.path).parent
326329
),
327330
walk_up=True,
328331
)
329332
)
330-
# relative_to removes a potential last '/' in the path, we add it back
331-
if item_path.value.endswith("/"):
333+
# relative_to removes a potential last '/' in the path, we add it back before
334+
# appending the querystring
335+
if item_parts.path.endswith("/"):
332336
relative_path += "/"
337+
# re-append the querystring (part of the ZIM entry name) now that the relative
338+
# path has been computed
339+
if item_parts.query:
340+
relative_path += "?" + item_parts.query
333341

334342
return (
335343
f"{quote(relative_path, safe='/')}"

tests/rewriting/test_url_rewriting.py

Lines changed: 40 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -532,6 +532,46 @@ def test_relative_url(
532532
== expected_rewrite_result
533533
)
534534

535+
@pytest.mark.parametrize(
536+
"article_url, item_url, expected_document_uri",
537+
[
538+
# `..` in the querystring of the document being rewritten must not be
539+
# interpreted as a navigable path segment (see
540+
# https://github.com/openzim/warc2zim/issues/380 ; it used to raise
541+
# `ValueError: '..' segment ... cannot be walked`)
542+
pytest.param(
543+
"http://kiwix.org/common/sub/page.html?css=../../prg",
544+
"http://kiwix.org/_zim_static/wombat.js",
545+
"../../_zim_static/wombat.js",
546+
id="dotdot_in_article_querystring",
547+
),
548+
# `..` in the querystring of the linked item is kept as-is in the (encoded)
549+
# entry name and must not trigger path navigation either
550+
pytest.param(
551+
"http://kiwix.org/common/page.html",
552+
"http://kiwix.org/common/other.html?css=../../prg",
553+
"other.html%3Fcss%3D../../prg",
554+
id="dotdot_in_item_querystring",
555+
),
556+
# a `/` inside the querystring is part of the entry name, not a directory
557+
pytest.param(
558+
"http://kiwix.org/a/b/page.html",
559+
"http://kiwix.org/a/b/img.png?x=1/2",
560+
"img.png%3Fx%3D1/2",
561+
id="slash_in_item_querystring",
562+
),
563+
],
564+
)
565+
def test_get_document_uri_querystring_segments(
566+
self,
567+
article_url: str,
568+
item_url: str,
569+
expected_document_uri: str,
570+
):
571+
rewriter = ArticleUrlRewriter(article_url=HttpUrl(article_url))
572+
item_path = ArticleUrlRewriter.normalize(HttpUrl(item_url))
573+
assert rewriter.get_document_uri(item_path, "") == expected_document_uri
574+
535575
@pytest.mark.parametrize(
536576
"article_url, original_content_url, expected_rewrite_result, know_paths, "
537577
"rewrite_all_url",

0 commit comments

Comments
 (0)