Stable backport of #1055 (main: 3a11e6eb). Change content is byte-identical
to the main twin; cherry-picked clean, no resolutions needed.
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.
This corrects the issue's diagnosis: it reported umlauts as the 500 case,
but umlauts are inside latin1 and mangle rather than throw.
Route all six through buildContentDisposition(), which emits an ASCII
fallback plus the RFC 5987 filename*=UTF-8'' form. Also stops sanitiseSegment
splitting surrogate pairs at its 80-unit cap — a dangling high surrogate makes
encodeURIComponent throw URIError inside the helper, reaching the same 500 a
different way (found by external review on the main twin).
Verified on this branch: 14/14 in the new suite, 151/151 across the nine
surrounding pdf/filename/quote/invoice suites, lint clean.
Co-authored-by: Paul Nothaft <[email protected]>
This commit is contained in:
co-authored by
Paul Nothaft
parent
88fa3c5297
commit
376311cb90
@@ -894,6 +894,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({
|
||||
@@ -902,7 +903,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);
|
||||
})
|
||||
);
|
||||
@@ -919,6 +920,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;
|
||||
@@ -928,7 +930,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);
|
||||
})
|
||||
);
|
||||
|
||||
@@ -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);
|
||||
})
|
||||
);
|
||||
|
||||
@@ -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');
|
||||
|
||||
@@ -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;
|
||||
}
|
||||
|
||||
|
||||
Reference in New Issue
Block a user