diff --git a/craft/src/utils/report-payload.test.ts b/craft/src/utils/report-payload.test.ts index c90acef..0995279 100644 --- a/craft/src/utils/report-payload.test.ts +++ b/craft/src/utils/report-payload.test.ts @@ -65,3 +65,41 @@ describe('buildReportPayload', () => { expect(p.canvas_state_omitted).toBeUndefined(); }); }); + +describe('description size bounding', () => { + const hugeDescription = 'y'.repeat(600 * 1024); + + test('oversized description does not blow the cap with canvas included', () => { + const p = buildReportPayload({ ...base, description: hugeDescription }); + expect(new Blob([JSON.stringify(p)]).size).toBeLessThanOrEqual(MAX_PAYLOAD_BYTES); + }); + + test('oversized description does not blow the cap when opted out', () => { + const p = buildReportPayload({ ...base, description: hugeDescription, includeCanvas: false }); + expect(new Blob([JSON.stringify(p)]).size).toBeLessThanOrEqual(MAX_PAYLOAD_BYTES); + expect(p.canvas_state).toBeNull(); + expect(p.canvas_state_omitted).toBe('opt-out'); + }); + + test('oversized description does not blow the cap with no canvas state', () => { + const p = buildReportPayload({ ...base, description: hugeDescription, canvasState: null }); + expect(new Blob([JSON.stringify(p)]).size).toBeLessThanOrEqual(MAX_PAYLOAD_BYTES); + expect(p.canvas_state).toBeNull(); + expect(p.canvas_state_omitted).toBeUndefined(); + }); + + test('description is truncated to 5000 characters', () => { + const p = buildReportPayload({ ...base, description: hugeDescription }); + expect(p.description.length).toBeLessThanOrEqual(5000); + }); +}); + +describe('honest size markers', () => { + test('the size marker never appears on a payload that is still oversized after dropping canvas_state', () => { + // consoleErrors is normally bounded upstream (console-buffer.ts caps it at + // 20 entries x 500 chars), but this function must not trust that -- it's + // just an array of the exported type as far as the signature is concerned. + const massiveErrors = Array.from({ length: 5000 }, (_, i) => ({ ts: i, message: 'x'.repeat(200) })); + expect(() => buildReportPayload({ ...base, consoleErrors: massiveErrors })).toThrow(); + }); +}); diff --git a/craft/src/utils/report-payload.ts b/craft/src/utils/report-payload.ts index 86254b5..e8955a2 100644 --- a/craft/src/utils/report-payload.ts +++ b/craft/src/utils/report-payload.ts @@ -15,6 +15,18 @@ import type { ConsoleErrorEntry } from './console-buffer'; export const MAX_PAYLOAD_BYTES = 512 * 1024; +/** + * Matches the limit the server-side validator enforces (a later task) so the + * client never builds a body the server would reject outright. Truncated + * silently, not flagged: unlike canvas_state (a serialized tree, where a cut + * mid-structure looks like a smaller-but-still-valid tree and actively + * misleads whoever reads it), a truncated free-text description is exactly + * what it looks like -- text that stops partway through. Nothing about the + * cut invents false structure, and the limit mirrors what the server would + * have discarded anyway. + */ +export const MAX_DESCRIPTION_CHARS = 5000; + export type ReportCategory = 'bug' | 'confusing' | 'feature'; export interface BuildReportPayloadInput { @@ -53,14 +65,44 @@ export interface ReportPayload { } function byteLength(value: string): number { + // TextEncoder is standard in every environment this module actually runs + // in (Node/vitest and every real browser target). The fallback below is + // unreachable by design -- kept only so a missing global degrades to a + // conservative-ish count rather than throwing, not because it's expected + // to fire. (It undercounts multi-byte UTF-8, so treat it as dead code.) if (typeof TextEncoder !== 'undefined') return new TextEncoder().encode(value).length; return value.length; } +function payloadBytes(payload: ReportPayload): number { + return byteLength(JSON.stringify(payload)); +} + +/** + * Every return path funnels through here so the cap is checked + * unconditionally, not just on the canvas-included branch. If the payload + * is still over budget after every available reduction (dropping + * canvas_state, truncating description), there is nothing left to cut -- + * returning it anyway would ship an oversized body that, if it carries + * `canvas_state_omitted: 'size'`, falsely claims the drop fixed things. + * Throwing surfaces that as a distinct, honest failure instead. + */ +function finalize(payload: ReportPayload): ReportPayload { + if (payloadBytes(payload) > MAX_PAYLOAD_BYTES) { + throw new Error( + `Report payload is ${payloadBytes(payload)} bytes, over the ${MAX_PAYLOAD_BYTES}-byte cap, ` + + 'even with canvas_state dropped. Nothing left to reduce.' + ); + } + return payload; +} + export function buildReportPayload(input: BuildReportPayloadInput): ReportPayload { - const payload: ReportPayload = { + const description = input.description.trim().slice(0, MAX_DESCRIPTION_CHARS); + + const base: ReportPayload = { category: input.category, - description: input.description.trim(), + description, site_id: input.siteId, site_domain: input.siteDomain, page_id: input.pageId, @@ -75,16 +117,22 @@ export function buildReportPayload(input: BuildReportPayloadInput): ReportPayloa }; if (!input.includeCanvas) { - payload.canvas_state_omitted = 'opt-out'; - return payload; + // The opt-out promise is inviolable: canvas_state is never populated + // from input.canvasState on this path, no matter what else changes + // below it. Only the cap is checked here, not whether to include canvas. + return finalize({ ...base, canvas_state_omitted: 'opt-out' }); } - if (!input.canvasState) return payload; + if (!input.canvasState) return finalize(base); - payload.canvas_state = input.canvasState; - if (byteLength(JSON.stringify(payload)) > MAX_PAYLOAD_BYTES) { - payload.canvas_state = null; - payload.canvas_state_omitted = 'size'; - } - return payload; + const withCanvas: ReportPayload = { ...base, canvas_state: input.canvasState }; + if (payloadBytes(withCanvas) <= MAX_PAYLOAD_BYTES) return withCanvas; + + // canvas_state alone pushed this over budget: drop it whole (never + // truncate -- a partial Craft tree looks valid but is missing nodes, + // which is worse than no tree) and re-measure. The 'size' marker is only + // attached once we've confirmed dropping the canvas actually brought the + // payload back under the cap; finalize() throws instead of returning it + // if it didn't. + return finalize({ ...base, canvas_state: null, canvas_state_omitted: 'size' }); }