fix(pdf): RFC 6266-encode Content-Disposition on quote/invoice PDFs (#1024) (#1055)

* fix(pdf): RFC 6266-encode Content-Disposition on quote/invoice PDFs (#1024)

The six quote/invoice PDF endpoints interpolated buildPdfFilename()'s result
straight into `inline; filename="${filename}"`. That result deliberately
preserves non-ASCII (it doubles as the PDF's internal Title metadata), and
HTTP header values are latin1 — so a customer label reaching the header
directly failed in one of two ways:

  - U+0080-U+00FF (ä ö ü ß — every German umlaut): no throw. The raw byte
    goes out and the client reads back a mangled name. Silent corruption.
  - above U+00FF (Polish ł, Czech ř, Turkish ş, €, Cyrillic, CJK, emoji):
    Node's setHeader rejects it with ERR_INVALID_CHAR. The throw lands
    after the PDF buffer is already rendered, so the request 500s.

Note this corrects the issue's diagnosis: it reported umlauts as the 500
case, but umlauts are inside latin1 and mangle rather than throw. Both
symptoms share this root cause and both are fixed here.

Route through buildContentDisposition() (utils/filenameSanitizer, already
used by secureImages.js), which emits an ASCII fallback plus the RFC 5987
`filename*=UTF-8''…` form, so the unicode name survives in browsers and the
header stays legal. Applied to all six sites: adminQuotes (persisted +
preview), adminInvoices (persisted + preview), customer (quote + invoice).

Also correct buildPdfFilename's docstring, which advertised the preserved
non-ASCII as suitable for Content-Disposition — the exact misreading that
produced these call sites.

* test(pdf): pin the ASCII fallback for fully non-Latin customer names (#1024)

A name written entirely in another script leaves the legacy filename= token
with just the document number (Q-2026-0042_.pdf) — filename* carries the real
name. That's the intended trade, but it's the token a client without RFC 5987
support actually saves, so assert it stays legal, non-empty and carries the
document number rather than leaving it unpinned.

* fix(pdf): don't split surrogate pairs when truncating the filename (#1024)

Codex review caught this. sanitiseSegment caps each segment at 80 UTF-16 code
units, so a cap landing inside an astral character (emoji, rarer CJK) left a
dangling high surrogate. encodeURIComponent throws URIError: URI malformed on
a lone surrogate, so buildContentDisposition — the helper this PR routes the
six PDF endpoints through — 500'd for e.g. company_name = 'a'.repeat(79)+'🎉',
well inside the 120-char validator limit. Same 500 the PR set out to remove,
reached a different way.

Drop the orphaned surrogate instead of widening the cap, so the byte budget
the limit exists to protect is unchanged. Tests cover both boundary cases and
assert the cap semantics; they fail against the previous slice().

---------

Co-authored-by: Paul Nothaft <paul@MacStudio-von-Paul.local>

backend/__tests__/utils/pdfContentDisposition.test.js

@@ -0,0 +1,143 @@
+/**
+ * Regression test for #1024: quote/invoice PDF endpoints 500'd (or silently
+ * corrupted the filename) for customers whose name carries non-ASCII.
+ *
+ * The six PDF routes built the header by interpolating buildPdfFilename()'s
+ * result straight into `inline; filename="${filename}"`. HTTP header values
+ * are latin1, which splits the failure in two — and the split matters,
+ * because the issue reported the umlaut case as the 500 and it isn't:
+ *
+ *   U+0080-U+00FF  (ä ö ü ß — every German umlaut)
+ *       No throw. The byte goes out raw and the client reads back a mangled
+ *       name. A silent corruption, not an error.
+ *
+ *   above U+00FF   (Polish ł, Czech ř, Turkish ş, €, Cyrillic, CJK, emoji)
+ *       Node's setHeader rejects it with ERR_INVALID_CHAR. Because the
+ *       throw lands after the PDF buffer is already rendered, the whole
+ *       request fails as an unhandled 500.
+ *
+ * buildContentDisposition() fixes both: an ASCII fallback for the legacy
+ * `filename=` parameter plus the RFC 5987 `filename*=UTF-8''…` form that
+ * carries the real name.
+ *
+ * These assertions run against the real Node header validator via a live
+ * express server, so they'd fail against the old interpolation rather than
+ * merely testing the helper in isolation.
+ */
+const express = require('express');
+const request = require('supertest');
+
+const { buildPdfFilename, sanitiseSegment } = require('../../src/utils/pdfFilename');
+const { buildContentDisposition } = require('../../src/utils/filenameSanitizer');
+
+// The RFC 5987 parameter prefix, i.e. filename*=UTF-8'' — the two trailing
+// quotes are the (empty) language tag the spec puts between the charset and
+// the percent-encoded value.
+const RFC5987_PREFIX = 'filename*=UTF-8\'\'';
+
+// Mirrors what the six PDF routes now do.
+function buildApp(customer, docNumber = 'Q-2026-0042') {
+  const app = express();
+  app.get('/pdf', (req, res) => {
+    const filename = buildPdfFilename({ docNumber, customer, fallback: 'quote-preview' });
+    res.set('Content-Type', 'application/pdf');
+    res.set('Content-Disposition', buildContentDisposition(filename, 'inline'));
+    res.send(Buffer.from('%PDF-1.4 fake'));
+  });
+  // Mirrors the real error handler: an ERR_INVALID_CHAR throw inside the
+  // handler surfaces as a 500, which is what #1024 reported.
+  // eslint-disable-next-line no-unused-vars
+  app.use((err, req, res, next) => res.status(500).json({ error: err.code || err.message }));
+  return app;
+}
+
+describe('#1024 — PDF Content-Disposition with non-ASCII customer names', () => {
+  it('serves a PDF for a German umlaut name and keeps the name intact', async () => {
+    const res = await request(buildApp({ company_name: 'Müller Fotografie' })).get('/pdf');
+
+    expect(res.status).toBe(200);
+    const cd = res.headers['content-disposition'];
+    // RFC 5987 form carries the real, unmangled name...
+    expect(cd).toContain(RFC5987_PREFIX);
+    expect(cd).toContain(encodeURIComponent('Müller-Fotografie.pdf'));
+    // ...and the ASCII fallback is legal latin1 with no raw umlaut byte.
+    const fallback = /filename="([^"]+)"/.exec(cd)[1];
+    expect(fallback).toMatch(/^[\x20-\x7e]+$/);
+  });
+
+  it.each([
+    ['Polish',   'Michał Kowalski'],
+    ['Czech',    'Dvořák Studio'],
+    ['Turkish',  'Şahin Fotoğraf'],
+    ['Cyrillic', 'Иванов Фото'],
+    ['CJK',      '山田写真'],
+    ['emoji',    'Studio 🎉 Berlin'],
+  ])('does not 500 for a %s customer name (was ERR_INVALID_CHAR)', async (_label, company) => {
+    const res = await request(buildApp({ company_name: company })).get('/pdf');
+
+    expect(res.status).toBe(200);
+    const cd = res.headers['content-disposition'];
+    expect(cd).toContain(RFC5987_PREFIX);
+    // The legacy filename= token drops non-ASCII, so a name written entirely
+    // in another script degrades to just the document number
+    // (`Q-2026-0042_.pdf`). That's the intended trade — filename* carries the
+    // real name — but the fallback must still be a legal, non-empty,
+    // ASCII-only token, since that is what a client without RFC 5987 support
+    // ends up saving.
+    const fallback = /filename="([^"]*)"/.exec(cd)[1];
+    expect(fallback.length).toBeGreaterThan(0);
+    expect(fallback).toMatch(/^[\x20-\x7e]+$/);
+    expect(fallback).toContain('Q-2026-0042');
+  });
+
+  it('leaves a plain ASCII name on the familiar filename= form', async () => {
+    const res = await request(buildApp({ company_name: 'Bright Studio' })).get('/pdf');
+
+    expect(res.status).toBe(200);
+    expect(res.headers['content-disposition'])
+      .toContain('filename="Q-2026-0042_Bright-Studio.pdf"');
+  });
+
+  it('still works when the customer row is missing entirely (preview path)', async () => {
+    const res = await request(buildApp(null, null)).get('/pdf');
+
+    expect(res.status).toBe(200);
+    expect(res.headers['content-disposition']).toContain('quote-preview_customer.pdf');
+  });
+
+  // sanitiseSegment caps each segment at 80 UTF-16 code units. A cap landing
+  // inside an astral character used to leave a dangling high surrogate, which
+  // makes encodeURIComponent throw URIError inside buildContentDisposition —
+  // a 500 on the very endpoint this PR fixes, reached a different way.
+  it.each([
+    ['emoji on the 80-char boundary',      `${'a'.repeat(79)}🎉`],
+    ['astral CJK on the boundary',         `${'a'.repeat(79)}𠜎`],
+    ['a label that is entirely astral',    '🎉'.repeat(60)],
+  ])('does not 500 when truncation splits a surrogate pair — %s', async (_label, company) => {
+    const res = await request(buildApp({ company_name: company })).get('/pdf');
+
+    expect(res.status).toBe(200);
+    expect(res.headers['content-disposition']).toContain(RFC5987_PREFIX);
+  });
+
+  it('drops the orphaned surrogate rather than widening the length cap', () => {
+    const seg = sanitiseSegment(`${'a'.repeat(79)}🎉`);
+
+    // 79 'a's + a half-emoji would be 80; the orphan is dropped, not kept.
+    expect(seg).toHaveLength(79);
+    expect(seg).toBe('a'.repeat(79));
+    // Nothing in the result may be an unpaired surrogate.
+    expect(seg).toBe(seg.replace(/[\uD800-\uDBFF](?![\uDC00-\uDFFF])/g, ''));
+  });
+
+  it('the raw interpolation these routes used to do really does throw', () => {
+    // Pins the root cause itself, so nobody "simplifies" the helper away.
+    const filename = buildPdfFilename({
+      docNumber: 'Q-2026-0042',
+      customer: { company_name: 'Michał Kowalski' },
+    });
+    const res = new (require('http').ServerResponse)({});
+    expect(() => res.setHeader('Content-Disposition', `inline; filename="${filename}"`))
+      .toThrow(/ERR_INVALID_CHAR|Invalid character/);
+  });
+});

