fix(opds): render HTML in publication descriptions (#4510)

* fix(opds): render HTML in publication descriptions, closes #4503 OPDS publication descriptions showed raw HTML tags (literal `<p>`, `"`, `'`) instead of rendering them. Some aggregator feeds serve the description as an Atom `type="text"` summary whose HTML has been escaped twice; foliate's getContent only un-escapes `type="html"`/ `"xhtml"`, so the markup survives parsing as entity text and the detail view dumped it straight into an unsanitized `dangerouslySetInnerHTML` (also an XSS sink for untrusted feed content). Add `getOPDSDescriptionHtml`: decode one extra entity level only when the value is entirely escaped markup (mixed content like `<p>see <code>` is left literal), then sanitize with the shared DOMPurify sanitizer. Wire it into PublicationView and render the sanitized HTML. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * refactor: consolidate HTML sanitizers into @/utils/sanitize sanitizeHtml/sanitizeForParsing are generic DOMPurify wrappers, not specific to Send-to-Readest. Now that OPDS description rendering also needs sanitizeHtml, move them out of services/send/conversion into the shared @/utils/sanitize module (alongside sanitizeString) so neither consumer reaches across the other's feature boundary. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
2026-06-10 12:52:34 +08:00
parent 75dc2e4e81
commit 8425d0b91f
8 changed files with 236 additions and 82 deletions
@@ -0,0 +1,117 @@
+import { describe, it, expect } from 'vitest';
+import { getPublication } from 'foliate-js/opds.js';
+import { getOPDSDescriptionHtml } from '@/app/opds/utils/opdsContent';
+import { SYMBOL, type OPDSPublication } from '@/types/opds';
+
+// Render an HTML string the way `dangerouslySetInnerHTML` would and return the
+// visible text, so a test can tell "renders as markup" from "shows raw tags".
+const renderedText = (html: string): string => {
+  const el = document.createElement('div');
+  el.innerHTML = html;
+  return el.textContent ?? '';
+};
+
+const parsePublication = (entryInner: string): OPDSPublication => {
+  const xml = `<entry xmlns="http://www.w3.org/2005/Atom"><title>T</title>${entryInner}</entry>`;
+  const doc = new DOMParser().parseFromString(xml, 'application/xml');
+  return getPublication(doc.documentElement) as OPDSPublication;
+};
+
+describe('getOPDSDescriptionHtml', () => {
+  it('returns empty string for missing content', () => {
+    expect(getOPDSDescriptionHtml(undefined)).toBe('');
+    expect(getOPDSDescriptionHtml({ value: '', type: 'text' })).toBe('');
+  });
+
+  it('renders real (single-escaped) HTML in a text summary as markup', () => {
+    // <summary type="text">&lt;p&gt;Hi &amp;quot;q&amp;quot;&lt;/p&gt;</summary>
+    const content = { value: '<p>Hi &quot;q&quot;</p>', type: 'text' as const };
+    const html = getOPDSDescriptionHtml(content);
+    expect(html).toContain('<p>');
+    expect(renderedText(html)).toBe('Hi "q"');
+  });
+
+  it('renders HTML for type="html" content', () => {
+    const content = { value: '<p>Hello <strong>world</strong></p>', type: 'html' as const };
+    const html = getOPDSDescriptionHtml(content);
+    expect(html).toContain('<strong>');
+    expect(renderedText(html)).toBe('Hello world');
+  });
+
+  it('renders xhtml content and unwraps the namespaced wrapper div', () => {
+    const content = {
+      value: '<div xmlns="http://www.w3.org/1999/xhtml"><p>Hi</p></div>',
+      type: 'xhtml' as const,
+    };
+    const html = getOPDSDescriptionHtml(content);
+    expect(renderedText(html)).toBe('Hi');
+  });
+
+  // The bug: issue #4503. An aggregator feed serves the description as a
+  // type="text" summary whose HTML has been escaped twice, so it survives
+  // parsing as entity *text* ("&lt;p&gt;...") and showed literal tags.
+  it('decodes double-escaped HTML so it renders instead of showing raw tags', () => {
+    const content = {
+      value:
+        '&lt;p&gt;Creators&lt;/p&gt;&lt;p&gt;&amp;quot;Wall&amp;quot; Sollenar&amp;#x27;s&lt;/p&gt;',
+      type: 'text' as const,
+    };
+    const html = getOPDSDescriptionHtml(content);
+    // No literal tag/entity text should remain visible to the user.
+    const text = renderedText(html);
+    expect(text).not.toContain('<p>');
+    expect(text).not.toContain('&quot;');
+    expect(text).not.toContain('&#x27;');
+    expect(text).toContain('Creators');
+    expect(text).toContain('"Wall"');
+    expect(text).toContain("Sollenar's");
+    // The markup itself contains real paragraph elements.
+    expect(html).toContain('<p>');
+  });
+
+  it('end-to-end: a double-escaped Atom entry (issue #4503) renders as HTML', () => {
+    const pub = parsePublication(
+      '<summary>&amp;lt;p&amp;gt;Creators: Algis Budrys&amp;lt;/p&amp;gt;' +
+        '&amp;lt;p&amp;gt;&amp;amp;quot;Wall of Crystal&amp;amp;quot; by Budrys&amp;lt;/p&amp;gt;</summary>',
+    );
+    const content = pub.metadata[SYMBOL.CONTENT];
+    const html = getOPDSDescriptionHtml(content);
+    const text = renderedText(html);
+    expect(text).not.toContain('<p>');
+    expect(text).toContain('Creators: Algis Budrys');
+    expect(text).toContain('"Wall of Crystal"');
+    expect(html).toContain('<p>');
+  });
+
+  it('leaves mixed real-and-escaped markup untouched (no over-decoding)', () => {
+    // Author intentionally shows a literal <code> tag inside a real paragraph.
+    const content = { value: '<p>Use &lt;code&gt; here</p>', type: 'text' as const };
+    const text = renderedText(getOPDSDescriptionHtml(content));
+    expect(text).toBe('Use <code> here');
+  });
+
+  it('strips scripts from untrusted feed HTML', () => {
+    const content = {
+      value: '<p>Hi</p><script>alert(1)</script>',
+      type: 'html' as const,
+    };
+    const html = getOPDSDescriptionHtml(content);
+    expect(html).not.toContain('<script');
+    expect(html).not.toContain('alert(1)');
+  });
+
+  it('strips scripts hidden behind double-escaping', () => {
+    const content = {
+      value: '&lt;script&gt;alert(1)&lt;/script&gt;&lt;p&gt;Hi&lt;/p&gt;',
+      type: 'text' as const,
+    };
+    const html = getOPDSDescriptionHtml(content);
+    expect(html).not.toContain('<script');
+    expect(html).not.toContain('alert(1)');
+    expect(renderedText(html)).toContain('Hi');
+  });
+
+  it('accepts a plain string value', () => {
+    expect(renderedText(getOPDSDescriptionHtml('<p>Plain</p>'))).toBe('Plain');
+  });
+});
@@ -1,5 +1,5 @@
 import { describe, test, expect } from 'vitest';
-import { sanitizeHtml } from '@/services/send/conversion/sanitizeHtml';
+import { sanitizeHtml } from '@/utils/sanitize';
 import { convertToEpub, isConvertible, mimeToKind } from '@/services/send/conversion/convertToEpub';
 import { ConversionError } from '@/services/send/conversion/types';

@@ -14,6 +14,7 @@ import { eventDispatcher } from '@/utils/event';
 import { navigateToReader } from '@/utils/nav';
 import { CachedImage } from '@/components/CachedImage';
 import { groupByArray } from '../utils/opdsUtils';
+import { getOPDSDescriptionHtml } from '../utils/opdsContent';
 import Dropdown from '@/components/Dropdown';
 import MenuItem from '@/components/MenuItem';

@@ -164,6 +165,7 @@ export function PublicationView({

  const content = publication.metadata?.[SYMBOL.CONTENT] || publication.metadata?.content;
  const description = publication.metadata?.description;
+  const descriptionHtml = useMemo(() => getOPDSDescriptionHtml(content), [content]);

  return (
    <div className='flex w-full flex-col px-6 py-6'>
@@ -325,14 +327,10 @@ export function PublicationView({

      <div className='max-w-xl items-start space-y-6'>
        {/* Description */}
-        {(content || description) && (
+        {(descriptionHtml || description) && (
          <div className='prose prose-sm max-w-none'>
-            {content ? (
-              <div
-                dangerouslySetInnerHTML={{
-                  __html: typeof content === 'string' ? content : content.value,
-                }}
-              />
+            {descriptionHtml ? (
+              <div dangerouslySetInnerHTML={{ __html: descriptionHtml }} />
            ) : (
              <p>{description}</p>
            )}
@@ -0,0 +1,38 @@
+import { sanitizeHtml } from '@/utils/sanitize';
+import type { OPDSContent } from '@/types/opds';
+
+/**
+ * Decode one level of HTML entities, e.g. `&lt;p&gt;` -> `<p>`. Used to recover
+ * HTML markup that a feed escaped one extra time (see below).
+ */
+const decodeEntities = (text: string): string => {
+  const doc = new DOMParser().parseFromString(text, 'text/html');
+  return doc.documentElement.textContent ?? '';
+};
+
+/**
+ * Turn an OPDS publication description/content value into sanitized HTML ready
+ * for `dangerouslySetInnerHTML` (issue #4503).
+ *
+ * Feeds deliver summaries in several shapes:
+ *  - `type="html"`/`"xhtml"`, or `type="text"` holding single-escaped HTML:
+ *    the value already contains real markup, so it renders as-is.
+ *  - `type="text"` holding *double*-escaped HTML: some aggregators escape an
+ *    already-escaped summary, so the markup survives parsing as entity *text*
+ *    (`&lt;p&gt;...`) and previously showed literal `<p>`/`&quot;` tags. When
+ *    the value is entirely escaped markup (no real tags), decode it one extra
+ *    level so it renders as HTML — matching what other readers (e.g. Thorium)
+ *    show. Mixed content like `<p>see &lt;code&gt;</p>` is left untouched so an
+ *    intentionally-escaped tag still displays literally.
+ *
+ * Every branch is sanitized to strip scripts and other unsafe markup from this
+ * untrusted, remote feed content.
+ */
+export const getOPDSDescriptionHtml = (content: OPDSContent | string | undefined): string => {
+  const raw = typeof content === 'string' ? content : content?.value;
+  if (!raw) return '';
+  const hasEscapedTags = /&lt;\/?[a-z]/i.test(raw);
+  const hasRealTags = /<[a-z]/i.test(raw);
+  const html = hasEscapedTags && !hasRealTags ? decodeEntities(raw) : raw;
+  return sanitizeHtml(html);
+};
@@ -6,7 +6,7 @@ import { detectLanguage } from '@/utils/lang';
 // browser extension imports `convertPageToEpub` and never reaches those
 // branches, so dynamic imports let webpack code-split them out of the
 // extension bundle entirely.
-import { sanitizeHtml, sanitizeForParsing } from './sanitizeHtml';
+import { sanitizeHtml, sanitizeForParsing } from '@/utils/sanitize';
 import { buildEpub } from './buildEpub';
 import { bundleAssets } from './assetBundler';
 import { generateCoverSvg } from './coverGenerator';
@@ -1,72 +0,0 @@
-import DOMPurify from 'dompurify';
-
-/**
- * Strip untrusted HTML (from email bodies, web pages, DOCX conversion) down to
- * safe, EPUB-appropriate structural markup. Removes scripts, event handlers,
- * styles, iframes, and form controls; keeps headings, text, lists, tables,
- * links and images.
- *
- * Runs against the real DOM — Send to Readest converts on the client (browser
- * or Tauri webview), both of which provide `window`/`DOMParser`.
- */
-export function sanitizeHtml(html: string): string {
-  return DOMPurify.sanitize(html, {
-    ALLOWED_TAGS: [
-      'h1',
-      'h2',
-      'h3',
-      'h4',
-      'h5',
-      'h6',
-      'p',
-      'br',
-      'hr',
-      'blockquote',
-      'pre',
-      'code',
-      'strong',
-      'em',
-      'b',
-      'i',
-      'u',
-      's',
-      'sup',
-      'sub',
-      'span',
-      'ul',
-      'ol',
-      'li',
-      'dl',
-      'dt',
-      'dd',
-      'table',
-      'thead',
-      'tbody',
-      'tr',
-      'th',
-      'td',
-      'a',
-      'img',
-      'figure',
-      'figcaption',
-    ],
-    // `id` is allowed so heading anchors survive — the EPUB's nested
-    // navMap uses `chapter1.xhtml#heading-id` to link the TOC sidebar to
-    // each section. Without it readers see one entry per chapter only.
-    ALLOWED_ATTR: ['href', 'src', 'alt', 'title', 'colspan', 'rowspan', 'id'],
-    // Drop anything that would load or run remote code.
-    FORBID_TAGS: ['script', 'style', 'iframe', 'object', 'embed', 'form', 'input'],
-    FORBID_ATTR: ['srcset'],
-    ALLOW_DATA_ATTR: false,
-  });
-}
-
-/**
- * Strip scripts and event handlers from an untrusted *document* before it is
- * handed to `DOMParser`. Unlike `sanitizeHtml`, this keeps the document
- * structure (`<head>`, `<title>`, sectioning elements) so title extraction and
- * Readability still work — it only removes anything executable.
- */
-export function sanitizeForParsing(html: string): string {
-  return DOMPurify.sanitize(html, { WHOLE_DOCUMENT: true });
-}
@@ -124,7 +124,7 @@ interface OPDSSubject {
  scheme?: string;
 }

-interface OPDSContent {
+export interface OPDSContent {
  value: string;
  type: 'text' | 'html' | 'xhtml';
 }
@@ -1,4 +1,77 @@
+import DOMPurify from 'dompurify';
+
 export const sanitizeString = (str?: string) => {
  if (!str) return str;
  return str.replace(/\u0000/g, '');
 };
+
+/**
+ * Strip untrusted HTML (Send-to-Readest email/web/DOCX conversion, OPDS feed
+ * descriptions, etc.) down to safe, EPUB-appropriate structural markup. Removes
+ * scripts, event handlers, styles, iframes, and form controls; keeps headings,
+ * text, lists, tables, links and images.
+ *
+ * Runs against the real DOM, so callers must be on the client (browser or Tauri
+ * webview), both of which provide `window`/`DOMParser`.
+ */
+export function sanitizeHtml(html: string): string {
+  return DOMPurify.sanitize(html, {
+    ALLOWED_TAGS: [
+      'h1',
+      'h2',
+      'h3',
+      'h4',
+      'h5',
+      'h6',
+      'p',
+      'br',
+      'hr',
+      'blockquote',
+      'pre',
+      'code',
+      'strong',
+      'em',
+      'b',
+      'i',
+      'u',
+      's',
+      'sup',
+      'sub',
+      'span',
+      'ul',
+      'ol',
+      'li',
+      'dl',
+      'dt',
+      'dd',
+      'table',
+      'thead',
+      'tbody',
+      'tr',
+      'th',
+      'td',
+      'a',
+      'img',
+      'figure',
+      'figcaption',
+    ],
+    // `id` is allowed so heading anchors survive — the EPUB's nested
+    // navMap uses `chapter1.xhtml#heading-id` to link the TOC sidebar to
+    // each section. Without it readers see one entry per chapter only.
+    ALLOWED_ATTR: ['href', 'src', 'alt', 'title', 'colspan', 'rowspan', 'id'],
+    // Drop anything that would load or run remote code.
+    FORBID_TAGS: ['script', 'style', 'iframe', 'object', 'embed', 'form', 'input'],
+    FORBID_ATTR: ['srcset'],
+    ALLOW_DATA_ATTR: false,
+  });
+}
+
+/**
+ * Strip scripts and event handlers from an untrusted *document* before it is
+ * handed to `DOMParser`. Unlike `sanitizeHtml`, this keeps the document
+ * structure (`<head>`, `<title>`, sectioning elements) so title extraction and
+ * Readability still work — it only removes anything executable.
+ */
+export function sanitizeForParsing(html: string): string {
+  return DOMPurify.sanitize(html, { WHOLE_DOCUMENT: true });
+}