From 3ec658ccf57eae98b1f5a7b2d5a52f66a747c567 Mon Sep 17 00:00:00 2001 From: Sushant Lokhande Date: Tue, 8 Sep 2026 00:29:27 -0700 Subject: [PATCH] fix(epub): resolve percent-encoded manifest hrefs to ZIP entries Manifest hrefs are URI references, so a chapter stored as `chapter 1.xhtml` appears in content.opf as `chapter%201.xhtml`. ZIP entry names are not URI-encoded, so joining the href to the OPF's directory verbatim produced `OEBPS/chapter%201.xhtml`, which is not a member of the archive. The spine loop skips anything missing from `z.namelist()`, so the chapter was dropped with no error: any EPUB whose filenames contain a space or a non-ASCII character lost that content silently. Decode the href and normalise the joined path before matching. The raw href is kept as a fallback candidate, so an archive that stores a literally-encoded name still resolves exactly as it did before. Normalising also lets an href reach outside the OPF's own directory, which the old string concatenation could not express. Adds tests for a space, a non-ASCII name, a literally-encoded entry, and a parent-relative href. The first, second and fourth fail without this change; the third passes either way and guards against regressing today's behaviour. --- .../markitdown/converters/_epub_converter.py | 30 ++++- .../markitdown/tests/test_epub_converter.py | 115 ++++++++++++++++++ 2 files changed, 143 insertions(+), 2 deletions(-) create mode 100644 packages/markitdown/tests/test_epub_converter.py diff --git a/packages/markitdown/src/markitdown/converters/_epub_converter.py b/packages/markitdown/src/markitdown/converters/_epub_converter.py index 2ba0b08009..e394d3ecf1 100644 --- a/packages/markitdown/src/markitdown/converters/_epub_converter.py +++ b/packages/markitdown/src/markitdown/converters/_epub_converter.py @@ -1,9 +1,11 @@ import os +import posixpath import zipfile +from urllib.parse import unquote from defusedxml import minidom from xml.dom.minidom import Document -from typing import BinaryIO, Any, Dict, List +from typing import BinaryIO, Any, Dict, List, Set from ._html_converter import HtmlConverter from .._base_converter import DocumentConverterResult @@ -91,8 +93,9 @@ def convert( base_path = "/".join( opf_path.split("/")[:-1] ) # Get base directory of content.opf + zip_names = set(z.namelist()) spine = [ - f"{base_path}/{manifest[item_id]}" if base_path else manifest[item_id] + self._resolve_manifest_href(manifest[item_id], base_path, zip_names) for item_id in spine_order if item_id in manifest ] @@ -129,6 +132,29 @@ def convert( markdown="\n\n".join(markdown_content), title=metadata["title"] ) + def _resolve_manifest_href( + self, href: str, base_path: str, zip_names: Set[str] + ) -> str: + """Resolve a manifest href to the matching ZIP entry name. + + Manifest hrefs are URI references relative to the OPF, so reserved + characters such as spaces arrive percent-encoded, while ZIP entry names + are not encoded. Prefer the decoded form, but fall back to the raw href + so archives that store a literally-encoded name still resolve. + """ + candidates: List[str] = [] + for candidate in (unquote(href), href): + resolved = posixpath.join(base_path, candidate) if base_path else candidate + resolved = posixpath.normpath(resolved) + if resolved not in candidates: + candidates.append(resolved) + + for candidate in candidates: + if candidate in zip_names: + return candidate + + return candidates[0] + def _get_text_from_node(self, dom: Document, tag_name: str) -> str | None: """Convenience function to extract a single occurrence of a tag (e.g., title).""" texts = self._get_all_texts_from_nodes(dom, tag_name) diff --git a/packages/markitdown/tests/test_epub_converter.py b/packages/markitdown/tests/test_epub_converter.py new file mode 100644 index 0000000000..ac8edd3202 --- /dev/null +++ b/packages/markitdown/tests/test_epub_converter.py @@ -0,0 +1,115 @@ +import io +import zipfile + +from markitdown import StreamInfo +from markitdown.converters import EpubConverter + +CONTAINER_XML = """ + + + + + +""" + +CHAPTER_XHTML = """ +

{title}

{body}

+ +""" + + +def _build_epub(manifest_items, spine_ids, documents) -> io.BytesIO: + """Assemble a minimal EPUB from manifest entries and ZIP member names.""" + manifest = "\n".join( + f'' + for item_id, href in manifest_items + ) + spine = "\n".join(f'' for item_id in spine_ids) + opf = f""" + + + Encoded Hrefs + + {manifest} + {spine} + +""" + + buffer = io.BytesIO() + with zipfile.ZipFile(buffer, "w") as z: + z.writestr("mimetype", "application/epub+zip") + z.writestr("META-INF/container.xml", CONTAINER_XML) + z.writestr("OEBPS/content.opf", opf) + for name, (title, body) in documents.items(): + z.writestr(name, CHAPTER_XHTML.format(title=title, body=body)) + buffer.seek(0) + return buffer + + +def _convert(stream: io.BytesIO) -> str: + result = EpubConverter().convert( + stream, StreamInfo(mimetype="application/epub+zip", extension=".epub") + ) + # markdownify escapes underscores, so compare against unescaped text + return result.markdown.replace("\\", "") + + +def test_percent_encoded_href_resolves_to_zip_entry() -> None: + """A space in a filename arrives percent-encoded in the manifest href.""" + stream = _build_epub( + manifest_items=[("c1", "chapter%201.xhtml"), ("c2", "plain.xhtml")], + spine_ids=["c1", "c2"], + documents={ + "OEBPS/chapter 1.xhtml": ("First", "SPACED_BODY"), + "OEBPS/plain.xhtml": ("Second", "PLAIN_BODY"), + }, + ) + + markdown = _convert(stream) + + assert "SPACED_BODY" in markdown, "percent-encoded href must resolve to its entry" + assert "PLAIN_BODY" in markdown, "unencoded hrefs must keep working" + assert markdown.index("SPACED_BODY") < markdown.index( + "PLAIN_BODY" + ), "spine order is preserved" + + +def test_non_ascii_percent_encoded_href_resolves() -> None: + """Non-ASCII filenames are percent-encoded UTF-8 in the manifest href.""" + stream = _build_epub( + manifest_items=[("c1", "cap%C3%ADtulo.xhtml")], + spine_ids=["c1"], + documents={"OEBPS/capítulo.xhtml": ("Capítulo", "ACCENTED_BODY")}, + ) + + assert "ACCENTED_BODY" in _convert(stream) + + +def test_literally_encoded_zip_entry_still_resolves() -> None: + """An archive storing the encoded name verbatim keeps working.""" + stream = _build_epub( + manifest_items=[("c1", "chapter%201.xhtml")], + spine_ids=["c1"], + documents={"OEBPS/chapter%201.xhtml": ("Literal", "LITERAL_BODY")}, + ) + + assert "LITERAL_BODY" in _convert(stream) + + +def test_parent_relative_href_resolves() -> None: + """Hrefs may point outside the OPF's own directory.""" + stream = _build_epub( + manifest_items=[("c1", "../shared/chapter.xhtml")], + spine_ids=["c1"], + documents={"shared/chapter.xhtml": ("Shared", "SHARED_BODY")}, + ) + + assert "SHARED_BODY" in _convert(stream) + + +if __name__ == "__main__": + test_percent_encoded_href_resolves_to_zip_entry() + test_non_ascii_percent_encoded_href_resolves() + test_literally_encoded_zip_entry_still_resolves() + test_parent_relative_href_resolves() + print("All tests passed")