epub2pdf: title-based export names, .pdf download headers, security hardening (68 tests green)
This commit is contained in:
BMad Master
@@ -0,0 +1,460 @@
|
||||
"""Focused regression tests for the 2026-08-29 multi-lens review fixes.
|
||||
|
||||
Source of truth: .a0proj/_bmad-output/test-artifacts/review-findings-20260829.json
|
||||
Each test maps to a finding (lens + location); guard behavior is asserted
|
||||
through the public HTTP API wherever possible.
|
||||
"""
|
||||
import io
|
||||
import os
|
||||
import re
|
||||
import subprocess
|
||||
import sys
|
||||
import threading
|
||||
import time
|
||||
import zipfile
|
||||
from pathlib import Path
|
||||
|
||||
import pytest
|
||||
|
||||
import app
|
||||
from tests.helpers import (
|
||||
_CONTAINER_XML,
|
||||
download_bytes,
|
||||
make_epub_zip,
|
||||
pdf_text,
|
||||
wait_for_job,
|
||||
)
|
||||
|
||||
ROOT = Path(__file__).resolve().parents[1]
|
||||
|
||||
|
||||
def _zip_with(items: dict) -> bytes:
|
||||
buf = io.BytesIO()
|
||||
with zipfile.ZipFile(buf, "w", zipfile.ZIP_DEFLATED) as zf:
|
||||
for name, data in items.items():
|
||||
if isinstance(data, str):
|
||||
data = data.encode("utf-8")
|
||||
zf.writestr(name, data)
|
||||
return buf.getvalue()
|
||||
|
||||
|
||||
def _append_member(epub_bytes, name, data=b"x", external_attr=None):
|
||||
src = io.BytesIO(epub_bytes)
|
||||
out = io.BytesIO()
|
||||
with zipfile.ZipFile(src, "r") as zin, zipfile.ZipFile(out, "w", zipfile.ZIP_DEFLATED) as zout:
|
||||
for item in zin.namelist():
|
||||
zout.writestr(item, zin.read(item))
|
||||
zi = zipfile.ZipInfo(name)
|
||||
if external_attr is not None:
|
||||
zi.external_attr = external_attr
|
||||
zout.writestr(zi, data)
|
||||
return out.getvalue()
|
||||
|
||||
|
||||
def _upload(client, content, filename="book.epub"):
|
||||
"""POST an upload, wait for the terminal job state, return the snapshot."""
|
||||
files = {"file": (io.BytesIO(content), filename)}
|
||||
resp = client.post("/api/convert", data=files, content_type="multipart/form-data")
|
||||
assert resp.status_code == 202, resp.get_json()
|
||||
return wait_for_job(client, resp.get_json()["job_id"])
|
||||
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# Fix 1: ZIP-SLIP + ZIP BOMB (adversarial, app.py extractall)
|
||||
# ---------------------------------------------------------------------------
|
||||
|
||||
def test_zip_slip_dotdot_member_rejected(client, fixture_epub_bytes):
|
||||
job = _upload(client, _append_member(fixture_epub_bytes, "../pwned.txt"))
|
||||
assert job["status"] == "error", job
|
||||
assert "unsafe zip member" in job["error"]
|
||||
|
||||
|
||||
def test_zip_symlink_member_rejected(client, fixture_epub_bytes):
|
||||
content = _append_member(
|
||||
fixture_epub_bytes, "OEBPS/link", external_attr=(0xA << 28) | 0o777
|
||||
)
|
||||
job = _upload(client, content)
|
||||
assert job["status"] == "error", job
|
||||
assert "unsafe zip member" in job["error"]
|
||||
|
||||
|
||||
def test_zip_bomb_guard_rejects_huge_expansion(client, fixture_epub_bytes, monkeypatch):
|
||||
# Production floor is 2 GiB (impractical in a unit test); shrink the floor
|
||||
# so the guard logic is exercised with a 4 MiB member in a ~8 KiB zip.
|
||||
monkeypatch.setattr(app, "ZIP_BOMB_MIN_CAP", 1024 * 1024)
|
||||
content = _append_member(fixture_epub_bytes, "OEBPS/bomb.bin", b"\x00" * (4 * 1024 * 1024))
|
||||
job = _upload(client, content)
|
||||
assert job["status"] == "error", job
|
||||
assert "zip-bomb" in job["error"].lower()
|
||||
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# Fix 2: UNBOUNDED THREADS -> bounded pool + 503 backpressure
|
||||
# (adversarial, app.py:293-307 / 355)
|
||||
# ---------------------------------------------------------------------------
|
||||
|
||||
def test_saturated_pool_returns_503_json(client, fixture_epub_bytes, monkeypatch):
|
||||
monkeypatch.setattr(app, "WORKER_SLOTS", threading.BoundedSemaphore(0))
|
||||
files = {"file": (io.BytesIO(fixture_epub_bytes), "book.epub")}
|
||||
resp = client.post("/api/convert", data=files, content_type="multipart/form-data")
|
||||
assert resp.status_code == 503
|
||||
body = resp.get_json()
|
||||
assert body is not None, "503 body must be JSON so client pollers never hang"
|
||||
assert "busy" in body["error"].lower()
|
||||
|
||||
|
||||
def test_worker_slot_released_after_job_finishes(client, submit_epub):
|
||||
before = app.WORKER_SLOTS._value
|
||||
job = wait_for_job(client, submit_epub().get_json()["job_id"])
|
||||
assert job["status"] == "done"
|
||||
assert app.WORKER_SLOTS._value == before, "slot leaked: semaphore not released"
|
||||
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# Fix 3: OUTPUT NAME COLLISION + TORN FILE (adversarial, app.py:274-287)
|
||||
# ---------------------------------------------------------------------------
|
||||
|
||||
def _clean_output(pattern: str) -> None:
|
||||
"""Remove output-dir files matching pattern so naming tests are deterministic."""
|
||||
for f in app.OUTPUT_DIR.glob(pattern):
|
||||
f.unlink()
|
||||
|
||||
|
||||
def _synthetic_epub_bytes(title=None) -> bytes:
|
||||
"""Minimal renderable EPUB; title=None omits <dc:title>, '' makes it empty."""
|
||||
title_tag = "" if title is None else f"<dc:title>{title}</dc:title>"
|
||||
opf = (
|
||||
'<?xml version="1.0" encoding="utf-8"?>'
|
||||
'<package xmlns="http://www.idpf.org/2007/opf" version="3.0" '
|
||||
'unique-identifier="uid">'
|
||||
'<metadata xmlns:dc="http://purl.org/dc/elements/1.1">'
|
||||
f"{title_tag}<dc:creator>Test</dc:creator>"
|
||||
"</metadata>"
|
||||
'<manifest><item id="ch1" href="chapter1.xhtml" '
|
||||
'media-type="application/xhtml+xml"/></manifest>'
|
||||
'<spine><itemref idref="ch1"/></spine>'
|
||||
"</package>"
|
||||
)
|
||||
buf = io.BytesIO()
|
||||
with zipfile.ZipFile(buf, "w") as zf:
|
||||
zf.writestr("META-INF/container.xml", _CONTAINER_XML)
|
||||
zf.writestr("OEBPS/book.opf", opf)
|
||||
zf.writestr(
|
||||
"OEBPS/chapter1.xhtml",
|
||||
'<?xml version="1.0" encoding="utf-8"?>'
|
||||
"<html><body><p>naming body</p></body></html>",
|
||||
)
|
||||
return buf.getvalue()
|
||||
|
||||
|
||||
def test_output_filename_unique_per_job(client, submit_epub):
|
||||
# New contract (export naming): clean title name first, -2 suffix on collision
|
||||
_clean_output("Structured Test Book*.pdf")
|
||||
job1 = wait_for_job(client, submit_epub(filename="book.epub").get_json()["job_id"])
|
||||
job2 = wait_for_job(client, submit_epub(filename="book.epub").get_json()["job_id"])
|
||||
assert job1["status"] == "done" and job2["status"] == "done"
|
||||
assert job1["filename"] != job2["filename"], "same book must not overwrite its PDF"
|
||||
assert job2["filename"].endswith("-2.pdf"), "second conversion must append -2"
|
||||
assert (app.OUTPUT_DIR / job1["filename"]).is_file()
|
||||
assert (app.OUTPUT_DIR / job2["filename"]).is_file()
|
||||
|
||||
|
||||
def test_atomic_write_leaves_no_partial_files(client, submit_epub):
|
||||
job = wait_for_job(client, submit_epub().get_json()["job_id"])
|
||||
assert job["status"] == "done"
|
||||
assert not list(app.OUTPUT_DIR.glob("*.part")), "torn-write .part temp file left behind"
|
||||
|
||||
|
||||
def test_export_name_is_book_title(client, submit_epub):
|
||||
# req 1: export name is the clean book title, not the upload name
|
||||
_clean_output("Structured Test Book*.pdf")
|
||||
job = wait_for_job(
|
||||
client, submit_epub(filename="weird-upload-name.epub").get_json()["job_id"]
|
||||
)
|
||||
assert job["status"] == "done", job
|
||||
assert job["filename"] == "Structured Test Book.pdf"
|
||||
|
||||
|
||||
def test_export_name_falls_back_to_epub_stem(client, submit_epub):
|
||||
# req 1 fallback: missing <dc:title> -> uploaded .epub stem
|
||||
_clean_output("pg2680-images-3*.pdf")
|
||||
job = wait_for_job(
|
||||
client,
|
||||
submit_epub(
|
||||
filename="pg2680-images-3.epub",
|
||||
content=_synthetic_epub_bytes(title=None),
|
||||
).get_json()["job_id"],
|
||||
)
|
||||
assert job["status"] == "done", job
|
||||
assert job["filename"] == "pg2680-images-3.pdf", job["filename"]
|
||||
# req 1 fallback: empty <dc:title></dc:title> -> same stem fallback
|
||||
_clean_output("pg2680-images-4*.pdf")
|
||||
job2 = wait_for_job(
|
||||
client,
|
||||
submit_epub(
|
||||
filename="pg2680-images-4.epub",
|
||||
content=_synthetic_epub_bytes(title=""),
|
||||
).get_json()["job_id"],
|
||||
)
|
||||
assert job2["status"] == "done", job2
|
||||
assert job2["filename"] == "pg2680-images-4.pdf", job2["filename"]
|
||||
|
||||
|
||||
def test_export_name_pdf_title_not_doubled(client, submit_epub):
|
||||
# req 2: title ending in .pdf (case-insensitive) must not double the extension
|
||||
_clean_output("My Book*.pdf")
|
||||
job = wait_for_job(
|
||||
client,
|
||||
submit_epub(
|
||||
filename="seed.epub",
|
||||
content=_synthetic_epub_bytes(title="My Book.PDF"),
|
||||
).get_json()["job_id"],
|
||||
)
|
||||
assert job["status"] == "done", job
|
||||
assert job["filename"] == "My Book.pdf", job["filename"]
|
||||
assert not job["filename"].endswith(".pdf.pdf")
|
||||
|
||||
|
||||
def test_download_headers_pdf_type_and_disposition(client, submit_epub):
|
||||
# req 4: Content-Type application/pdf + attachment filename ending in .pdf
|
||||
_clean_output("Structured Test Book*.pdf")
|
||||
job = wait_for_job(client, submit_epub(filename="book.epub").get_json()["job_id"])
|
||||
assert job["status"] == "done", job
|
||||
resp = client.get(f"/download/{job['filename']}")
|
||||
assert resp.status_code == 200
|
||||
assert resp.headers["Content-Type"].startswith("application/pdf")
|
||||
cd = resp.headers.get("Content-Disposition", "")
|
||||
assert "attachment" in cd
|
||||
assert cd.split("filename")[-1].strip().strip('"').endswith(".pdf")
|
||||
|
||||
|
||||
def test_download_defensively_appends_pdf_suffix(client):
|
||||
# req 4: non-.pdf file served by /download gets a .pdf download_name
|
||||
target = app.OUTPUT_DIR / "note.txt"
|
||||
target.write_bytes(b"%PDF-1.4 fake")
|
||||
try:
|
||||
resp = client.get("/download/note.txt")
|
||||
assert resp.status_code == 200
|
||||
assert resp.headers["Content-Type"].startswith("application/pdf")
|
||||
assert resp.headers["Content-Disposition"].split("filename")[-1].strip().strip('"').endswith(".pdf")
|
||||
finally:
|
||||
target.unlink(missing_ok=True)
|
||||
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# Fix 4: EVICTION KILLS IN-FLIGHT JOBS (adversarial app.py:69-73 +
|
||||
# verification-gap index.html:253-279)
|
||||
# ---------------------------------------------------------------------------
|
||||
|
||||
def test_eviction_never_touches_queued_or_working_jobs():
|
||||
now = time.time()
|
||||
ids = []
|
||||
for i in range(52):
|
||||
jid = app._register_job(f"j{i}.epub")
|
||||
ids.append(jid)
|
||||
app.JOBS[jid]["created"] = now - (2000 - i)
|
||||
app.JOBS[jid]["status"] = "done"
|
||||
# At the 52nd registration 51 jobs were terminal -> oldest (j0) evicted.
|
||||
assert ids[0] not in app.JOBS
|
||||
# Make the oldest *remaining* job in-flight, then force another eviction
|
||||
# pass with two more terminal jobs.
|
||||
app.JOBS[ids[1]]["status"] = "working"
|
||||
for i in range(2):
|
||||
jid = app._register_job(f"extra{i}.epub")
|
||||
app.JOBS[jid]["created"] = now + 100 + i
|
||||
app.JOBS[jid]["status"] = "done"
|
||||
assert app.JOBS[ids[1]]["status"] == "working", "in-flight job must survive eviction"
|
||||
assert ids[1] in app.JOBS
|
||||
|
||||
|
||||
def test_evicted_job_yields_404_unknown_job_body(client):
|
||||
"""The exact body the rewritten UI poll loop receives on a 404."""
|
||||
now = time.time()
|
||||
first = None
|
||||
for i in range(52):
|
||||
jid = app._register_job(f"t{i}.epub")
|
||||
if i == 0:
|
||||
first = jid
|
||||
app.JOBS[jid]["created"] = now - (1000 - i)
|
||||
app.JOBS[jid]["status"] = "done"
|
||||
resp = client.get(f"/api/jobs/{first}")
|
||||
assert resp.status_code == 404
|
||||
assert resp.get_json() == {"error": "unknown job"}
|
||||
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# Fix 5: 0-PAGE + LEAKS + FAKE HEALTH (adversarial app.py:278-288 / 347-352)
|
||||
# ---------------------------------------------------------------------------
|
||||
|
||||
def test_zero_page_book_errors_instead_of_blank_pdf(client):
|
||||
# Spine item is html but the file is absent -> every chunk is skipped ->
|
||||
# no pages. Must be a job error, not a blank-PDF success.
|
||||
content = make_epub_zip(
|
||||
items=[("ch1", "chapter1.xhtml", "application/xhtml+xml", "")],
|
||||
spine_ids=["ch1"],
|
||||
)
|
||||
job = _upload(client, content, "ghost.epub")
|
||||
assert job["status"] == "error", job
|
||||
assert "no renderable content" in job["error"]
|
||||
|
||||
|
||||
def test_upload_unlinked_after_successful_job(client, submit_epub):
|
||||
jid = submit_epub(filename="book.epub").get_json()["job_id"]
|
||||
job = wait_for_job(client, jid)
|
||||
assert job["status"] == "done"
|
||||
assert not (app.UPLOAD_DIR / f"{jid}.epub").exists(), "upload leaked after success"
|
||||
|
||||
|
||||
def test_health_503_when_cjk_fonts_missing(client, monkeypatch):
|
||||
monkeypatch.setattr(app, "_cjk_fonts_present", lambda: False)
|
||||
resp = client.get("/health")
|
||||
assert resp.status_code == 503
|
||||
body = resp.get_json()
|
||||
assert body["status"] == "degraded"
|
||||
assert any("CJK" in p for p in body["problems"])
|
||||
|
||||
|
||||
def test_health_503_when_data_dir_not_writable(client, monkeypatch, tmp_path):
|
||||
monkeypatch.setattr(app, "DATA_DIR", tmp_path / "gone") # nonexistent dir
|
||||
resp = client.get("/health")
|
||||
assert resp.status_code == 503
|
||||
assert any("writable" in p for p in resp.get_json()["problems"])
|
||||
|
||||
|
||||
def test_health_exposes_max_upload_mb_single_source(client, monkeypatch):
|
||||
monkeypatch.setattr(app, "MAX_UPLOAD_MB", 77)
|
||||
body = client.get("/health").get_json()
|
||||
assert body["max_upload_mb"] == 77
|
||||
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# Fix 6: CHARSET (adversarial app.py:262) — legacy windows-1252/latin-1 books
|
||||
# ---------------------------------------------------------------------------
|
||||
|
||||
def test_charset_windows_1252_xml_declaration_honored(client):
|
||||
chap = (
|
||||
'<?xml version="1.0" encoding="windows-1252"?>\n'
|
||||
"<html><body><p>caf\xe9 \u2014 em dash test</p></body></html>"
|
||||
)
|
||||
base = make_epub_zip(
|
||||
items=[("ch1", "ch1.xhtml", "application/xhtml+xml", "")],
|
||||
spine_ids=["ch1"],
|
||||
)
|
||||
content = _append_member(base, "OEBPS/ch1.xhtml", chap.encode("windows-1252"))
|
||||
job = _upload(client, content, "legacy.epub")
|
||||
assert job["status"] == "done", job
|
||||
full = pdf_text(download_bytes(client, job["filename"]))
|
||||
assert "\u2014" in full, "em dash lost: declared windows-1252 encoding not honored"
|
||||
assert "café" in full, "latin-1 mojibake: windows-1252 not honored"
|
||||
|
||||
|
||||
def test_decode_html_meta_charset_latin1():
|
||||
raw = b'<html><head><meta charset="latin-1"></head><body>caf\xe9</body></html>'
|
||||
assert app._decode_html(raw) == "café"
|
||||
|
||||
|
||||
def test_decode_html_falls_back_to_utf8_replace():
|
||||
raw = b"<html><body>" + b"\xff\xfe\xfa" + b"</body></html>"
|
||||
assert app._decode_html(raw), "undecodable chapter must not raise"
|
||||
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# Fix 7: LAUNCHER DIVERGENCE + LIMIT DRY (verification-gap start.sh:25-38 /
|
||||
# Dockerfile:37 + adversarial index.html:139)
|
||||
# ---------------------------------------------------------------------------
|
||||
|
||||
def test_launcher_flags_aligned_single_source():
|
||||
start = (ROOT / "start.sh").read_text(encoding="utf-8")
|
||||
docker = (ROOT / "Dockerfile").read_text(encoding="utf-8")
|
||||
for text, label in ((start, "start.sh"), (docker, "Dockerfile")):
|
||||
assert "--workers 1" in text, f"{label}: single worker required (in-memory JOBS)"
|
||||
assert "--timeout 1200" in text, f"{label}: 1200s timeout for long conversions"
|
||||
assert "--threads 8" in text or "THREADS:-8" in text, f"{label}: 8 threads"
|
||||
|
||||
|
||||
def test_ui_consumes_max_upload_mb_from_health(client):
|
||||
html = client.get("/").get_data(as_text=True)
|
||||
assert "max_upload_mb" in html, "UI must read the upload limit from /health"
|
||||
assert 'id="maxmb"' in html
|
||||
|
||||
|
||||
def test_ui_poll_loop_treats_404_as_terminal(client):
|
||||
html = client.get("/").get_data(as_text=True)
|
||||
assert "status === 404" in html, "poll loop must treat a 404 /api/jobs as terminal"
|
||||
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# Cheap edge-case findings folded into the same code paths
|
||||
# ---------------------------------------------------------------------------
|
||||
|
||||
def test_non_numeric_port_env_exits_2():
|
||||
env = dict(os.environ)
|
||||
env["PORT"] = "not-a-port"
|
||||
proc = subprocess.run(
|
||||
[sys.executable, str(ROOT / "app.py")],
|
||||
capture_output=True, text=True, timeout=60, env=env, cwd=str(ROOT),
|
||||
)
|
||||
assert proc.returncode == 2
|
||||
assert "PORT must be an integer" in proc.stdout
|
||||
|
||||
|
||||
def test_contained_rejects_escaping_path(tmp_path):
|
||||
with pytest.raises(ValueError, match="escapes"):
|
||||
app._contained(tmp_path, "../escape.txt")
|
||||
|
||||
|
||||
def test_contained_accepts_inner_path(tmp_path):
|
||||
assert app._contained(tmp_path, "a/b.txt") == (tmp_path / "a" / "b.txt").resolve()
|
||||
|
||||
|
||||
_OPF_TMPL = (
|
||||
'<?xml version="1.0" encoding="utf-8"?>'
|
||||
'<package xmlns="http://www.idpf.org/2007/opf" version="3.0" unique-identifier="uid">'
|
||||
'<metadata xmlns:dc="http://purl.org/dc/elements/1.1/">'
|
||||
"<dc:title>T</dc:title></metadata>"
|
||||
'<manifest><item id="ch1" href="{href}" media-type="application/xhtml+xml"/></manifest>'
|
||||
'<spine><itemref idref="ch1"/></spine></package>'
|
||||
)
|
||||
|
||||
|
||||
def test_opf_path_escaping_tree_rejected(client):
|
||||
container = _CONTAINER_XML.replace('full-path="OEBPS/book.opf"', 'full-path="../book.opf"')
|
||||
content = _zip_with({
|
||||
"META-INF/container.xml": container,
|
||||
"OEBPS/book.opf": _OPF_TMPL.format(href="ch1.xhtml"),
|
||||
"OEBPS/ch1.xhtml": "<html><body>hi</body></html>",
|
||||
})
|
||||
job = _upload(client, content, "escape.epub")
|
||||
assert job["status"] == "error", job
|
||||
assert "escapes" in job["error"]
|
||||
|
||||
|
||||
def test_spine_href_escaping_tree_skipped(client):
|
||||
content = _zip_with({
|
||||
"META-INF/container.xml": _CONTAINER_XML,
|
||||
"OEBPS/book.opf": _OPF_TMPL.format(href="../../etc/passwd"),
|
||||
"OEBPS/ch1.xhtml": "<html><body>hi</body></html>",
|
||||
})
|
||||
job = _upload(client, content, "href.epub")
|
||||
assert job["status"] == "error", job
|
||||
assert "no renderable content" in job["error"]
|
||||
|
||||
|
||||
def test_startup_sweep_removes_only_stale_work_dirs(tmp_path, monkeypatch):
|
||||
monkeypatch.setattr(app, "WORK_DIR", tmp_path)
|
||||
stale = tmp_path / "job-old"
|
||||
stale.mkdir()
|
||||
fresh = tmp_path / "job-new"
|
||||
fresh.mkdir()
|
||||
old = time.time() - 7 * 3600
|
||||
os.utime(stale, (old, old))
|
||||
app._sweep_stale_work_dirs()
|
||||
assert not stale.exists(), "stale work dir must be swept"
|
||||
assert fresh.exists(), "fresh work dir must be kept"
|
||||
|
||||
|
||||
def test_progress_message_contract_matches_ui_regex():
|
||||
"""Verification-gap finding: the message contract the UI poll loop parses."""
|
||||
m = re.search(r"\((\d+)/(\d+)\)", app._format_progress(3, 9, "ch1.xhtml"))
|
||||
assert m is not None
|
||||
assert (m.group(1), m.group(2)) == ("3", "9")
|
||||
Reference in New Issue
Block a user