From 6a551af88ee3ba3869d5ee54b1e9f4889a3e1692 Mon Sep 17 00:00:00 2001 From: Holger Selover-Stephan Date: Fri, 21 Aug 2026 15:26:45 +0200 Subject: [PATCH 1/6] =?UTF-8?q?Add=20POST=20/convert/html-to-pdf=20?= =?UTF-8?q?=E2=80=94=20the=20missing=20conversion=20direction?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Closes #5. /convert/* exposed markdown-to-pdf, markdown-to-html, pdf-to-markdown and html-to-markdown. The one missing combination was HTML -> PDF, which is what a caller needs whenever the document is a LAYOUT rather than prose. This adds no rendering capability: htmlToPdf() already existed and markdown-to-pdf already calls it as its second stage. The route wraps it, with the same bearer-token gate, the same application/pdf default and base64 variant, and the same JavaScript-disabled, remote-blocked rendering. The caller's document is rendered VERBATIM — no wrapper, no default stylesheet. That is the point of the direction: going through Markdown to produce a positioned document means embedding raw HTML in Markdown and fighting the converter over what it escapes, wraps or reflows. Accepts JSON { html, options? } or a raw text/html body, mirroring html-to-markdown, since app.ts already registers a text/html parser and a caller sending one would otherwise get a confusing schema error. Two option semantics worth stating, because a complete document makes them ambiguous: - `css` is APPENDED after the document's own styles, last in , matching what "extra CSS" means on the Markdown path. It falls back to the top of , then to the front of the string, so a fragment still gets it rather than silently dropping it. The breakout is neutralized the same way markdownToHtml does it. - `title` sets the download filename only. The document already carries a and overwriting it would be an edit to the caller's markup. injectExtraCss uses FUNCTION replacements rather than string ones. A `$&` or `$1` in caller CSS would otherwise be interpreted as a replacement pattern and silently corrupt the stylesheet, and re-emitting a literal `</head>` would rewrite a caller's `</HEAD>` to lowercase — a silent edit to a document this endpoint promises to render verbatim. The second was caught by its own test. Tests: 7 unit tests for injectExtraCss (including the $-pattern and case cases), 4 route validation tests, and 4 Chromium round-trips — verbatim render, raw text/html body, an inlined data: URI image under the remote-block hardening, and the css option. 39 passing. Docs: README gains a section covering the endpoint plus the two traps a layout caller hits — inline assets as data: URIs, because a remote reference renders as nothing and still returns 200; and fonts are the image's, so embed the face if exact typography matters. The README endpoint table was also missing markdown-to-html and html-to-markdown; added, since a table that lists some endpoints reads as authoritative. Worker: Nils (backend-engineer) <hrn:worker:micromentor.org:mm-eng-team:nils> Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> --- CLAUDE.md | 8 ++++ README.md | 49 +++++++++++++++++++ src/lib/htmlToPdf.test.ts | 55 ++++++++++++++++++++++ src/lib/htmlToPdf.ts | 35 ++++++++++++++ src/routes/convert.test.ts | 96 ++++++++++++++++++++++++++++++++++++++ src/routes/convert.ts | 66 +++++++++++++++++++++++++- src/types.ts | 20 ++++++++ 7 files changed, 327 insertions(+), 2 deletions(-) create mode 100644 src/lib/htmlToPdf.test.ts diff --git a/CLAUDE.md b/CLAUDE.md index f26d60e..669759c 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -44,6 +44,14 @@ npm run test:watch # vitest - `POST /convert/markdown-to-pdf` — render Markdown to PDF; returns `application/pdf` bytes, or `{ pdfBase64, bytes }` with `Accept: application/json` / `?format=base64` (for embedding in a GraphQL response — the shape `hadron-server`'s `nodeExport` PDF path consumes). +- `POST /convert/html-to-pdf` — render a **complete** HTML document to PDF; same response shapes, + auth and page geometry as `markdown-to-pdf`. JSON `{ html, options? }`, or a raw `text/html` body + when no options are needed. The document is rendered **verbatim** — no wrapper, no default + stylesheet — because this direction exists for documents that are a *layout* rather than prose + (certificate, invoice, report cover), where the Markdown path's wrapper fights the caller's own + positioning. `options.css` is appended after the document's own styles; `options.title` sets the + download filename only, since the document already carries a `<title>`. Wraps the same + `htmlToPdf()` the Markdown path calls as its second stage, so it adds no rendering capability. - `POST /convert/markdown-to-html` — render Markdown to a complete, self-contained HTML document (inline print CSS, no external fetches); returns `text/html` bytes, or `{ html, bytes }` with `Accept: application/json` / `?format=json`. Consumer: `hadron-server`'s `nodeExport` HTML phase diff --git a/README.md b/README.md index fc3b0f0..5e7c1fb 100644 --- a/README.md +++ b/README.md @@ -13,7 +13,10 @@ platform and why the boundary is drawn here. | Method | Path | Purpose | |---|---|---| | `POST` | `/convert/markdown-to-pdf` | Render Markdown to a PDF | +| `POST` | `/convert/html-to-pdf` | Render a complete HTML document to a PDF | +| `POST` | `/convert/markdown-to-html` | Render Markdown to a self-contained HTML document | | `POST` | `/convert/pdf-to-markdown` | Extract Markdown text from a PDF | +| `POST` | `/convert/html-to-markdown` | Convert content HTML to Markdown | | `GET` | `/convert/info` | Service capabilities (no secrets) | | `GET` | `/healthz` | Liveness | | `GET` | `/readyz` | Readiness (+ whether Chromium is warm) | @@ -51,6 +54,52 @@ curl -sS -X POST http://localhost:8080/convert/markdown-to-pdf \ -o out.pdf ``` +### `POST /convert/html-to-pdf` + +For documents that are a **layout** rather than prose — a certificate, an +invoice, a report cover, a badge. Routing those through Markdown means embedding +raw HTML in Markdown and fighting the converter over what it escapes, wraps or +reflows; here the document is rendered **verbatim**, with no wrapper and no +default stylesheet. + +Request (JSON), or a raw `text/html` body when you don't need options: + +```jsonc +{ + "html": "<!DOCTYPE html><html>…</html>", + "options": { + "title": "Certificate", // filename only — the document has its own <title> + "format": "A4", // A4 | A3 | A5 | Letter | Legal | Tabloid + "landscape": true, + "printBackground": true, + "margin": { "top": "0mm", "right": "0mm", "bottom": "0mm", "left": "0mm" }, + "css": "@page { size: A4 landscape }" // optional, appended AFTER your styles + } +} +``` + +Response: identical to `markdown-to-pdf` — `application/pdf` bytes by default, +or `{ "pdfBase64": "...", "bytes": 12345 }` with `Accept: application/json` or +`?format=base64`. + +```bash +curl -sS -X POST http://localhost:8080/convert/html-to-pdf \ + -H 'content-type: application/json' \ + -H "authorization: Bearer $PDF_SERVICE_TOKEN" \ + -d '{"html":"<h1>Hello</h1>","options":{"landscape":true}}' \ + -o out.pdf +``` + +**Inline your assets as `data:` URIs.** Rendering runs with JavaScript disabled +and, unless `PDF_ALLOW_REMOTE=true`, every non-`data:` request is aborted — so a +background image or signature referenced by URL renders as nothing and still +returns `200`. Inlining also makes the render deterministic and offline, which +matters for a document people keep. + +**Fonts are the image's, not yours.** A document naming a typeface the container +does not have falls back silently. Embed the face as a `data:` URI `@font-face` +if exact typography matters. + ### `POST /convert/pdf-to-markdown` Send the PDF either as a raw `application/pdf` body or as JSON diff --git a/src/lib/htmlToPdf.test.ts b/src/lib/htmlToPdf.test.ts new file mode 100644 index 0000000..18c55d7 --- /dev/null +++ b/src/lib/htmlToPdf.test.ts @@ -0,0 +1,55 @@ +import { describe, expect, it } from 'vitest'; +import { injectExtraCss } from './htmlToPdf.js'; + +describe('injectExtraCss', () => { + it('returns the document untouched when no css is given', () => { + const html = '<!DOCTYPE html><html><head></head><body>hi</body></html>'; + expect(injectExtraCss(html)).toBe(html); + expect(injectExtraCss(html, '')).toBe(html); + }); + + it('appends LAST in <head> so it overrides the document own styles', () => { + const html = '<html><head><style>body{color:red}</style></head><body>hi</body></html>'; + const out = injectExtraCss(html, 'body{color:blue}'); + + expect(out).toContain('<style>\nbody{color:blue}\n</style>\n</head>'); + // Order is the point: the caller's rule must come after the document's. + expect(out.indexOf('color:blue')).toBeGreaterThan(out.indexOf('color:red')); + }); + + it('falls back to the top of <body> when there is no </head>', () => { + const out = injectExtraCss('<body class="x">hi</body>', 'p{margin:0}'); + expect(out).toContain('<body class="x">\n<style>'); + expect(out).toContain('p{margin:0}'); + }); + + it('falls back to prepending when the document has neither', () => { + const out = injectExtraCss('<p>fragment</p>', 'p{margin:0}'); + expect(out.startsWith('<style>')).toBe(true); + expect(out).toContain('<p>fragment</p>'); + }); + + it('neutralizes a </style> breakout in caller css', () => { + const out = injectExtraCss( + '<html><head></head><body></body></html>', + '</style><script>alert(1)</script>', + ); + // The injected close tag must not end the real <style> element. + expect(out).not.toContain('</style><script>'); + expect(out).toContain('<\\/style'); + }); + + it('does not interpret $ patterns in caller css as replacement patterns', () => { + // `$&` in a string replacement expands to the whole match, which would + // silently corrupt the stylesheet. The implementation uses a function + // replacement precisely to avoid this. + const css = 'p::after{content:"$& $1 $`"}'; + const out = injectExtraCss('<html><head></head><body></body></html>', css); + expect(out).toContain('content:"$& $1 $`"'); + }); + + it('is case-insensitive about </HEAD>', () => { + const out = injectExtraCss('<HTML><HEAD></HEAD><BODY></BODY></HTML>', 'p{margin:0}'); + expect(out).toContain('<style>\np{margin:0}\n</style>\n</HEAD>'); + }); +}); diff --git a/src/lib/htmlToPdf.ts b/src/lib/htmlToPdf.ts index b0adb8b..bc91737 100644 --- a/src/lib/htmlToPdf.ts +++ b/src/lib/htmlToPdf.ts @@ -12,6 +12,41 @@ import type { PdfOptions } from '../types.js'; * aborted — only `data:` and the initial `about:blank`/document load proceed. * That blocks `file://` reads and remote (SSRF) fetches via crafted Markdown. */ +/** + * Append caller-supplied CSS to a COMPLETE HTML document, last in `<head>` so + * it overrides the document's own styles — the same meaning `css` has on the + * Markdown path, where it is appended after the default stylesheet. + * + * Falls back to the top of `<body>`, then to the front of the string, so a + * fragment or a headless document still gets the styles rather than silently + * dropping them. + * + * The `</style>` neutralization mirrors `markdownToHtml`: without it, CSS + * carrying a close tag would end the `<style>` element and inject arbitrary + * markup into the rendered document. + * + * Replacements are FUNCTIONS, not strings: a `$&` or `$1` inside caller CSS + * would otherwise be interpreted as a replacement pattern and silently corrupt + * the stylesheet. + */ +export function injectExtraCss(html: string, css?: string): string { + if (!css) return html; + + const safe = css.replace(/<\/style/gi, '<\\/style'); + const block = `<style>\n${safe}\n</style>`; + + if (/<\/head>/i.test(html)) { + // Re-emit the MATCHED text rather than a literal `</head>`: this endpoint + // renders the caller's document verbatim, and rewriting their `</HEAD>` to + // lowercase would be a silent edit to it. + return html.replace(/<\/head>/i, match => `${block}\n${match}`); + } + if (/<body[^>]*>/i.test(html)) { + return html.replace(/<body[^>]*>/i, match => `${match}\n${block}`); + } + return `${block}\n${html}`; +} + export async function htmlToPdf(html: string, options: PdfOptions): Promise<Buffer> { const browser = await getBrowser(); const page = await browser.newPage(); diff --git a/src/routes/convert.test.ts b/src/routes/convert.test.ts index f2f9b23..769daf0 100644 --- a/src/routes/convert.test.ts +++ b/src/routes/convert.test.ts @@ -31,6 +31,7 @@ describe('health + routing', () => { const res = await request(app).get('/convert/info'); expect(res.status).toBe(200); expect(res.body.directions).toContain('markdown-to-pdf'); + expect(res.body.directions).toContain('html-to-pdf'); expect(res.body.directions).toContain('markdown-to-html'); expect(res.body.directions).toContain('pdf-to-markdown'); expect(res.body.directions).toContain('html-to-markdown'); @@ -54,6 +55,33 @@ describe('POST /convert/markdown-to-pdf validation', () => { }); }); +describe('POST /convert/html-to-pdf validation', () => { + it('rejects an empty html body', async () => { + const res = await request(app).post('/convert/html-to-pdf').send({ html: '' }); + expect(res.status).toBe(400); + }); + + it('rejects a missing html field', async () => { + const res = await request(app).post('/convert/html-to-pdf').send({}); + expect(res.status).toBe(400); + }); + + it('rejects an empty raw text/html body', async () => { + const res = await request(app) + .post('/convert/html-to-pdf') + .set('content-type', 'text/html') + .send(''); + expect(res.status).toBe(400); + }); + + it('rejects an unknown paper format', async () => { + const res = await request(app) + .post('/convert/html-to-pdf') + .send({ html: '<p>hi</p>', options: { format: 'Poster' } }); + expect(res.status).toBe(400); + }); +}); + describe('POST /convert/pdf-to-markdown validation', () => { it('rejects a non-PDF raw body', async () => { const res = await request(app) @@ -206,4 +234,72 @@ describe('round-trip (requires Chromium)', () => { expect(typeof res.body.pdfBase64).toBe('string'); expect(Buffer.from(res.body.pdfBase64, 'base64').subarray(0, 5).toString('latin1')).toBe('%PDF-'); }); + + it('html -> pdf renders the caller document verbatim, then round-trips back', async (ctx) => { + if (!browserOk) ctx.skip(); + + const html = + '<!DOCTYPE html><html><head><title>Ignored' + + '

Certificate of Completion

Awarded to Amara Okonkwo.

'; + + const pdfRes = await request(app) + .post('/convert/html-to-pdf') + .send({ html, options: { title: 'Cert', landscape: true } }) + .buffer(true) + .parse((res, cb) => { + const chunks: Buffer[] = []; + res.on('data', (c: Buffer) => chunks.push(c)); + res.on('end', () => cb(null, Buffer.concat(chunks))); + }); + + expect(pdfRes.status).toBe(200); + expect(pdfRes.headers['content-type']).toContain('application/pdf'); + expect(pdfRes.headers['content-disposition']).toContain('Cert.pdf'); + const pdf = pdfRes.body as Buffer; + expect(pdf.subarray(0, 5).toString('latin1')).toBe('%PDF-'); + + const mdRes = await request(app) + .post('/convert/pdf-to-markdown') + .set('content-type', 'application/pdf') + .send(pdf); + + expect(mdRes.status).toBe(200); + expect(mdRes.body.markdown.toLowerCase()).toContain('certificate of completion'); + expect(mdRes.body.markdown).toContain('Amara Okonkwo'); + }); + + it('accepts a raw text/html body and returns base64 on request', async (ctx) => { + if (!browserOk) ctx.skip(); + const res = await request(app) + .post('/convert/html-to-pdf?format=base64') + .set('content-type', 'text/html') + .send('

Raw body

'); + expect(res.status).toBe(200); + expect(Buffer.from(res.body.pdfBase64, 'base64').subarray(0, 5).toString('latin1')).toBe('%PDF-'); + }); + + it('renders an inlined data: URI image with remote fetching disabled', async (ctx) => { + if (!browserOk) ctx.skip(); + // The hardening aborts every non-data: request, so an inlined asset is the + // only way a layout can carry a background or a signature. Pinning it here + // because a regression would return 200 with the image silently missing. + const svg = Buffer.from( + '', + ).toString('base64'); + const html = `

with art

`; + + const res = await request(app).post('/convert/html-to-pdf?format=base64').send({ html }); + expect(res.status).toBe(200); + expect(Buffer.from(res.body.pdfBase64, 'base64').subarray(0, 5).toString('latin1')).toBe('%PDF-'); + }); + + it('applies the css option on top of the caller own styles', async (ctx) => { + if (!browserOk) ctx.skip(); + const html = '

Tinted

'; + const res = await request(app) + .post('/convert/html-to-pdf?format=base64') + .send({ html, options: { css: 'h1{color:#00ff00}' } }); + expect(res.status).toBe(200); + expect(res.body.bytes).toBeGreaterThan(0); + }); }); diff --git a/src/routes/convert.ts b/src/routes/convert.ts index e3f932c..f5619aa 100644 --- a/src/routes/convert.ts +++ b/src/routes/convert.ts @@ -7,11 +7,12 @@ import { PdfToMarkdownSchema, PdfOptionsSchema, HtmlToMarkdownSchema, + HtmlToPdfSchema, MarkdownToHtmlSchema, MAX_HTML_CHARS, } from '../types.js'; import { markdownToHtml } from '../lib/markdownToHtml.js'; -import { htmlToPdf } from '../lib/htmlToPdf.js'; +import { htmlToPdf, injectExtraCss } from '../lib/htmlToPdf.js'; import { pdfToMarkdown } from '../lib/pdfToMarkdown.js'; import { htmlToMarkdown } from '../lib/htmlToMarkdown.js'; import { logger } from '../logger.js'; @@ -58,6 +59,67 @@ convertRouter.post( }, ); +/** + * POST /convert/html-to-pdf + * Body: JSON { html: string, options?: PdfOptions }, OR a raw text/html payload + * (which takes the default options, since there is nowhere to carry them). + * Returns: application/pdf bytes by default, or { pdfBase64, bytes } when the + * client sends `Accept: application/json` or `?format=base64`. + * + * The caller's document is rendered VERBATIM — no wrapping, no default + * stylesheet. That is the point of this direction: it is for documents that are + * a layout rather than prose, where the Markdown path's wrapper would fight the + * caller's own positioning. Everything else matches markdown-to-pdf: same auth, + * same page geometry, same JavaScript-disabled and remote-blocked rendering. + */ +convertRouter.post( + '/html-to-pdf', + async (req: Request, res: Response, next: NextFunction) => { + try { + let html: string; + let options: unknown; + const contentType = req.get('content-type') ?? ''; + + if (contentType.includes('text/html')) { + if (typeof req.body !== 'string' || req.body.length === 0) { + throw new HttpError(400, 'empty text/html body'); + } + // Enforce the same cap the JSON schema applies, so both modes match. + if (req.body.length > MAX_HTML_CHARS) { + throw new HttpError(413, `html body exceeds ${MAX_HTML_CHARS} characters`); + } + html = req.body; + } else { + const parsed = HtmlToPdfSchema.parse(req.body); + html = parsed.html; + options = parsed.options; + } + + const opts = PdfOptionsSchema.parse(options ?? {}); + const pdf = await htmlToPdf(injectExtraCss(html, opts.css), opts); + + const wantsJson = + req.query.format === 'base64' || req.accepts(['application/pdf', 'application/json']) === 'application/json'; + + logger.info('html-to-pdf', { bytes: pdf.length, format: opts.format, json: wantsJson }); + + if (wantsJson) { + res.json({ pdfBase64: pdf.toString('base64'), bytes: pdf.length }); + return; + } + + res + .status(200) + .setHeader('Content-Type', 'application/pdf') + .setHeader('Content-Length', pdf.length) + .setHeader('Content-Disposition', `inline; filename="${(opts.title || 'document').replace(/[^\w.-]/g, '_')}.pdf"`) + .send(pdf); + } catch (err) { + next(err); + } + }, +); + /** * POST /convert/markdown-to-html * Body: { markdown: string, options?: { title, css } } @@ -171,7 +233,7 @@ convertRouter.post( convertRouter.get('/info', (_req, res) => { res.json({ service: 'hadrontool-pdf', - directions: ['markdown-to-pdf', 'markdown-to-html', 'pdf-to-markdown', 'html-to-markdown'], + directions: ['markdown-to-pdf', 'html-to-pdf', 'markdown-to-html', 'pdf-to-markdown', 'html-to-markdown'], allowRemote: config.allowRemote, authRequired: Boolean(config.serviceToken), }); diff --git a/src/types.ts b/src/types.ts index aa85260..9f55a8c 100644 --- a/src/types.ts +++ b/src/types.ts @@ -38,6 +38,26 @@ export const MarkdownToPdfSchema = z.object({ export type MarkdownToPdfRequest = z.infer; +/** + * JSON body shape for html->pdf (alternative to a raw text/html body). + * + * The caller supplies a COMPLETE document and it is rendered verbatim — this is + * the direction to use when the output is a *layout* rather than prose (a + * certificate, an invoice, a report cover), where routing through Markdown + * would mean fighting the converter over what it escapes, wraps or reflows. + * + * Shares `PdfOptionsSchema` with markdown-to-pdf so page geometry behaves + * identically. `css` is appended to the caller's own styles rather than + * replacing them; `title` sets the download filename only, since the document + * already carries its own ``. + */ +export const HtmlToPdfSchema = z.object({ + html: z.string().min(1, 'html must not be empty').max(MAX_HTML_CHARS), + options: PdfOptionsSchema.optional(), +}); + +export type HtmlToPdfRequest = z.infer<typeof HtmlToPdfSchema>; + /** * Options for markdown->HTML. A subset of PdfOptions — only the two fields that * affect the HTML document itself apply; page geometry (format/margin/...) is From 858aab4b156e9328ae3c2295ab0751f49efd5f5b Mon Sep 17 00:00:00 2001 From: Holger Selover-Stephan <holger@baragaun.com> Date: Fri, 21 Aug 2026 18:08:27 +0200 Subject: [PATCH 2/6] Address review feedback: doctype, inert regions, and the fragment wording MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Codex found two real defects in injectExtraCss, both silent: 1. QUIRKS MODE. For a standards-compliant document that omits the optional <head>/<body> tags — `<!DOCTYPE html><title>x
y
` — the prepend fallback put the hi'; const out = injectExtraCss(html, 'body{color:blue}'); @@ -48,8 +48,52 @@ describe('injectExtraCss', () => { expect(out).toContain('content:"$& $1 $`"'); }); - it('is case-insensitive about ', () => { + it('is case-insensitive about , and preserves the caller casing', () => { const out = injectExtraCss('', 'p{margin:0}'); expect(out).toContain('\n'); + // The caller's tag must come back exactly as written, not lowercased. + expect(out).not.toContain(''); + }); + + // A late doctype puts Chromium in quirks mode, which changes the box model — + // on an endpoint whose whole purpose is rendering a layout the caller has + // already positioned. The document below is standards-compliant: and + // are both optional tags. + it('inserts AFTER a leading doctype when there is no head or body', () => { + const html = 'Invoice
total
'; + const out = injectExtraCss(html, 'main{margin:0}'); + + expect(out.toLowerCase().indexOf('')).toBe(0); + expect(out.indexOf('\n'); + // And the comment is left exactly as the caller wrote it. + expect(out).toContain(''); + }); + + it('ignores a that appears inside a script', () => { + const html = 'b'; + const out = injectExtraCss(html, 'p{margin:0}'); + expect(out).toContain('\n'); + }); + + it('ignores a that appears inside a comment when falling back', () => { + const html = 'hi'; + const out = injectExtraCss(html, 'p{margin:0}'); + expect(out).toContain('\n` neutralization mirrors `markdownToHtml`: without it, CSS * carrying a close tag would end the ``; + const mask = maskInertRegions(html); - if (/<\/head>/i.test(html)) { - // Re-emit the MATCHED text rather than a literal ``: this endpoint - // renders the caller's document verbatim, and rewriting their `` to - // lowercase would be a silent edit to it. - return html.replace(/<\/head>/i, match => `${block}\n${match}`); + const headClose = mask.search(/<\/head\s*>/i); + if (headClose !== -1) { + return `${html.slice(0, headClose)}${block}\n${html.slice(headClose)}`; } - if (/]*>/i.test(html)) { - return html.replace(/]*>/i, match => `${match}\n${block}`); + + const bodyOpen = /]*>/i.exec(mask); + if (bodyOpen) { + const at = bodyOpen.index + bodyOpen[0].length; + return `${html.slice(0, at)}\n${block}${html.slice(at)}`; } - return `${block}\n${html}`; + + const doctype = /^\s*]*>/i.exec(html); + const at = doctype ? doctype[0].length : 0; + return at > 0 + ? `${html.slice(0, at)}\n${block}${html.slice(at)}` + : `${block}\n${html}`; } +/** + * Render an HTML document to PDF bytes using the shared headless browser. Each + * call gets its own page, which is always closed. + * + * Security hardening: JavaScript is disabled (the document is static print + * content), and unless PDF_ALLOW_REMOTE is set, every non-inline request is + * aborted — only `data:` and the initial `about:blank`/document load proceed. + * That blocks `file://` reads and remote (SSRF) fetches via crafted Markdown. + */ export async function htmlToPdf(html: string, options: PdfOptions): Promise { const browser = await getBrowser(); const page = await browser.newPage(); diff --git a/src/routes/convert.test.ts b/src/routes/convert.test.ts index 769daf0..faeca64 100644 --- a/src/routes/convert.test.ts +++ b/src/routes/convert.test.ts @@ -293,7 +293,7 @@ describe('round-trip (requires Chromium)', () => { expect(Buffer.from(res.body.pdfBase64, 'base64').subarray(0, 5).toString('latin1')).toBe('%PDF-'); }); - it('applies the css option on top of the caller own styles', async (ctx) => { + it("applies the css option on top of the caller's own styles", async (ctx) => { if (!browserOk) ctx.skip(); const html = '