backend/src/routes/adminInvoices.js

@@ -921,6 +921,7 @@ router.get(
     // re-fetching here keeps the route a thin shim over the
     // service rather than reaching inside its internals.
     const { buildPdfFilename } = require('../utils/pdfFilename');
+    const { buildContentDisposition } = require('../utils/filenameSanitizer');
     const inv = await db('invoices').where({ id }).first();
     const customer = inv ? await db('customer_accounts').where({ id: inv.customer_account_id }).first() : null;
     const filename = buildPdfFilename({
@@ -929,7 +930,7 @@ router.get(
       fallback: `invoice-${id}`,
     });
     res.set('Content-Type', 'application/pdf');
-    res.set('Content-Disposition', `inline; filename="${filename}"`);
+    res.set('Content-Disposition', buildContentDisposition(filename, 'inline'));
     res.send(buf);
   })
 );
@@ -946,6 +947,7 @@ router.post(
     // the customer so the filename still reflects who the invoice
     // is for; the number segment falls back to "invoice-preview".
     const { buildPdfFilename } = require('../utils/pdfFilename');
+    const { buildContentDisposition } = require('../utils/filenameSanitizer');
     const customer = payload.customerAccountId
       ? await db('customer_accounts').where({ id: payload.customerAccountId }).first()
       : null;
@@ -955,7 +957,7 @@ router.post(
       fallback: 'invoice-preview',
     });
     res.set('Content-Type', 'application/pdf');
