From 1eaf16ffc26c9fec148cbfcb731873b37db42d40 Mon Sep 17 00:00:00 2001 From: Huang Xin Date: Tue, 9 Jun 2026 23:26:47 +0800 Subject: [PATCH] fix(opds): tolerate junk after document element in feeds (#4479) (#4506) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The Hungarian MEK catalog (a PHP backend) returns a valid Atom feed followed by trailing junk after — a stray PHP warning, an extra tag, or text. Chrome's DOMParser ignores it, but Firefox's strict parser fails with "junk after document element" and replaces the whole document with a . The reader then sees a non-feed root, treats the response as HTML, finds no OPDS link, and silently navigates back, so browsing the catalog on Firefox web is broken on nearly every subpage. Add parseOPDSXML(): on a parser error, re-parse the slice from the root element's start tag to its last matching end tag, dropping any leading prolog and trailing junk. If recovery still fails the original error document is returned, so callers fall through to their existing HTML/non-OPDS handling. Wire it into the three OPDS XML parse sites: the reader (page.tsx), validateOPDSURL (adding a catalog), and the subscription/auto-download feed checker. feedChecker also switches its text.startsWith('<') detection to looksLikeXMLContent so the MEK feed's leading newlines (no declaration, #4181) are recognized. jsdom mirrors Firefox's strict behavior (same parsererror namespace), so the regression tests run in the normal unit suite. Co-authored-by: Claude Opus 4.8 (1M context) --- apps/readest-app/.claude/memory/MEMORY.md | 1 + .../memory/opds-firefox-strict-xml-4479.md | 18 +++++ .../src/__tests__/app/opds/opds-utils.test.ts | 73 +++++++++++++++++++ apps/readest-app/src/app/opds/page.tsx | 3 +- .../src/app/opds/utils/opdsUtils.ts | 46 +++++++++++- .../src/services/opds/feedChecker.ts | 13 ++-- 6 files changed, 147 insertions(+), 7 deletions(-) create mode 100644 apps/readest-app/.claude/memory/opds-firefox-strict-xml-4479.md diff --git a/apps/readest-app/.claude/memory/MEMORY.md b/apps/readest-app/.claude/memory/MEMORY.md index fdc93f71..c685915d 100644 --- a/apps/readest-app/.claude/memory/MEMORY.md +++ b/apps/readest-app/.claude/memory/MEMORY.md @@ -41,6 +41,7 @@ - [Window-state sanitizer (#4398)](window-state-sanitize-4398.md) — Windows launch crash (WebView2 0x80070057) from invalid `.window-state.json` (`-32000` minimized sentinel / `0×0`); our plugin already has upstream #253 fix so bad files are stale; defense-in-depth `window-state-sanitizer` plugin registered BEFORE window-state (plugin init = registration order); coord threshold `-16000` (~halfway to the -32000 sentinel; real desktops sit a few thousand px off origin) keeps multi-monitor negatives ## Feature Notes +- [OPDS Firefox strict-XML parse (#4479)](opds-firefox-strict-xml-4479.md) — MEK feed has junk after ``; Firefox DOMParser → `` (silent back-nav), Chrome lenient; `parseOPDSXML` slices root start→last close tag; jsdom mirrors Firefox; wired into page.tsx + validateOPDSURL + feedChecker (latter also #4181 `looksLikeXMLContent` swap) - [Manage Cache + iOS container layout](manage-cache-ios-layout.md) — `'Cache'` base = `Library/Caches/` only (not all of Caches); iOS `Documents/Inbox` cleared too; WebKit cache + tmp out of reach; never touch App Support - [D-pad Navigation](dpad-navigation.md) — Android TV remote / keyboard arrow navigation design, key files, and pitfalls - [Cloudflare Workers WebSocket](cloudflare-workers-websocket.md) — use fetch() Upgrade pattern (not `ws` npm); CF delivers binary frames as Blob (must serialize async decodes) diff --git a/apps/readest-app/.claude/memory/opds-firefox-strict-xml-4479.md b/apps/readest-app/.claude/memory/opds-firefox-strict-xml-4479.md new file mode 100644 index 00000000..5bb9430f --- /dev/null +++ b/apps/readest-app/.claude/memory/opds-firefox-strict-xml-4479.md @@ -0,0 +1,18 @@ +--- +name: opds-firefox-strict-xml-4479 +description: OPDS feeds fail on Firefox but work on Chrome — strict DOMParser parsererror on junk after root; parseOPDSXML recovery +metadata: + node_type: memory + type: project + originSessionId: 9539d003-7df3-4643-99ca-bdee69be1b3f +--- + +#4479 (MEK catalog `bookserver.mek.oszk.hu`, a PHP backend `teljes.php`): OPDS feeds load on Chrome but on Firefox clicking anything shows loading then silently navigates back. Root cause: the server emits a valid Atom feed followed by **trailing junk after ``** (a stray PHP warning / extra tag / text). Chrome's `DOMParser` ignores it; **Firefox's strict parser replaces the WHOLE document with a ``** ("junk after document element" / "text data outside of root node", Mozilla namespace `http://www.mozilla.org/newlayout/xml/parsererror.xml`). The code then sees a non-`feed` root → treats response as HTML → finds no OPDS link → `router.back()`. + +**jsdom mirrors Firefox exactly** (same strict behavior + same parsererror namespace), so this reproduces in vitest — no need for a real browser. Detect via `doc.documentElement.localName === 'parsererror' || doc.getElementsByTagName('parsererror').length > 0`. Leading whitespace before the root is VALID XML (not the issue); trailing non-whitespace is the killer. + +**Fix:** `parseOPDSXML(text)` helper in `src/app/opds/utils/opdsUtils.ts` — parse, and on parser error re-parse the slice from the first element start tag (`/<([A-Za-z_][\w.:-]*)/`) to its last matching `` close tag (drops leading prolog + trailing junk). Returns the original error doc if recovery fails (no regression — falls through to existing HTML branch). Wired into all 3 OPDS XML parse sites: `page.tsx` (reader nav), `validateOPDSURL` (opdsUtils, adding catalog), `feedChecker.ts` (subscriptions/auto-download). feedChecker also had the #4181 detection bug — switched `text.startsWith('<')` → `looksLikeXMLContent(text)` (the MEK feed has ~13 leading newlines + no `` decl), else the parse fix is unreachable there. + +This is the same MEK server family as [[empty-start-cfi-sync]]-style "tolerate broken servers" work; related #4181 = leading-whitespace detection (`looksLikeXMLContent`). + +NOT fixed (separate, out of scope unless asked): #4479 also reports Android **download** failures (acquisition links point to `mek.oszk.hu/.../*.epub`; works in plain browser, red "download failed" toast in app) — distinct from the XML parse issue. diff --git a/apps/readest-app/src/__tests__/app/opds/opds-utils.test.ts b/apps/readest-app/src/__tests__/app/opds/opds-utils.test.ts index a532408f..d274eb5f 100644 --- a/apps/readest-app/src/__tests__/app/opds/opds-utils.test.ts +++ b/apps/readest-app/src/__tests__/app/opds/opds-utils.test.ts @@ -50,6 +50,7 @@ import { resolveURL, getFileExtFromPath, looksLikeXMLContent, + parseOPDSXML, MIME, validateOPDSURL, } from '@/app/opds/utils/opdsUtils'; @@ -395,6 +396,62 @@ describe('opdsUtils', () => { }); }); + // Regression for https://github.com/readest/readest/issues/4479 + // The Hungarian MEK catalog (a PHP backend) emits a valid Atom feed followed + // by trailing junk after . Firefox's strict DOMParser replaces the + // whole document with a ("junk after document element"), while + // Chrome ignores it. jsdom matches Firefox, so these run in the test env. + describe('parseOPDSXML', () => { + const parserError = (doc: Document) => + doc.documentElement?.localName === 'parsererror' || + doc.getElementsByTagName('parsererror').length > 0; + + it('parses a well-formed feed unchanged', () => { + const doc = parseOPDSXML('x'); + expect(parserError(doc)).toBe(false); + expect(doc.documentElement.localName).toBe('feed'); + }); + + it('recovers from text junk after the root element', () => { + const doc = parseOPDSXML( + 'x\nWarning: junk', + ); + expect(parserError(doc)).toBe(false); + expect(doc.documentElement.localName).toBe('feed'); + expect(doc.querySelector('title')?.textContent).toBe('x'); + }); + + it('recovers from a stray tag after the root element', () => { + const doc = parseOPDSXML( + 'x
', + ); + expect(parserError(doc)).toBe(false); + expect(doc.documentElement.localName).toBe('feed'); + }); + + it('recovers from leading whitespace and trailing junk (the MEK shape)', () => { + const doc = parseOPDSXML( + '\n\n\nMEK\n
extra', + ); + expect(parserError(doc)).toBe(false); + expect(doc.documentElement.localName).toBe('feed'); + }); + + it('recovers an entry document with trailing junk', () => { + const doc = parseOPDSXML( + 'book junk', + ); + expect(parserError(doc)).toBe(false); + expect(doc.documentElement.localName).toBe('entry'); + }); + + it('returns the error document when recovery is impossible', () => { + // Unclosed root element — there is no trailing junk to strip. + const doc = parseOPDSXML('x'); + expect(parserError(doc)).toBe(true); + }); + }); + describe('validateOPDSURL', () => { beforeEach(() => { mockFetchWithAuth.mockReset(); @@ -579,6 +636,22 @@ describe('opdsUtils', () => { expect(result.data?.type).toBe('feed'); }); + // Regression for https://github.com/readest/readest/issues/4479 + it('should accept a feed with trailing junk after (Firefox strict parse)', async () => { + const xmlFeed = `\n\n\n MEK\n\nWarning: stray PHP output`; + + mockFetchWithAuth.mockResolvedValue({ + ok: true, + url: 'https://bookserver.mek.oszk.hu/abc/teljes/C/0', + text: () => Promise.resolve(xmlFeed), + headers: new Headers({ 'Content-Type': 'text/html' }), + } as Response); + + const result = await validateOPDSURL('https://bookserver.mek.oszk.hu/abc/teljes/C/0'); + expect(result.isValid).toBe(true); + expect(result.data?.type).toBe('feed'); + }); + it('should return invalid for XML that is not a recognized OPDS document type', async () => { const xmlDoc = ` diff --git a/apps/readest-app/src/app/opds/page.tsx b/apps/readest-app/src/app/opds/page.tsx index ddfe3cc5..d1786dfa 100644 --- a/apps/readest-app/src/app/opds/page.tsx +++ b/apps/readest-app/src/app/opds/page.tsx @@ -31,6 +31,7 @@ import { looksLikeXMLContent, MIME, parseMediaType, + parseOPDSXML, resolveURL, } from './utils/opdsUtils'; import { @@ -247,7 +248,7 @@ export default function BrowserPage() { const text = await res.text(); if (looksLikeXMLContent(text)) { - const doc = new DOMParser().parseFromString(text, MIME.XML as DOMParserSupportedType); + const doc = parseOPDSXML(text); const { documentElement: { localName }, } = doc; diff --git a/apps/readest-app/src/app/opds/utils/opdsUtils.ts b/apps/readest-app/src/app/opds/utils/opdsUtils.ts index 40d2e907..e6e18a6e 100644 --- a/apps/readest-app/src/app/opds/utils/opdsUtils.ts +++ b/apps/readest-app/src/app/opds/utils/opdsUtils.ts @@ -82,6 +82,50 @@ export const parseMediaType = (str?: string) => { */ export const looksLikeXMLContent = (text: string): boolean => text.trimStart().startsWith('<'); +/** + * Detect a DOMParser error document. Strict XML parsers (Firefox, and jsdom in + * tests) replace the whole document with a element on any + * well-formedness violation; Chrome is lenient and often parses on regardless. + */ +const hasXMLParseError = (doc: Document): boolean => + doc.documentElement?.localName === 'parsererror' || + doc.getElementsByTagName('parsererror').length > 0; + +/** + * Parse an OPDS/Atom XML string, tolerating "junk after the document element". + * + * Old OPDS servers (e.g. the Hungarian MEK catalog, issue #4479) emit a valid + * feed followed by trailing junk — a stray PHP warning, an extra tag, or text + * after . Chrome's XML parser ignores it, but Firefox's strict parser + * fails with "junk after document element" / "text data outside of root node" + * and replaces the whole document with a . Callers then see a + * non-feed root, treat the response as HTML, find no OPDS link, and silently + * navigate back. + * + * Recovery: on a parser error, re-parse the slice from the root element's start + * tag to its last matching end tag (dropping any leading prolog and trailing + * junk). If recovery still fails, the original error document is returned so + * callers fall through to their existing HTML/non-OPDS handling. + */ +export const parseOPDSXML = (text: string): Document => { + const doc = new DOMParser().parseFromString(text, MIME.XML as DOMParserSupportedType); + if (!hasXMLParseError(doc)) return doc; + + const rootMatch = text.match(/<([A-Za-z_][\w.:-]*)/); + const rootName = rootMatch?.[1]; + if (rootMatch && rootName !== undefined) { + const startIdx = rootMatch.index ?? 0; + const closeTag = ``; + const closeIdx = text.lastIndexOf(closeTag); + if (closeIdx > startIdx) { + const sliced = text.slice(startIdx, closeIdx + closeTag.length); + const retry = new DOMParser().parseFromString(sliced, MIME.XML as DOMParserSupportedType); + if (!hasXMLParseError(retry)) return retry; + } + } + return doc; +}; + export const isSearchLink = (link: OPDSBaseLink): boolean => { const rels = Array.isArray(link.rel) ? link.rel : [link.rel || '']; return rels.includes('search') && (link.type === MIME.OPENSEARCH || link.type === MIME.ATOM); @@ -146,7 +190,7 @@ export const validateOPDSURL = async ( // Check if it's XML-based OPDS if (looksLikeXMLContent(text)) { - const doc = new DOMParser().parseFromString(text, MIME.XML as DOMParserSupportedType); + const doc = parseOPDSXML(text); const { documentElement: { localName }, } = doc; diff --git a/apps/readest-app/src/services/opds/feedChecker.ts b/apps/readest-app/src/services/opds/feedChecker.ts index 57f5232d..516f683c 100644 --- a/apps/readest-app/src/services/opds/feedChecker.ts +++ b/apps/readest-app/src/services/opds/feedChecker.ts @@ -13,7 +13,12 @@ import { REL } from '@/types/opds'; import { MIMETYPES } from '@/libs/document'; import { isWebAppPlatform } from '@/services/environment'; import { fetchWithAuth } from '@/app/opds/utils/opdsReq'; -import { resolveURL, parseMediaType } from '@/app/opds/utils/opdsUtils'; +import { + resolveURL, + parseMediaType, + looksLikeXMLContent, + parseOPDSXML, +} from '@/app/opds/utils/opdsUtils'; import { normalizeOPDSCustomHeaders } from '@/app/opds/utils/customHeaders'; import type { OPDSSubscriptionState, PendingItem } from './types'; import { MAX_PAGES_PER_FEED } from './types'; @@ -29,8 +34,6 @@ const NEWNESS_TITLE_RE = const NEWNESS_HREF_RE = /(?:sort_order=release_date|sort=(?:new|date|added|date_added|recent|release_date|release_date_desc)|\b(?:new[-_]?releases?|newest|recently[-_]?added|by[-_]?date)\b|\/new(?:[/?#]|$))/i; -const MIME_XML = 'application/xml'; - // Acquisition rels safe for unattended download. Excludes buy / borrow / // subscribe / sample (need user action) and indirect (link points to a // landing document, not the file). @@ -224,8 +227,8 @@ async function fetchFeed( const responseURL = res.url; const text = await res.text(); - if (text.startsWith('<')) { - const doc = new DOMParser().parseFromString(text, MIME_XML as DOMParserSupportedType); + if (looksLikeXMLContent(text)) { + const doc = parseOPDSXML(text); const { localName } = doc.documentElement; if (localName === 'feed') {