Repository navigation
fix(epub): resolve single-quoted and percent-escaped image sources - #4602
Merged
Merged
Conversation
Signed-off-by: Ritankar Mondal <[email protected]>
Contributor
|
✅ DCO Check Passed Thanks @CyberRik, all your commits are properly signed off. 🎉 |
Contributor
Merge Protections🟢 Merge protection satisfied — ready to merge. Show 1 satisfied protection🟢 Enforce conventional commitMake sure that we follow https://www.conventionalcommits.org/en/v1.0.0/
|
ceberam
added
bug
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
ceberam
reviewed
Oct 6, 2026
Signed-off-by: Ritankar Mondal <[email protected]>
dolfim-ibm
approved these changes
Oct 7, 2026
dolfim-ibm
left a comment
dolfim-ibm
left a comment
Member
There was a problem hiding this comment.
Looks good. Two follow-ups:
- strip the query/fragment before
unquote(decode onlyurlsplit(src).path), somap.png?v=1resolves and an escaped%3Fis not taken as a query; - a path that fails the base-directory check raises
ValueErrorand fails the whole document. Such a path can now also arrive through%2e%2e%2for%2F…. This is shared withmainfor plain../paths; dropping only that picture, with a warning, would be friendlier.
1 of 3 tasks
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.
When
fetch_images=True, the EPUB backend rewrites each imagesrcto a path relative to the extraction directory. Two kinds ofsrcwere missed:src="...". Asrc='../Images/map.png'stays relative to the merged document at the archive root, resolves outside the extraction directory, and the conversion fails withPath traversal blocked.srcis a URL, while the archive stores the literal file name.src="../Images/cover%20art.png"points tocover%20art.png, which does not exist, so the image is lost. Manifest hrefs are already decoded the same way (fix(epub): decode percent-escaped manifest hrefs #4199).Changes in
_fix_image_paths:srcin either quote style;unquote;posixpath, as archive paths are POSIX paths. This also avoids backslashes in the rewrittensrcon Windows.The new test builds an EPUB with both cases and checks that both pictures carry image data. It fails on
main.Checklist: