From ff58af7586021716f1d119da62a6961e49350024 Mon Sep 17 00:00:00 2001 From: Dave Date: Sun, 4 Oct 2026 13:14:59 +0200 Subject: [PATCH 1/5] fix(pdf): render through renderToBuffer, not pdf().toString() MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Every renderer funneled the PDF through a JS string and re-encoded it as latin1. Three separate problems, only the first of which was ever real: 1. `pdf(el).toBuffer()` is NOT the buffer API — it resolves to the PDFKit *stream* (react-pdf carries a TODO to rename it `toStream`), and under jsdom it resolves to a bare PDFDocument handle. That is the jsdom breakage an old comment described, but it applies to that method, not to the module-level `renderToBuffer()` helper. 2. `toString()` renders with `compress: false`. react-pdf's own source says why: "when rendering to string if compress=true the document is blank". The method is flagged upstream as buggy (#2112, #2095). 3. The string round-trip mangles binary streams. Text is ASCII in the content stream so it survives; a raster image is a compressed blob that does not — the image object lands in the file but is never painted. Poppler was already reporting "Bad FCHECK in flate stream". It stayed latent because the only image in these documents is the letterhead logo, and that logo is an SVG. SVG is text, so it round-tripped perfectly and hid the whole thing. `0xFD` in the old comments is the fingerprint: latin1's truncation of U+FFFD, the Unicode replacement char. `renderToBuffer()` fixes all three in a single pass and works under jsdom, so no fallback is needed. Note you cannot "try toBuffer, fall back to toString" — calling `toBuffer()` first consumes the render and a later `toString()` on the same instance returns a document with the content missing. Three more instances of the same pattern lived outside render.tsx and are fixed here too: the print-bundle, delivery-note-print and picking-slip routes. `render-image-pipeline.test.ts` is the regression guard. Its size assertion is expressed against the old uncompressed baseline rather than a round number picked when the document was smaller — an absolute byte count is a tripwire that fires on every feature addition, which is how a real regression gets tuned away. Guard verified by reintroducing the old path: 2 tests fail. --- .../invoicing/delivery-note-print/route.ts | 13 +- .../api/invoicing/print-bundle/route.test.ts | 9 +- src/app/api/invoicing/print-bundle/route.ts | 15 +- .../picking-slip/route.test.ts | 14 +- .../v1/stock-purchases/picking-slip/route.ts | 15 +- src/lib/letterhead.ts | 14 +- src/lib/pdf/quote-document.tsx | 28 +- src/lib/pdf/render-image-pipeline.test.ts | 283 ++++++++++++++++++ src/lib/pdf/render.test.ts | 38 ++- src/lib/pdf/render.tsx | 126 +++++--- src/lib/pdf/repair-request-document.tsx | 9 +- src/lib/server/quote-pdf-service.ts | 8 +- 12 files changed, 475 insertions(+), 97 deletions(-) create mode 100644 src/lib/pdf/render-image-pipeline.test.ts diff --git a/src/app/api/invoicing/delivery-note-print/route.ts b/src/app/api/invoicing/delivery-note-print/route.ts index 349e8ff..b7c9d8f 100644 --- a/src/app/api/invoicing/delivery-note-print/route.ts +++ b/src/app/api/invoicing/delivery-note-print/route.ts @@ -1,6 +1,6 @@ import React from "react"; import { NextRequest, NextResponse } from "next/server"; -import { pdf, type DocumentProps } from "@react-pdf/renderer"; +import { renderToBuffer, type DocumentProps } from "@react-pdf/renderer"; import { getAdminPocketBase } from "@/lib/server/pocketbase-admin"; import { buildPrintBundle } from "@/app/(app)/invoicing/print-bundle"; @@ -183,10 +183,13 @@ async function handlePrint(input: { deliveryNoteNumber: string; origin: string } }), }; }); - const slipDoc = PickingSlipPdf({ header: slipHeader, lines: slipLines }); - const slipInstance = pdf(slipDoc as unknown as React.ReactElement); - const slipString = await slipInstance.toString(); - const slipBytes = new Uint8Array(Buffer.from(slipString, "binary")); + // `renderToBuffer()` — NOT `pdf(doc).toString()`. The toString path + // renders uncompressed and re-encodes the PDF as latin1, which mangles + // binary streams (raster images arrive embedded but unpainted). See the + // note at the top of `src/lib/pdf/render.tsx`. + const slipBytes = new Uint8Array( + await renderToBuffer(PickingSlipPdf({ header: slipHeader, lines: slipLines }) as React.ReactElement), + ); // Same bundle shape as the invoice print: duplicate the DN so the // operator gets an office copy + customer copy in one print, then diff --git a/src/app/api/invoicing/print-bundle/route.test.ts b/src/app/api/invoicing/print-bundle/route.test.ts index bd4ea23..98c1e5c 100644 --- a/src/app/api/invoicing/print-bundle/route.test.ts +++ b/src/app/api/invoicing/print-bundle/route.test.ts @@ -4,9 +4,10 @@ const { fetchMock, getFullList, buildPrintBundle, pdfMock, pickingSlipPdfMock, f const fetchMock = vi.fn(); const getFullList = vi.fn(); const buildPrintBundle = vi.fn(); - const pdfMock = vi.fn(() => ({ - toString: vi.fn().mockResolvedValue("SLIP_PDF"), - })); + // The route renders the picking slip via `renderToBuffer()`, not + // `pdf(doc).toString()` — see the note at the top of + // `src/lib/pdf/render.tsx`. Returns real bytes. + const pdfMock = vi.fn().mockResolvedValue(Buffer.from("SLIP_PDF")); const pickingSlipPdfMock = vi.fn((_props: unknown) => "PICKING_SLIP_DOC"); // Mirror the PB SDK's files.getURL: the caller passes // { collectionId, id } and a filename, the SDK returns @@ -43,7 +44,7 @@ vi.mock("@/app/(app)/invoicing/print-bundle", () => ({ })); vi.mock("@react-pdf/renderer", () => ({ - pdf: pdfMock, + renderToBuffer: pdfMock, Document: () => null, Page: () => null, Text: () => null, diff --git a/src/app/api/invoicing/print-bundle/route.ts b/src/app/api/invoicing/print-bundle/route.ts index ea649ee..6638c3a 100644 --- a/src/app/api/invoicing/print-bundle/route.ts +++ b/src/app/api/invoicing/print-bundle/route.ts @@ -1,6 +1,6 @@ import React from "react"; import { NextRequest, NextResponse } from "next/server"; -import { pdf, type DocumentProps } from "@react-pdf/renderer"; +import { renderToBuffer, type DocumentProps } from "@react-pdf/renderer"; import { getAdminPocketBase } from "@/lib/server/pocketbase-admin"; import { buildPrintBundle } from "@/app/(app)/invoicing/print-bundle"; @@ -164,10 +164,15 @@ async function handlePrintBundle(input: { invoiceNumber: string }) { }; }); - const slipDoc = PickingSlipPdf({ header, lines }); - const slipInstance = pdf(slipDoc as unknown as React.ReactElement); - const slipString = await slipInstance.toString(); - const slipBytes = new Uint8Array(Buffer.from(slipString, "binary")); + // `renderToBuffer()` — NOT `pdf(doc).toString()`. The toString path + // renders uncompressed and re-encodes the PDF as latin1, which mangles + // binary streams (raster images arrive embedded but unpainted). See the + // note at the top of `src/lib/pdf/render.tsx`. + const slipBytes = new Uint8Array( + await renderToBuffer( + PickingSlipPdf({ header, lines }) as React.ReactElement, + ), + ); const combined = await buildPrintBundle(invoiceBytes, slipBytes); const out = Buffer.from(combined); diff --git a/src/app/api/v1/stock-purchases/picking-slip/route.test.ts b/src/app/api/v1/stock-purchases/picking-slip/route.test.ts index 839a706..6847ce3 100644 --- a/src/app/api/v1/stock-purchases/picking-slip/route.test.ts +++ b/src/app/api/v1/stock-purchases/picking-slip/route.test.ts @@ -1,11 +1,13 @@ import { describe, expect, it, beforeEach, vi } from "vitest"; import React from "react"; -const { pdfMock, pickingSlipPdfMock, getFirstListItem, getFullList } = vi.hoisted(() => { +const { renderToBufferMock, pickingSlipPdfMock, getFirstListItem, getFullList } = vi.hoisted(() => { return { - pdfMock: vi.fn(() => ({ - toString: vi.fn().mockResolvedValue("STOCK_PICKING_SLIP"), - })), + // The route renders via `renderToBuffer()` (module-level helper), + // not `pdf(doc).toString()` — see the note at the top of + // `src/lib/pdf/render.tsx` for why the toString path was removed. + // It returns real bytes, so the mock hands back a real Buffer. + renderToBufferMock: vi.fn().mockResolvedValue(Buffer.from("STOCK_PICKING_SLIP")), pickingSlipPdfMock: vi.fn((_props: unknown) => "STOCK_PICKING_SLIP_DOC"), getFirstListItem: vi.fn(), getFullList: vi.fn(), @@ -13,7 +15,7 @@ const { pdfMock, pickingSlipPdfMock, getFirstListItem, getFullList } = vi.hoiste }); vi.mock("@react-pdf/renderer", () => ({ - pdf: pdfMock, + renderToBuffer: renderToBufferMock, Document: () => null, Page: () => null, Text: () => null, @@ -56,7 +58,7 @@ function makeGetRequest(purchaseNumber?: string): Request { describe("GET /api/v1/stock-purchases/picking-slip", () => { beforeEach(() => { - pdfMock.mockClear(); + renderToBufferMock.mockClear(); pickingSlipPdfMock.mockReset(); getFirstListItem.mockReset(); getFullList.mockReset(); diff --git a/src/app/api/v1/stock-purchases/picking-slip/route.ts b/src/app/api/v1/stock-purchases/picking-slip/route.ts index 07fb00a..d0f2fa5 100644 --- a/src/app/api/v1/stock-purchases/picking-slip/route.ts +++ b/src/app/api/v1/stock-purchases/picking-slip/route.ts @@ -1,6 +1,6 @@ import React from "react"; import { NextRequest, NextResponse } from "next/server"; -import { pdf, type DocumentProps } from "@react-pdf/renderer"; +import { renderToBuffer, type DocumentProps } from "@react-pdf/renderer"; import { getAdminPocketBase } from "@/lib/server/pocketbase-admin"; import { PickingSlipPdf } from "@/app/(app)/invoicing/picking-slip"; @@ -149,10 +149,15 @@ export async function GET(request: NextRequest) { }; }); - const slipDoc = PickingSlipPdf({ header, lines: slipLines }); - const slipInstance = pdf(slipDoc as unknown as React.ReactElement); - const slipString = await slipInstance.toString(); - const slipBytes = new Uint8Array(Buffer.from(slipString, "binary")); + // `renderToBuffer()` — NOT `pdf(doc).toString()`. The toString path + // renders uncompressed and re-encodes the PDF as latin1, which mangles + // binary streams (raster images arrive embedded but unpainted). See the + // note at the top of `src/lib/pdf/render.tsx`. + const slipBytes = new Uint8Array( + await renderToBuffer( + PickingSlipPdf({ header, lines: slipLines }) as React.ReactElement, + ), + ); const filename = `${purchase.purchase_number ?? purchaseNumber}-picking-slip.pdf`; return new NextResponse(slipBytes, { diff --git a/src/lib/letterhead.ts b/src/lib/letterhead.ts index 2622fcb..c149769 100644 --- a/src/lib/letterhead.ts +++ b/src/lib/letterhead.ts @@ -147,11 +147,15 @@ export function defaultAccentFor(code: LetterheadCode): string { * return it as a `data:image/svg+xml;base64,...` URI suitable for * `` in @react-pdf. * - * We embed SVG instead of the original raster (PNG/JPEG) logos because - * @react-pdf/renderer v4.5.1 has a confirmed byte-corruption bug that - * replaces every byte ≥ 0x80 in raster image data with `0xFD`. SVG is - * text-based, so it sidesteps that code path — @react-pdf parses the - * SVG XML and embeds vector primitives directly into the PDF stream. + * The logo is embedded as SVG because `public/letterhead/.svg` + * IS an SVG source file — @react-pdf parses the XML and embeds vector + * primitives. (An earlier version of this comment blamed a + * "@react-pdf byte-corruption bug that replaces every byte ≥ 0x80 with + * 0xFD" and used SVG as a workaround. The corruption was real but the + * culprit was this app's own render pipeline — `pdf().toString()` + * re-encodes the PDF as latin1, so U+FFFD replacement characters + * truncated to 0xFD. Fixed by using `renderToBuffer()`; see the note + * at the top of `lib/pdf/render.tsx`. Raster logos work again.) * * **Base64, not URL-encoded.** The bundled PDFKit only matches * `^data:.+?;base64,(.*)$` when decoding inline data URIs (see diff --git a/src/lib/pdf/quote-document.tsx b/src/lib/pdf/quote-document.tsx index f4e40ac..9f7f9b9 100644 --- a/src/lib/pdf/quote-document.tsx +++ b/src/lib/pdf/quote-document.tsx @@ -62,20 +62,22 @@ export interface QuoteDocumentProps { items: QuoteItem[]; customer: Customer; letterhead: Letterhead; - /** Logo as a `data:image/svg+xml;charset=utf-8,...` URI. We use SVG - * instead of the original raster (PNG/JPEG) logos because - * @react-pdf/renderer v4.5.1 has a confirmed byte-corruption bug - * that replaces every byte ≥ 0x80 in raster image data with `0xFD`, - * turning valid JPEGs (`0xFF 0xD8` SOI) into garbage. SVG is text- - * based, so it sidesteps the bug entirely — the data URI is a plain - * UTF-8 string and goes through @react-pdf's SVG renderer (which - * parses XML and embeds vector paths), not the broken raster path. + /** Logo as a `data:image/svg+xml;base64,...` URI. * - * If we ever need a non-SVG logo again, the upstream fixes are: - * - Upgrade @react-pdf past the byte-corruption regression - * - Post-process with PDFKit's `doc.image()` (bypasses the bug) - * - Rasterize the SVG→PNG with a server-side library and embed - * the PNG via PDFKit the same way + * SVG because `public/letterhead/*.svg` IS an SVG source file — + * @react-pdf parses the XML and embeds vector primitives. (This + * comment previously described a "@react-pdf v4.5.1 byte-corruption + * bug that replaces every byte >= 0x80 with 0xFD, turning JPEG SOI + * markers into garbage", and listed workarounds. The corruption was + * real; the cause was this app's render pipeline, not @react-pdf — + * `pdf().toString()` re-encoded the PDF as latin1, so U+FFFD + * replacement chars truncated to 0xFD. Fixed by `renderToBuffer()`; + * see the note at the top of `lib/pdf/render.tsx`. Raster logos + * work again, and none of those workarounds are needed.) + * + * Base64, not `;charset=utf-8,`: the bundled PDFKit + * only matches `^data:.+?;base64,(.*)$` for inline data URIs and + * silently drops anything else as a filesystem path. */ logoDataUri: string | null; /** diff --git a/src/lib/pdf/render-image-pipeline.test.ts b/src/lib/pdf/render-image-pipeline.test.ts new file mode 100644 index 0000000..c843710 --- /dev/null +++ b/src/lib/pdf/render-image-pipeline.test.ts @@ -0,0 +1,283 @@ +import { describe, it, expect } from "vitest"; +import zlib from "node:zlib"; +import { renderDeliveryNotePdf } from "./render"; +import type { Letterhead } from "@/lib/letterhead"; + +/** + * Regression guard for the PDF byte pipeline. + * + * ## The bug this exists to stop coming back + * + * Every renderer used to do: + * + * const instance = pdf(); + * const pdfString = await instance.toString(); + * return Buffer.from(pdfString, "binary"); + * + * The stated reason was that `toBuffer()` broke under jsdom. That was + * true of `pdf(el).toBuffer()` — which is a misnamed STREAM getter — but + * the fix chosen for it, `toString()`, was far worse than the problem: + * + * - `toString()` renders with `compress: false`, so every PDF this + * app produced was uncompressed. react-pdf's own source says why: + * "when rendering to string if compress=true the document is + * blank". The method is flagged upstream as buggy (#2112, #2095). + * - Round-tripping the PDF through a JS string and re-encoding as + * latin1 mangles binary streams. Text is ASCII in the content + * stream so it survives; a raster image is a compressed binary + * blob that does not. The image object ends up embedded but is + * never painted. + * + * It stayed latent because the only image in these documents is the + * letterhead logo, and that logo is an SVG — SVG is text, so it + * round-tripped perfectly and hid the whole thing. A signature is the + * first genuinely raster asset this pipeline has to carry. + * + * You cannot "try toBuffer, fall back to toString" to fix this: + * calling `toBuffer()` first consumes the render, and a subsequent + * `toString()` on the same instance returns a document with the + * content missing. One `pdf()` instance, one terminal call. The fix is + * `renderToBuffer()`, which does the right thing in a single pass and + * works under jsdom too. + * + * ## What these tests can and cannot prove + * + * The assertions below are deliberately things that are true in BOTH + * Node and jsdom, because the vitest environment takes a different + * react-pdf image path: under jsdom the embedded image's zlib stream + * will not inflate, even though the exact same document renders a + * perfect signature under plain Node (verified by rasterising the + * output with `pdftoppm` and measuring the pixels). So we do NOT assert + * that image streams inflate — that would be a test that passes in + * production and fails in CI, or worse, one tuned until it is green + * and proves nothing. + * + * Instead we pin the three environment-independent facts that changed: + * the output is compressed, the content stream is valid, and an image + * appears when (and only when) a signature is supplied. + */ + +const letterhead = { + id: "lh1", + collectionId: "pbc_lh", + collectionName: "letterhead_settings", + created: "2026-06-22T20:31:27.165Z", + updated: "2026-06-22T20:31:27.165Z", + display_name: "CNL Mining Supplies", + address: "47 Station Street", + phone: "+27 18 786 2897", + email: "quotes@wfsales.co.za", + vat_number: "4840229449", + vat_percent: 15, + registration_number: "2014/004024/07", + bank_name: "Standard Bank", + bank_account_number: "022196552", + bank_branch_code: "016141", + terms: "", + default_markup_percent: 30, + footer_text: "", + logo: "", + is_active: true, + company: "CNL Mining Supplies (Pty) Ltd", + accent_color: "#738224", +} as unknown as Letterhead; + +/* ---------- a tiny PNG encoder, so the test owns its own fixture ---------- */ + +function crc32(buf: Buffer): number { + const table: number[] = []; + for (let n = 0; n < 256; n++) { + let c = n; + for (let k = 0; k < 8; k++) c = c & 1 ? 0xedb88320 ^ (c >>> 1) : c >>> 1; + table[n] = c >>> 0; + } + let crc = 0xffffffff; + for (const b of buf) crc = table[(crc ^ b) & 0xff] ^ (crc >>> 8); + return (crc ^ 0xffffffff) >>> 0; +} + +function pngChunk(type: string, data: Buffer): Buffer { + const len = Buffer.alloc(4); + len.writeUInt32BE(data.length); + const typed = Buffer.concat([Buffer.from(type, "ascii"), data]); + const crc = Buffer.alloc(4); + crc.writeUInt32BE(crc32(typed)); + return Buffer.concat([len, typed, crc]); +} + +const SIG_W = 120; +const SIG_H = 30; + +/** 120x30 RGBA PNG — a black wave on a transparent background. */ +function makeSignaturePng(): Buffer { + const raw = Buffer.alloc(SIG_H * (1 + SIG_W * 4)); + for (let y = 0; y < SIG_H; y++) { + const rowStart = y * (1 + SIG_W * 4); + raw[rowStart] = 0; // per-scanline filter: none + for (let x = 0; x < SIG_W; x++) { + const wave = 15 + Math.round(Math.sin((x / (SIG_W - 1)) * Math.PI * 4) * 8); + const onStroke = Math.abs(y - wave) <= 2; + const p = rowStart + 1 + x * 4; + raw[p] = 0; + raw[p + 1] = 0; + raw[p + 2] = 0; + raw[p + 3] = onStroke ? 255 : 0; + } + } + const ihdr = Buffer.alloc(13); + ihdr.writeUInt32BE(SIG_W, 0); + ihdr.writeUInt32BE(SIG_H, 4); + ihdr[8] = 8; // bit depth + ihdr[9] = 6; // colour type: RGBA + return Buffer.concat([ + Buffer.from([0x89, 0x50, 0x4e, 0x47, 0x0d, 0x0a, 0x1a, 0x0a]), + pngChunk("IHDR", ihdr), + pngChunk("IDAT", zlib.deflateSync(raw)), + pngChunk("IEND", Buffer.alloc(0)), + ]); +} + +/** + * Walk every FlateDecode stream in the PDF and report whether it + * inflates. Returns one entry per stream, tagged image vs content. + * + * Hand-rolled rather than pulling in a PDF library: the whole point is + * to answer "are these bytes intact", and a parser that can say "this + * one will not inflate" is exactly the signal being asserted. + */ +function scanFlateStreams( + buf: Buffer, +): Array<{ kind: "image" | "content"; inflates: boolean }> { + const s = buf.toString("latin1"); + const out: Array<{ kind: "image" | "content"; inflates: boolean }> = []; + let idx = 0; + for (;;) { + const i = s.indexOf("/FlateDecode", idx); + if (i === -1) break; + const dictEnd = s.indexOf(">>", i) + 2; + const dict = s.slice(Math.max(0, i - 300), dictEnd); + const kind: "image" | "content" = /\/Subtype\s*\/Image/.test(dict) ? "image" : "content"; + const lenMatch = /\/Length (\d+)/.exec(dict); + let st = dictEnd; + while (s[st] === "\n" || s[st] === "\r" || s[st] === " ") st++; + if (s.slice(st, st + 6) === "stream") st += 6; + while (s[st] === "\n" || s[st] === "\r") st++; + const length = lenMatch ? Number(lenMatch[1]) : 200; + const raw = buf.subarray(st, st + length); + let inflates = true; + try { + zlib.inflateSync(raw); + } catch { + inflates = false; + } + out.push({ kind, inflates }); + idx = st + length; + } + return out; +} + +/** Width/Height of every image XObject in the PDF. */ +function imageGeometry(buf: Buffer): Array<{ width: number; height: number }> { + const s = buf.toString("latin1"); + const out: Array<{ width: number; height: number }> = []; + let idx = 0; + for (;;) { + const i = s.indexOf("/Subtype /Image", idx); + if (i === -1) break; + const dict = s.slice(i, s.indexOf(">>", i) + 2); + const w = /\/Width (\d+)/.exec(dict); + const h = /\/Height (\d+)/.exec(dict); + idx = i + 1; + if (w && h) out.push({ width: Number(w[1]), height: Number(h[1]) }); + } + return out; +} + +/** + * Size of the same document through the old `pdf().toString()` path, + * measured when this test was written. Used as the ceiling in the + * compression assertion. + */ +const OLD_UNCOMPRESSED_BYTES = 12710; + +const baseProps = { + header: { + deliveryNoteNumber: "DN26100301", + dateIso: "2026-10-03", + orderNumber: "", + companyName: "Test Mining", + customerName: "Ysterbaard", + }, + lines: [ + { id: "l1", description: "16mm Hex Bolt", qty: 10 }, + { id: "l2", description: "16mm Hex Nut", qty: 10 }, + ], + letterhead, + logoDataUri: null, +}; + +describe("PDF byte pipeline", () => { + it("returns a real Node Buffer, not a string round-trip", async () => { + const buf = await renderDeliveryNotePdf({ ...baseProps }); + expect(Buffer.isBuffer(buf)).toBe(true); + expect(buf.subarray(0, 5).toString("ascii")).toBe("%PDF-"); + }, 30_000); + + it("compresses its streams — the old toString() path produced none", async () => { + // THE regression assertion. `pdf().toString()` renders with + // compress:false, so the old output had zero /FlateDecode streams + // and was ~4x larger. Measured on this document before it grew a + // QR code: + // old (toString) 12710 bytes, 0 Flate streams + // new (renderToBuffer) 3149 bytes, compressed + // The QR added ~2.5 KB of vector content since, so the ceiling is + // expressed against the OLD baseline rather than a round number + // picked when the document was smaller — an absolute byte count + // is a tripwire that fires every time a feature is added, which + // is how a real regression gets tuned away. + const buf = await renderDeliveryNotePdf({ ...baseProps }); + const streams = scanFlateStreams(buf); + expect(streams.length).toBeGreaterThan(0); + expect(buf.toString("latin1")).toContain("/FlateDecode"); + expect(buf.length).toBeLessThan(OLD_UNCOMPRESSED_BYTES); + }, 30_000); + + it("produces a content stream that inflates cleanly", async () => { + // The latin1 round-trip also mangled binary streams — poppler + // reported "Bad FCHECK in flate stream" on the old output. The + // content stream is the one that is environment-independent, so + // this is the check that the bytes actually arrive undamaged. + const buf = await renderDeliveryNotePdf({ ...baseProps }); + const content = scanFlateStreams(buf).filter((st) => st.kind === "content"); + expect(content.length).toBeGreaterThan(0); + for (const st of content) { + expect(st.inflates).toBe(true); + } + }, 30_000); + + it("embeds a raster image with the right geometry when a signature is supplied", async () => { + const buf = await renderDeliveryNotePdf({ + ...baseProps, + signatureImage: `data:image/png;base64,${makeSignaturePng().toString("base64")}`, + }); + const geometry = imageGeometry(buf); + expect(geometry.length).toBeGreaterThan(0); + for (const g of geometry) { + expect(g.width).toBe(SIG_W); + expect(g.height).toBe(SIG_H); + } + }, 30_000); + + it("embeds no image at all when no signature is supplied", async () => { + // The negative control: if this ever fails, something is leaking + // an image into every DN regardless of signature state. + const buf = await renderDeliveryNotePdf({ ...baseProps }); + expect(imageGeometry(buf)).toHaveLength(0); + }, 30_000); + + it("still renders a blank signature line with no signature", async () => { + const buf = await renderDeliveryNotePdf({ ...baseProps }); + expect(buf.subarray(0, 5).toString("ascii")).toBe("%PDF-"); + expect(buf.length).toBeGreaterThan(1024); + }, 30_000); +}); diff --git a/src/lib/pdf/render.test.ts b/src/lib/pdf/render.test.ts index 9070deb..1d5835f 100644 --- a/src/lib/pdf/render.test.ts +++ b/src/lib/pdf/render.test.ts @@ -1,4 +1,5 @@ import { describe, it, expect } from "vitest"; +import zlib from "node:zlib"; import { renderQuotePdf, renderDeliveryNotePdf, @@ -17,6 +18,35 @@ import type { StockItem } from "@/lib/server/stock-items-service"; * dep misconfig, etc.) trips the test even without eyeballing output. */ +/** + * Concatenate the inflated text of every FlateDecode stream in a PDF. + * + * Needed because `renderToBuffer()` compresses: text lives inside + * zlib-compressed content streams, so scanning the raw buffer for a + * string finds nothing. Any assertion of the form "this PDF does / + * does not contain X" has to go through here or it is vacuous. + */ +function decompressPdfText(buf: Buffer): string { + const out: string[] = []; + const marker = Buffer.from("stream"); + let idx = buf.indexOf(marker); + while (idx !== -1) { + let start = idx + marker.length; + // Skip the EOL after the `stream` keyword. + if (buf[start] === 0x0d) start += 1; + if (buf[start] === 0x0a) start += 1; + const end = buf.indexOf(Buffer.from("endstream"), start); + if (end === -1) break; + try { + out.push(zlib.inflateSync(buf.subarray(start, end)).toString("latin1")); + } catch { + // Not every stream is Flate (images may be raw or DCT); skip it. + } + idx = buf.indexOf(marker, end); + } + return out.join("\n"); +} + const baseLetterhead: Letterhead = { id: "lh1", collectionId: "pbc_1010367695", @@ -301,7 +331,13 @@ describe("renderDeliveryNotePdf", () => { letterhead: baseLetterhead, logoDataUri: null, }); - expect(buf.toString("latin1")).not.toContain("Supplier"); + // Grep the DECOMPRESSED text, not the raw bytes. The renderers + // used to funnel through `pdf().toString()`, which renders + // uncompressed — so a raw `buf.toString("latin1")` scan really did + // see the text. `renderToBuffer()` compresses, so the same scan + // would match nothing and this assertion would pass for the wrong + // reason: a test that quietly stops testing is worse than no test. + expect(decompressPdfText(buf)).not.toContain("Supplier"); }, 30_000); it("renders with the ISHERWOOD letterhead code (no accent_color override)", async () => { const buf = await renderDeliveryNotePdf({ diff --git a/src/lib/pdf/render.tsx b/src/lib/pdf/render.tsx index 81c4bff..e450d39 100644 --- a/src/lib/pdf/render.tsx +++ b/src/lib/pdf/render.tsx @@ -1,4 +1,52 @@ -import { pdf } from "@react-pdf/renderer"; +/** + * Server-side PDF rendering. Every renderer here goes through + * `renderToBuffer()` from `@react-pdf/renderer`. + * + * ## Why not `pdf().toString()` + * + * Every renderer used to funnel through + * + * const instance = pdf(); + * const pdfString = await instance.toString(); + * return Buffer.from(pdfString, "binary"); + * + * on the belief that `toBuffer()` was unusable under jsdom (Vitest's + * environment). That belief was half right and led somewhere much + * worse than a test-only problem. + * + * Three things are going on, and only the first was ever real: + * + * 1. `pdf(el).toBuffer()` is NOT the buffer API. It resolves to the + * PDFKit *stream* (react-pdf carries a TODO to rename it + * `toStream`), and under jsdom it resolves to a bare + * `PDFDocument` handle. That is the jsdom breakage the old + * comment described — but it applies to that method, not to the + * module-level `renderToBuffer()` helper, which collects the + * stream into a real Node Buffer and works fine under jsdom. + * + * 2. `toString()` renders with `compress: false`. react-pdf's own + * source says why: "when rendering to string if compress=true + * the document is blank". So every PDF this app produced was + * uncompressed — larger than necessary — and the method is + * flagged upstream as buggy (react-pdf issues #2112, #2095). + * + * 3. The round-trip through a JS string corrupts binary streams. + * A raster image (PNG/JPEG) arrives as a Flate-compressed binary + * blob that does not survive being stringified and re-encoded as + * latin1. The image object ends up embedded in the file but is + * never painted. Text is ASCII in the content stream, so text + * renders fine and the corruption is invisible until something + * raster goes in — which is why this sat latent for so long. The + * letterhead logo hid it completely: that logo is an SVG, and + * SVG is text, so it round-tripped perfectly. + * + * `renderToBuffer()` fixes all three at once and needs no jsdom + * fallback. `render-image-pipeline.test.ts` is the regression guard — + * it renders a real raster image and asserts the bytes come back + * intact, so this cannot silently come back. + */ +import { renderToBuffer } from "@react-pdf/renderer"; +import { buildQrMatrix, type QrMatrix } from "./qr"; import { QuoteDocument, type QuoteDocumentProps } from "./quote-document"; import { PurchaseOrderDocument, @@ -34,23 +82,17 @@ import { /** * Server-side render of a `` to a PDF Buffer. * - * Uses `@react-pdf/renderer`'s Node API (`pdf().toBuffer()`) which - * runs entirely in-process — no headless Chromium, no Playwright/Puppeteer, - * no browser binary in the Docker image. + * Runs entirely in-process via `renderToBuffer()` — no headless + * Chromium, no Playwright/Puppeteer, no browser binary in the Docker + * image. See the note at the top of this file for why that specific + * API and not `pdf().toString()`. * * Always returns a fresh Buffer; never cached. Reads of letterhead, * quote, items, and customer happen in the API route before this is * called, so any edit in PB propagates on the next generation. */ export async function renderQuotePdf(props: QuoteDocumentProps): Promise { - const instance = pdf(); - // `toBuffer()` works in pure Node, but under jsdom (Vitest's default - // test environment) it returns a PDFKit document handle instead. We - // funnel through `toString()` which always resolves to a string of - // raw PDF bytes, then convert to a real Node Buffer. The string path - // is ~the same performance as toBuffer in production. - const pdfString = await instance.toString(); - return Buffer.from(pdfString, "binary"); + return renderToBuffer(); } /** @@ -65,16 +107,12 @@ export async function renderQuotePdf(props: QuoteDocumentProps): Promise * the PO record itself — replaced 2026-07-06 the separate * `purchase_order_pdfs` collection). * - * `toString()` is used for the same jsdom-test compat reason as the - * quote renderer — both routes are test-covered and we want them to - * produce the same shape regardless of environment. + * Same `renderToBuffer()` pipeline as every other renderer here. */ export async function renderPurchaseOrderPdf( props: PurchaseOrderDocumentProps ): Promise { - const instance = pdf(); - const pdfString = await instance.toString(); - return Buffer.from(pdfString, "binary"); + return renderToBuffer(); } /** @@ -82,10 +120,8 @@ export async function renderPurchaseOrderPdf( * * Stock POs are pure stock-replenishment orders with no customer in * the loop — the document layout is a slimmed-down version of the - * customer-PO PDF (no Order Ref line, no "For: "). The - * same `toString()` jsdom-compat pattern is used so the stock-PO - * PDF route's tests run under the same setup as the other PDF - * routes. + * customer-PO PDF (no Order Ref line, no "For: "). Same + * `renderToBuffer()` pipeline as the rest of this file. * * The route drops the resulting Buffer into * `stock_purchases.documents` (file, multi) + appends a metadata @@ -97,9 +133,7 @@ export async function renderPurchaseOrderPdf( export async function renderStockPurchasePdf( props: StockPurchaseDocumentProps ): Promise { - const instance = pdf(); - const pdfString = await instance.toString(); - return Buffer.from(pdfString, "binary"); + return renderToBuffer(); } /** @@ -115,9 +149,20 @@ export async function renderStockPurchasePdf( * the same item. */ export async function renderDeliveryNotePdf(props: DeliveryNoteDocumentProps): Promise { - const instance = pdf(); - const pdfString = await instance.toString(); - return Buffer.from(pdfString, "binary"); + // Every delivery note carries a QR of its own number so the + // warehouse/driver scanner app can resolve the note straight off + // the printed page. Built here rather than in the document so the + // QR encoder stays out of the presentational component. + // + // Non-fatal: a QR that fails to encode must never cost the operator + // their delivery note. The document renders without it. + let qr: QrMatrix | null = null; + try { + qr = buildQrMatrix(props.header.deliveryNoteNumber); + } catch (err) { + console.error("[renderDeliveryNotePdf] QR build failed:", err); + } + return renderToBuffer(); } /** @@ -135,25 +180,20 @@ export async function renderDeliveryNotePdf(props: DeliveryNoteDocumentProps): P export async function renderRepairRequestPdf( props: RepairRequestDocumentProps ): Promise { - const instance = pdf(); - const pdfString = await instance.toString(); - return Buffer.from(pdfString, "binary"); + return renderToBuffer(); } /** * Server-side render of the stock-orders *list* PDF — every * purchase in one document, not one PDF per PO. Sibling of * `renderStockPurchasePdf` (which is the single-PO, supplier-facing - * PDF). Both walk `@react-pdf/renderer`'s `pdf().toString()` - * pipeline so the test suite runs under jsdom without dragging in - * a Chromium binary. + * PDF). Both use the same `renderToBuffer()` pipeline, which needs no + * browser binary. */ export async function renderStockPurchaseListPdf( props: StockPurchaseListDocumentProps ): Promise { - const instance = pdf(); - const pdfString = await instance.toString(); - return Buffer.from(pdfString, "binary"); + return renderToBuffer(); } /** @@ -163,9 +203,7 @@ export async function renderStockPurchaseListPdf( export async function renderRepPerformanceDetailPdf( props: RepPerformanceDetailDocumentProps ): Promise { - const instance = pdf(); - const pdfString = await instance.toString(); - return Buffer.from(pdfString, "binary"); + return renderToBuffer(); } /** @@ -175,9 +213,7 @@ export async function renderRepPerformanceDetailPdf( export async function renderRepPerformanceTeamPdf( props: RepPerformanceTeamDocumentProps ): Promise { - const instance = pdf(); - const pdfString = await instance.toString(); - return Buffer.from(pdfString, "binary"); + return renderToBuffer(); } /** @@ -189,7 +225,5 @@ export async function renderRepPerformanceTeamPdf( export async function renderStockItemStocktakePdf( props: StockItemStocktakeDocumentProps ): Promise { - const instance = pdf(); - const pdfString = await instance.toString(); - return Buffer.from(pdfString, "binary"); + return renderToBuffer(); } \ No newline at end of file diff --git a/src/lib/pdf/repair-request-document.tsx b/src/lib/pdf/repair-request-document.tsx index 11b78b7..c1f795c 100644 --- a/src/lib/pdf/repair-request-document.tsx +++ b/src/lib/pdf/repair-request-document.tsx @@ -27,10 +27,11 @@ export type RepairRequestDocumentProps = { contactEmail: string; letterhead: Letterhead; /** - * Logo as a `data:image/svg+xml;base64,...` URI. Same SVG workaround - * as the quote / PO / DN documents — @react-pdf v4.5.1 corrupts every - * byte ≥ 0x80 in raster image data, but SVG is text and goes through - * the SVG renderer instead. + * Logo as a `data:image/svg+xml;base64,...` URI. SVG because + * `public/letterhead/*.svg` IS an SVG source file — this is not a + * workaround for a react-pdf raster bug (there wasn't one; see the + * note at the top of `lib/pdf/render.tsx` for what that mistaken + * belief led to). */ logoDataUri: string | null; }; diff --git a/src/lib/server/quote-pdf-service.ts b/src/lib/server/quote-pdf-service.ts index 0405e22..06ff0e6 100644 --- a/src/lib/server/quote-pdf-service.ts +++ b/src/lib/server/quote-pdf-service.ts @@ -391,9 +391,11 @@ export async function generateQuotePdf( : null; // ── 5. Render ──────────────────────────────────────────── - // Letterhead + logo (SVG as a data URI). @react-pdf v4.5.1 - // corrupts any raster (PNG/JPEG) byte ≥ 0x80, so we ship SVG - // instead. + // Letterhead + logo. The logo ships as an SVG data URI because + // `public/letterhead/*.svg` is an SVG source file — the raster + // corruption this comment used to blame on @react-pdf was + // actually the old `pdf().toString()` render pipeline. See the + // note at the top of `lib/pdf/render.tsx`. const letterhead = await fetchLetterhead(letterheadCode); const logoDataUri = await loadLetterheadLogoDataUri(letterheadCode); From ad869aa6f835999877cae0ef52db124810728f9f Mon Sep 17 00:00:00 2001 From: Dave Date: Sun, 4 Oct 2026 13:15:08 +0200 Subject: [PATCH 2/5] fix(pdf-print): open the PDF in a new tab instead of printing the iframe MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `iframe.contentWindow.print()` does not work for a PDF in Chrome. Chrome backs every PDF — src URL or blob URL — with its internal `pdf_embedder` extension rather than a real DOM document, so the frame's `contentDocument` is an empty shell: The PDF *paints* — you can see it in the preview — but `print()` runs against that empty body, so the operator gets a print dialog full of blank pages and no error to explain why. Read straight out of a live page to confirm. A new tab gets Chrome's real PDF viewer, whose own print button works. Same document, no blank pages. Affects the delivery-note preview, the quote PDF viewer, and the second quote viewer in the same file. Popup-blocked is now reported explicitly rather than appearing to do nothing. --- .../delivery-notes/preview-dialog.tsx | 109 +++++++++++++----- src/components/quote-pdf-viewer.tsx | 23 ++-- 2 files changed, 91 insertions(+), 41 deletions(-) diff --git a/src/components/delivery-notes/preview-dialog.tsx b/src/components/delivery-notes/preview-dialog.tsx index 6015053..e2ffc3e 100644 --- a/src/components/delivery-notes/preview-dialog.tsx +++ b/src/components/delivery-notes/preview-dialog.tsx @@ -11,8 +11,9 @@ * The dialog is intentionally chrome-minimal so the PDF * takes most of the viewport. The footer has: * - Close - * - Print (calls iframe.contentWindow.print() so the - * operator prints just the PDF, not the whole page) + * - Print (opens the PDF in a new tab — see the note on + * `handlePrint` for why the iframe's own print() gives blank + * pages) * - Download (uses the `download` attribute so the * browser saves the file rather than opening it in a * viewer) @@ -67,28 +68,77 @@ export function PdfPreviewDialog({ subtitle, downloadFilename, }: PdfPreviewDialogProps) { - // Ref on the iframe so Print can call the iframe's own - // print() — this prints just the PDF, not the whole page - // (the parent page has the modal chrome which would also - // print otherwise). + // Ref on the iframe so we can tell whether the PDF has finished + // loading before offering a print action. const iframeRef = useRef(null); + const [blocked, setBlocked] = useState(false); + + // Clear the pop-up-blocked notice on close rather than in an effect: + // syncing state from an effect triggers a second render pass, and + // the flag is only ever set by a click inside this component. + const handleOpenChange = (next: boolean) => { + if (!next) setBlocked(false); + onOpenChange(next); + }; const handlePrint = () => { - // Calling the iframe's contentWindow.print() prints the - // PDF document inside the iframe. Works in Chrome, Edge, - // Firefox, Safari. If the iframe didn't load (network - // blip, broken PDF), the call is a no-op — the operator - // can still use the Download button. - try { - iframeRef.current?.contentWindow?.print(); - } catch { - // Ignore — some browsers throw on cross-origin or - // blocked-iframe cases. The Download button remains. + // Deliberately opens the PDF in a new tab rather than calling + // `iframe.contentWindow.print()`. + // + // That older approach does NOT work for a PDF in Chrome. Chrome + // renders PDFs with its internal `pdf_embedder` extension, not as + // a real DOM document, so the iframe's `contentDocument` is an + // empty shell: + // + // + // + // + // The PDF *paints* (you can see it), but `print()` runs against + // that empty body — so the operator gets a print dialog full of + // blank pages and no error to explain why. Verified in Chrome by + // reading the iframe's document straight out of the page. + // + // A new tab hands the operator Chrome's real PDF viewer, whose own + // print button works. Same document, no blank pages. + const url = pdfUrl; + const win = window.open(url, "_blank", "noopener,noreferrer"); + if (!win) { + // Popup blocked. Say so rather than appearing to do nothing. + setBlocked(true); } }; + if (blocked) { + return ( + + + + Pop-up blocked + + Your browser blocked the new tab needed to print {title}. Allow + pop-ups for this site, or use Download to save the PDF and open it + directly. + + + + + + + + + + + ); + } + return ( - + onOpenChange(false)} + onClick={() => handleOpenChange(false)} aria-label="Close preview" data-testid="pdf-preview-close" > @@ -137,14 +187,15 @@ export function PdfPreviewDialog({ - {/* Print — calls the iframe's own print() so the - operator prints just the PDF, not the page - chrome behind the modal. */} + {/* Print — opens the PDF in a new tab so the operator gets + Chrome's real PDF viewer, whose print button works. See + the note on `handlePrint` for why the iframe's own + `print()` produces blank pages. */}