-    res.set('Content-Disposition', `inline; filename="${filename}"`);
+    res.set('Content-Disposition', buildContentDisposition(filename, 'inline'));
     res.send(buf);
   })
 );

backend/src/routes/adminQuotes.js

@@ -537,6 +537,7 @@ router.get(
     const id = parseInt(req.params.id, 10);
     const buf = await quoteService.renderQuotePdfBuffer(id);
     const { buildPdfFilename } = require('../utils/pdfFilename');
+    const { buildContentDisposition } = require('../utils/filenameSanitizer');
     const quote = await db('quotes').where({ id }).first();
     const customer = quote ? await db('customer_accounts').where({ id: quote.customer_account_id }).first() : null;
     const filename = buildPdfFilename({
@@ -545,7 +546,7 @@ router.get(
       fallback: `quote-${id}`,
     });
     res.set('Content-Type', 'application/pdf');
-    res.set('Content-Disposition', `inline; filename="${filename}"`);
+    res.set('Content-Disposition', buildContentDisposition(filename, 'inline'));
     res.send(buf);
   })
 );
@@ -559,6 +560,7 @@ router.post(
     const payload = mapPayloadToService(req.body);
     const buf = await quoteService.renderQuotePdfFromPayload(payload);
     const { buildPdfFilename } = require('../utils/pdfFilename');
+    const { buildContentDisposition } = require('../utils/filenameSanitizer');
     const customer = payload.customerAccountId
       ? await db('customer_accounts').where({ id: payload.customerAccountId }).first()
       : null;
@@ -568,7 +570,7 @@ router.post(
       fallback: 'quote-preview',
     });
     res.set('Content-Type', 'application/pdf');
-    res.set('Content-Disposition', `inline; filename="${filename}"`);
+    res.set('Content-Disposition', buildContentDisposition(filename, 'inline'));
     res.send(buf);
   })
 );

backend/src/routes/customer.js

@@ -566,6 +566,7 @@ router.get('/quotes/:id/pdf', customerAuth, async (req, res) => {
     const quoteService = require('../services/quoteService');
     const buf = await quoteService.renderQuotePdfBuffer(quote.id);
     const { buildPdfFilename } = require('../utils/pdfFilename');
+    const { buildContentDisposition } = require('../utils/filenameSanitizer');
     const customer = await dbi('customer_accounts').where({ id: req.customer.id }).first();
     const filename = buildPdfFilename({
       docNumber: quote.quote_number,
@@ -573,7 +574,7 @@ router.get('/quotes/:id/pdf', customerAuth, async (req, res) => {
       fallback: `quote-${quote.id}`,
     });
     res.set('Content-Type', 'application/pdf');
-    res.set('Content-Disposition', `inline; filename="${filename}"`);
+    res.set('Content-Disposition', buildContentDisposition(filename, 'inline'));
     res.send(buf);
   } catch (error) {
     errorResponse(res, error, 500, 'Failed to render quote PDF');
@@ -597,6 +598,7 @@ router.get('/invoices/:id/pdf', customerAuth, async (req, res) => {
     const invoiceService = require('../services/invoiceService');
     const buf = await invoiceService.renderInvoicePdfBuffer(invoice.id);
     const { buildPdfFilename } = require('../utils/pdfFilename');
+    const { buildContentDisposition } = require('../utils/filenameSanitizer');
     const customer = await dbi('customer_accounts').where({ id: req.customer.id }).first();
     const filename = buildPdfFilename({
       docNumber: invoice.invoice_number,
@@ -604,7 +606,7 @@ router.get('/invoices/:id/pdf', customerAuth, async (req, res) => {
       fallback: `invoice-${invoice.id}`,
     });
     res.set('Content-Type', 'application/pdf');
-    res.set('Content-Disposition', `inline; filename="${filename}"`);
+    res.set('Content-Disposition', buildContentDisposition(filename, 'inline'));
     res.send(buf);
   } catch (error) {
     errorResponse(res, error, 500, 'Failed to render invoice PDF');

backend/src/utils/pdfFilename.js

@@ -20,6 +20,19 @@
  *   - The PDF's internal `Title` metadata (Chrome's PDF viewer
  *     uses this as the default name when saving from a blob URL,
  *     where Content-Disposition can't reach)
+ *
+ * IMPORTANT (#1024): the preserved non-ASCII is exactly what a raw
+ * `filename="${...}"` header cannot carry. HTTP header values are
+ * latin1, so a customer label reaching a header directly either
+ * mangles (U+0080-U+00FF — every German umlaut: `Müller` is sent as
+ * the byte 0xFC and read back as garbage) or throws ERR_INVALID_CHAR
+ * and 500s the request (anything above U+00FF — Polish ł, Czech ř,
+ * Turkish ş, €, Cyrillic, CJK, emoji).
+ *
+ * Never interpolate this result into a header. Pass it through
+ * `buildContentDisposition()` in utils/filenameSanitizer, which emits
+ * an ASCII fallback plus the RFC 5987 `filename*=UTF-8''…` form so the
+ * unicode name survives in browsers and the header stays legal.
  */
 
 function sanitiseSegment(input, maxLen = 80) {
@@ -33,7 +46,18 @@ function sanitiseSegment(input, maxLen = 80) {
   s = s.replace(/-+/g, '-');
   // Trim leading/trailing dashes + dots.
   s = s.replace(/^[-.]+|[-.]+$/g, '');
-  if (s.length > maxLen) s = s.slice(0, maxLen);
+  if (s.length > maxLen) {
+    s = s.slice(0, maxLen);
+    // slice() cuts UTF-16 code units, so a boundary landing inside an astral
+    // character (emoji, rarer CJK) leaves a dangling high surrogate. That is
+    // not merely cosmetic: the lone surrogate makes encodeURIComponent throw
+    // `URIError: URI malformed` inside buildContentDisposition, which 500s
+    // the PDF endpoint — the exact failure #1024 set out to remove, just via
+    // a different route. Drop the orphan rather than widening the cap, so the
+    // byte budget this limit exists to protect is unchanged.
+    const lastUnit = s.charCodeAt(s.length - 1);
+    if (lastUnit >= 0xD800 && lastUnit <= 0xDBFF) s = s.slice(0, -1);
+  }
   return s;
 }