Tinted

'; const res = await request(app) diff --git a/src/routes/convert.ts b/src/routes/convert.ts index f5619aa..c2f3c1c 100644 --- a/src/routes/convert.ts +++ b/src/routes/convert.ts @@ -66,18 +66,22 @@ convertRouter.post( * Returns: application/pdf bytes by default, or { pdfBase64, bytes } when the * client sends `Accept: application/json` or `?format=base64`. * - * The caller's document is rendered VERBATIM — no wrapping, no default - * stylesheet. That is the point of this direction: it is for documents that are - * a layout rather than prose, where the Markdown path's wrapper would fight the - * caller's own positioning. Everything else matches markdown-to-pdf: same auth, - * same page geometry, same JavaScript-disabled and remote-blocked rendering. + * The caller's HTML is rendered VERBATIM — nothing is wrapped, nothing is + * restyled. Typically that is a complete document; a bare fragment also works, + * since Chromium supplies the missing structure. That is the point of this + * direction: it is for output that is a layout rather than prose, where the + * Markdown path's wrapper and default stylesheet would fight the caller's own + * positioning. Everything else matches markdown-to-pdf: same auth, same page + * geometry, same JavaScript-disabled and remote-blocked rendering. */ convertRouter.post( '/html-to-pdf', async (req: Request, res: Response, next: NextFunction) => { try { let html: string; - let options: unknown; + // Explicitly undefined: the raw text/html mode carries no options, and + // `?? {}` below turns that into the documented defaults. + let options: unknown = undefined; const contentType = req.get('content-type') ?? ''; if (contentType.includes('text/html')) { diff --git a/src/types.ts b/src/types.ts index 9f55a8c..e3e8283 100644 --- a/src/types.ts +++ b/src/types.ts @@ -41,10 +41,12 @@ export type MarkdownToPdfRequest = z.infer; /** * JSON body shape for html->pdf (alternative to a raw text/html body). * - * The caller supplies a COMPLETE document and it is rendered verbatim — this is - * the direction to use when the output is a *layout* rather than prose (a - * certificate, an invoice, a report cover), where routing through Markdown - * would mean fighting the converter over what it escapes, wraps or reflows. + * The caller's HTML is rendered verbatim — usually a complete document, though + * a bare fragment works too, since Chromium supplies the missing structure. + * Nothing is wrapped and nothing is restyled. This is the direction to use when + * the output is a *layout* rather than prose (a certificate, an invoice, a + * report cover), where routing through Markdown would mean fighting the + * converter over what it escapes, wraps or reflows. * * Shares `PdfOptionsSchema` with markdown-to-pdf so page geometry behaves * identically. `css` is appended to the caller's own styles rather than From 553888675b4f4e4d82b0f423306b294084b486b5 Mon Sep 17 00:00:00 2001 From: Holger Selover-Stephan Date: Fri, 21 Aug 2026 18:20:10 +0200 Subject: [PATCH 3/6] Address second review pass: RCDATA elements and the pre-doctype prologue MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Codex's re-review found both earlier fixes stopped one step short. Both are the same defect class as before — a silent drop of options.css, or a silent slide into quirks mode — reached by a different route. 1. RCDATA. The mask covered comments and the raw text of \n'); }); + // is RCDATA: its content is text until its own closing tag, so the + // document below is valid and its FIRST </head> is not the head boundary. + it('ignores a </head> that appears inside a <title>', () => { + const html = '<html><head><title>Example </head> textb'; + const out = injectExtraCss(html, 'p{margin:0}'); + + expect(out).toContain('Example </head> text\n'); + // The title must come back exactly as written. + expect(out).toContain('Example </head> text'); + }); + + it('ignores a that appears inside a '; + const out = injectExtraCss(html, 'p{margin:0}'); + // The real (and only genuine) comes first, so it wins regardless — + // what this pins is that the textarea text is not treated as a tag. + expect(out).toContain('\n'); + expect(out).toContain(''); + }); + + // Comments are legal before the doctype. Inserting ahead of them makes the + // doctype late, which is quirks mode again by a different route. + it('inserts after a doctype that is preceded by comments', () => { + const html = 'Invoice
total
'; + const out = injectExtraCss(html, 'main{margin:0}'); + + expect(out.startsWith('')).toBe(true); + expect(out.indexOf('hi'; const out = injectExtraCss(html, 'body{color:blue}'); - expect(out).toContain('\n'); - // Order is the point: the caller's rule must come after the document's. expect(out.indexOf('color:blue')).toBeGreaterThan(out.indexOf('color:red')); - }); - - it('falls back to the top of when there is no ', () => { - const out = injectExtraCss('hi', 'p{margin:0}'); - expect(out).toContain('\n')).toBe(true); + }); + + /** + * The property that replaced a pile of insertion-point special cases. Each + * document below contains a `` or `` that is TEXT, not a tag — + * a comment, RCDATA, raw text, a quoted attribute. An implementation that + * searches for a boundary has to get every one of them right; one that + * appends cannot get any of them wrong. + */ + it.each([ + ['complete document', 'b'], + ['uppercase tags', ''], + ['bare fragment', '

fragment

'], + ['no head or body', 'Invoice
total
'], + ['comment before the doctype', 'x'], + [' inside a comment', 'tb'], + [' inside a title', 'Example </head> textb'], + [' inside a script', 'b'], + [' inside an attribute', 'b'], + [' inside a comment', 'hi'], + ])('leaves %s byte-identical as a prefix', (_label, html) => { + const out = injectExtraCss(html, 'p{margin:0}'); + expect(out.startsWith(html)).toBe(true); + expect(out.slice(html.length)).toBe('\n'); }); it('neutralizes a breakout in caller css', () => { @@ -40,90 +51,11 @@ describe('injectExtraCss', () => { }); it('does not interpret $ patterns in caller css as replacement patterns', () => { - // `$&` in a string replacement expands to the whole match, which would - // silently corrupt the stylesheet. The implementation uses a function - // replacement precisely to avoid this. + // Structurally impossible now that this is a template append rather than a + // String.replace, but worth pinning: `$&` silently expanding to the whole + // match would corrupt the stylesheet with no error. const css = 'p::after{content:"$& $1 $`"}'; const out = injectExtraCss('', css); expect(out).toContain('content:"$& $1 $`"'); }); - - it('is case-insensitive about , and preserves the caller casing', () => { - const out = injectExtraCss('', 'p{margin:0}'); - expect(out).toContain('\n'); - // The caller's tag must come back exactly as written, not lowercased. - expect(out).not.toContain(''); - }); - - // A late doctype puts Chromium in quirks mode, which changes the box model — - // on an endpoint whose whole purpose is rendering a layout the caller has - // already positioned. The document below is standards-compliant: and - // are both optional tags. - it('inserts AFTER a leading doctype when there is no head or body', () => { - const html = 'Invoice
total
'; - const out = injectExtraCss(html, 'main{margin:0}'); - - expect(out.toLowerCase().indexOf('')).toBe(0); - expect(out.indexOf('\n'); - // And the comment is left exactly as the caller wrote it. - expect(out).toContain(''); - }); - - it('ignores a that appears inside a script', () => { - const html = 'b'; - const out = injectExtraCss(html, 'p{margin:0}'); - expect(out).toContain('\n'); - }); - - // is RCDATA: its content is text until its own closing tag, so the - // document below is valid and its FIRST </head> is not the head boundary. - it('ignores a </head> that appears inside a <title>', () => { - const html = '<html><head><title>Example </head> textb'; - const out = injectExtraCss(html, 'p{margin:0}'); - - expect(out).toContain('Example </head> text\n'); - // The title must come back exactly as written. - expect(out).toContain('Example </head> text'); - }); - - it('ignores a that appears inside a '; - const out = injectExtraCss(html, 'p{margin:0}'); - // The real (and only genuine) comes first, so it wins regardless — - // what this pins is that the textarea text is not treated as a tag. - expect(out).toContain('\n'); - expect(out).toContain(''); - }); - - // Comments are legal before the doctype. Inserting ahead of them makes the - // doctype late, which is quirks mode again by a different route. - it('inserts after a doctype that is preceded by comments', () => { - const html = 'Invoice
total
'; - const out = injectExtraCss(html, 'main{margin:0}'); - - expect(out.startsWith('')).toBe(true); - expect(out.indexOf('` neutralization mirrors `markdownToHtml`: without it, CSS * carrying a close tag would end the ``; - const mask = maskInertRegions(html); - - const headClose = mask.search(/<\/head\s*>/i); - if (headClose !== -1) { - return `${html.slice(0, headClose)}${block}\n${html.slice(headClose)}`; - } - - const bodyOpen = /]*>/i.exec(mask); - if (bodyOpen) { - const at = bodyOpen.index + bodyOpen[0].length; - return `${html.slice(0, at)}\n${block}${html.slice(at)}`; - } - - // A doctype may be preceded by comments as well as whitespace, so the scan - // steps over the whole legal prologue before looking for it. - const prologue = prologueLength(html); - const doctype = /^]*>/i.exec(html.slice(prologue)); - if (doctype) { - const at = prologue + doctype[0].length; - return `${html.slice(0, at)}\n${block}${html.slice(at)}`; - } - // No doctype at all: the document is already in quirks mode whatever we do, - // so position carries no consequence and the block goes first. - return `${block}\n${html}`; + return `${html}\n`; } /** diff --git a/src/routes/convert.test.ts b/src/routes/convert.test.ts index faeca64..6fe4e97 100644 --- a/src/routes/convert.test.ts +++ b/src/routes/convert.test.ts @@ -293,13 +293,34 @@ describe('round-trip (requires Chromium)', () => { expect(Buffer.from(res.body.pdfBase64, 'base64').subarray(0, 5).toString('latin1')).toBe('%PDF-'); }); - it("applies the css option on top of the caller's own styles", async (ctx) => { + it('the css option actually takes effect in the rendered PDF', async (ctx) => { if (!browserOk) ctx.skip(); - const html = '

Tinted

'; - const res = await request(app) - .post('/convert/html-to-pdf?format=base64') - .send({ html, options: { css: 'h1{color:#00ff00}' } }); - expect(res.status).toBe(200); - expect(res.body.bytes).toBeGreaterThan(0); + + // Asserts the OUTCOME rather than where the neutralization is retired. DOM text content cannot close an element, so caller CSS containing is now just CSS rather than something to defuse. - The Markdown path is unaffected: markdownToHtml already bakes options.css into the document it builds, so it passes no extraCss and the CSS is not applied twice. Verified rather than assumed: page.addStyleTag works despite setJavaScriptEnabled(false), because that disables scripts belonging to the PAGE while addStyleTag runs over the DevTools protocol. That is pinned by the end-to-end tests, not asserted in a comment — a silent failure there would look exactly like success. Tests. The unit tests for injectExtraCss are gone with it; the string-level properties they pinned no longer exist. In their place, five end-to-end cases covering the documents that defeated each earlier approach — in a comment, in a title, in an attribute, an unclosed textarea, an unterminated comment — each asserting on the RENDERED output that the stylesheet took effect. Mutation-verified: reverting to the string append fails exactly the two open-context cases and passes the other three, which is precisely the boundary review described. 37 passing. Worker: Nils (backend-engineer) Co-Authored-By: Claude Opus 5 --- src/lib/htmlToPdf.test.ts | 61 ---------------------------- src/lib/htmlToPdf.ts | 83 ++++++++++++++++++-------------------- src/routes/convert.test.ts | 30 ++++++++++++++ src/routes/convert.ts | 4 +- 4 files changed, 72 insertions(+), 106 deletions(-) delete mode 100644 src/lib/htmlToPdf.test.ts diff --git a/src/lib/htmlToPdf.test.ts b/src/lib/htmlToPdf.test.ts deleted file mode 100644 index bf8c489..0000000 --- a/src/lib/htmlToPdf.test.ts +++ /dev/null @@ -1,61 +0,0 @@ -import { describe, expect, it } from 'vitest'; -import { injectExtraCss } from './htmlToPdf.js'; - -describe('injectExtraCss', () => { - it('returns the document untouched when no css is given', () => { - const html = 'hi'; - expect(injectExtraCss(html)).toBe(html); - expect(injectExtraCss(html, '')).toBe(html); - }); - - it('appends AFTER the document so it wins the cascade', () => { - const html = 'hi'; - const out = injectExtraCss(html, 'body{color:blue}'); - - expect(out.indexOf('color:blue')).toBeGreaterThan(out.indexOf('color:red')); - expect(out.endsWith('')).toBe(true); - }); - - /** - * The property that replaced a pile of insertion-point special cases. Each - * document below contains a `` or `` that is TEXT, not a tag — - * a comment, RCDATA, raw text, a quoted attribute. An implementation that - * searches for a boundary has to get every one of them right; one that - * appends cannot get any of them wrong. - */ - it.each([ - ['complete document', 'b'], - ['uppercase tags', ''], - ['bare fragment', '

fragment

'], - ['no head or body', 'Invoice
total
'], - ['comment before the doctype', 'x'], - [' inside a comment', 'tb'], - [' inside a title', 'Example </head> textb'], - [' inside a script', 'b'], - [' inside an attribute', 'b'], - [' inside a comment', 'hi'], - ])('leaves %s byte-identical as a prefix', (_label, html) => { - const out = injectExtraCss(html, 'p{margin:0}'); - expect(out.startsWith(html)).toBe(true); - expect(out.slice(html.length)).toBe('\n'); - }); - - it('neutralizes a breakout in caller css', () => { - const out = injectExtraCss( - '', - '', - ); - // The injected close tag must not end the real