diff --git a/docs/guides/web/browser-scenario-contract.md b/docs/guides/web/browser-scenario-contract.md index d403b4de9a..9c4b58e1af 100644 --- a/docs/guides/web/browser-scenario-contract.md +++ b/docs/guides/web/browser-scenario-contract.md @@ -531,7 +531,10 @@ id the application assigns: start origin as the browser spells it, so a path may name `My List` or `My%20List` (`{*}` matches one segment, and the query is ignored), and fails the step unless a 2xx JSON response arrives before its `timeout_ms`; one still - being read when the step times out or is cancelled saves nothing. A pointer + being read when the step times out or is cancelled saves nothing. Saved values + are committed only while the step remains active, after response reading and + any request-route cleanup succeed. Cancellation after that commit retains + them; a physical request or cleanup failure keeps its original reason. A pointer that names no string or number fails the step, so later steps never send a missing value, as does a response over 4,000,000 characters: a request step stops reading and aborts its fetch there. A UI step reads a body only when @@ -592,15 +595,17 @@ id the application assigns: partial failures without retaining an image to scrub later. Other requested artifacts remain available; affected screenshot/aggregate completeness is partial and is not eligible for equality. -- Saved URLs and expanded request JSON are admitted before building their expanded strings or - serializing. Its allowance is the smaller of Node's string limit and - one sixteenth of current V8 available heap bytes, allowing for the expanded - tree, serialization, Playwright transport and browser-side copies. Size +- Saved URLs, structured queries, and expanded request JSON are admitted before + building their expanded strings or serializing. Their allowance is the smaller + of Node's string limit and one sixteenth of current V8 available heap bytes, + allowing for the expanded tree, serialization, Playwright transport and + browser-side copies. Size accounting includes JSON escaping and repeated references; each distinct saved value's size is measured once per request. An oversized request fails before browser dispatch; the complete request envelope, including repeated - header values, is checked as well. URL encoding uses a conservative - nine-characters-per-UTF-16-unit allowance before encoding a saved value. + header values, is checked as well. URI-component and form-query sizes are + counted with their respective escaping rules before encoding; repeated equal + components reuse their measured size. Reduce the payload/repetitions, or raise the parent Node heap and restart; this is a conservative allocation estimate, not a measurement or guarantee of browser/process-family memory. diff --git a/src/browser/BrowserScenarioSecrets.ts b/src/browser/BrowserScenarioSecrets.ts index 49122c0c00..33a53eebb4 100644 --- a/src/browser/BrowserScenarioSecrets.ts +++ b/src/browser/BrowserScenarioSecrets.ts @@ -1,3 +1,7 @@ +import { + scenarioSerializationAllowance, + scenarioQueryLength, +} from "./BrowserScenarioSerialization.js"; import type { AnalysisError } from "../domain/analysisErrorBase.js"; import { redactSensitiveFailure } from "../domain/explicitSensitiveFailure.js"; import { AnalysisInputError } from "../domain/analysisErrorCore.js"; @@ -666,8 +670,28 @@ export class BrowserScenarioSecrets { url(destination: BrowserScenarioUrl): string { const url = new URL(destination.url); - for (const { name, value } of destination.query) - url.searchParams.append(name, this.value(value)); + if (destination.query.length > 0) { + const query: [string, string][] = destination.query.map( + ({ name, value }) => [name, this.value(value)], + ); + const entries = [...url.searchParams, ...query]; + // Appending form entries reserializes the literal query too; fragments remain unchanged. + const queryStart = url.href.indexOf("?"); + const hashStart = url.href.indexOf("#"); + const existingQueryLength = + queryStart >= 0 && (hashStart < 0 || queryStart < hashStart) + ? (hashStart < 0 ? url.href.length : hashStart) - queryStart + : 0; + const budget = + scenarioSerializationAllowance() - + (url.href.length - existingQueryLength) - + 1; + if (scenarioQueryLength(entries, budget, "form") > budget) + throw new Error( + "Expanded scenario URL exceeds the current heap/string serialization allowance; reduce repeated values or raise the parent Node heap and restart.", + ); + for (const [name, value] of query) url.searchParams.append(name, value); + } return url.href; } diff --git a/src/browser/BrowserScenarioSerialization.ts b/src/browser/BrowserScenarioSerialization.ts new file mode 100644 index 0000000000..1655c96caa --- /dev/null +++ b/src/browser/BrowserScenarioSerialization.ts @@ -0,0 +1,65 @@ +import { constants as bufferConstants } from "node:buffer"; +import { getHeapStatistics } from "node:v8"; + +/** Allow for expanded values, stringify, Playwright transport and browser-side copies. */ +const SCENARIO_ALLOCATION_FACTOR = 16; +export const scenarioSerializationAllowance = (): number => + Math.min( + bufferConstants.MAX_STRING_LENGTH, + Math.floor( + getHeapStatistics().total_available_size / SCENARIO_ALLOCATION_FACTOR, + ), + ); + +type QueryEncoding = "uri-component" | "form"; +const URI_KEPT = /^[\w.!~*'()-]$/u; +const FORM_KEPT = /^[\w *.-]$/u; + +/** Count the selected encoder's ASCII output without allocating an encoded copy. */ +const componentLength = ( + text: string, + limit: number, + encoding: QueryEncoding, +): number => { + let length = 0; + for (const character of text) { + const point = character.codePointAt(0)!; + if (point >= 0xd800 && point <= 0xdfff && encoding === "uri-component") + throw new URIError("URI malformed"); + // URLSearchParams replaces an unpaired UTF-16 unit with U+FFFD (three UTF-8 bytes). + length += (encoding === "form" ? FORM_KEPT : URI_KEPT).test(character) + ? 1 + : 3 * (point < 0x80 ? 1 : point < 0x800 ? 2 : point < 0x10000 ? 3 : 4); + if (length > limit) return length; + } + return length; +}; + +export const uriComponentLength = (text: string, limit: number): number => + componentLength(text, limit, "uri-component"); + +/** Query separators and encoded entries; a result above the budget establishes overflow only. */ +export const scenarioQueryLength = ( + entries: readonly (readonly [string, string])[], + budget: number, + encoding: QueryEncoding, +): number => { + let rawLength = Math.max(0, entries.length * 2 - 1); + for (const [name, value] of entries) { + rawLength += name.length + value.length; + if (rawLength > budget) return rawLength; + } + let length = Math.max(0, entries.length * 2 - 1); + const sizes = new Map(); + for (const entry of entries) + for (const text of entry) { + let size = sizes.get(text); + if (size === undefined) { + size = componentLength(text, budget - length, encoding); + sizes.set(text, size); + } + length += size; + if (length > budget) return length; + } + return length; +}; diff --git a/src/browser/PlaywrightScenarioRequest.test.ts b/src/browser/PlaywrightScenarioRequest.test.ts index 76194c53a2..962b7b840a 100644 --- a/src/browser/PlaywrightScenarioRequest.test.ts +++ b/src/browser/PlaywrightScenarioRequest.test.ts @@ -390,6 +390,42 @@ describe("request route cleanup", () => { expect(save).not.toHaveBeenCalled(); expect(ready).not.toHaveBeenCalled(); }); + + it.each([ + { status: 200, end: "cancellation" }, + { status: 503, end: "cancellation" }, + { status: 200, end: "deadline" }, + { status: 503, end: "deadline" }, + ])( + "publishes nothing after $end during final route cleanup (status $status)", + async ({ status, end }) => { + stubFetch(status, ['{"id":"late-object"}']); + const parsed = scenario([ + requestStep({ + expected_status: [200], + timeout_ms: 5000, + save: [{ name: "id", json_pointer: "/id" }], + }), + ]); + const page = fakePage({}); + const secrets = resolved(parsed); + const cancel = new AbortController(); + const began = Date.now(); + const unroute = vi.spyOn(page, "unroute").mockImplementation(async () => { + if (end === "cancellation") + cancel.abort("cancelled during final route cleanup"); + else { + const now = vi.spyOn(Date, "now").mockReturnValue(began + 5001); + onTestFinished(() => now.mockRestore()); + } + }); + await expect( + perform(page, parsed.actions[0]!, secrets, cancel.signal), + ).rejects.toThrow(status === 503 ? "answered 503" : "ended before"); + expect(() => secrets.savedValue("id")).toThrow("No earlier step saved"); + expect(unroute).toHaveBeenCalledOnce(); + }, + ); }); describe("request page boundaries", () => { @@ -513,12 +549,14 @@ describe("request step confinement", () => { const parsed = scenario([ requestStep({ path: "/api/items?flag&a=b%20c&s=x;y", - query: [{ name: "q", value: { source: "literal", value: "a b" } }], + query: [ + { name: "q", value: { source: "literal", value: "a bē§šŸ˜€!~'" } }, + ], }), ]); await perform(fakePage({}), parsed.actions[0]!, resolved(parsed)); expect(calls[0]!.url).toBe( - `${ORIGIN}/api/items?flag&a=b%20c&s=x;y&q=a%20b`, + `${ORIGIN}/api/items?flag&a=b%20c&s=x;y&q=a%20b%E7%A7%81%F0%9F%98%80!~'`, ); for (const [path, sent] of [ ["/api/items?", "/api/items?q=1"], @@ -1700,7 +1738,7 @@ describe("saving from a UI step's response", () => { await expect( perform(uiPage([]).page, parsed.actions[0]!, resolved(parsed)), ).rejects.toThrow( - "no POST /api/items response arrived before the step's timeout", + "no POST /api/items response could be read, so nothing was saved: Timeout exceeded", ); await expect( perform( @@ -1721,6 +1759,72 @@ describe("saving from a UI step's response", () => { }); }); +it.each([false, true])( + "retains the physical response-wait failure despite late cancellation %s and removes its request watcher", + async (cancelAtCompletion) => { + const parsed = clickSaving("/api/items"); + const secrets = resolved(parsed); + const { page, listeners } = uiPage([]); + const primary = new Error( + "page.waitForResponse: Target page has been closed", + ); + vi.spyOn(page, "waitForResponse").mockRejectedValue(primary); + const cancel = new AbortController(); + if (cancelAtCompletion) { + const remove = cancel.signal.removeEventListener.bind(cancel.signal); + const removing = vi + .spyOn(cancel.signal, "removeEventListener") + .mockImplementation((type, listener, options) => { + remove(type, listener, options); + cancel.abort("cancelled after the physical wait failed"); + }); + onTestFinished(() => removing.mockRestore()); + } + const failure = await perform( + page, + parsed.actions[0]!, + secrets, + cancel.signal, + ).catch((cause: unknown) => cause); + expect(failure).toMatchObject({ cause: primary }); + expect((failure as Error).message).toContain(primary.message); + expect(() => secrets.savedValue("item")).toThrow("No earlier step saved"); + expect(listeners.size).toBe(0); + }, +); + +it.each(["request", "click"])( + "retains saved values when cancellation follows completed %s publication", + async (kind) => { + onTestFinished(() => { + vi.unstubAllGlobals(); + }); + const parsed = + kind === "request" + ? scenario([ + requestStep({ save: [{ name: "item", json_pointer: "/id" }] }), + ]) + : clickSaving("/api/items"); + stubFetch(200, ['{"id":"committed-object"}']); + const page = + kind === "request" + ? fakePage({}) + : uiPage([ + { + method: "POST", + url: `${ORIGIN}/api/items`, + status: 201, + body: '{"id":"committed-object"}', + }, + ]).page; + const secrets = resolved(parsed); + const cancel = new AbortController(); + await perform(page, parsed.actions[0]!, secrets, cancel.signal); + cancel.abort("cancelled after publication"); + expect(secrets.savedValue("item")).toBe("committed-object"); + }, +); + describe("saving from a compressed or cached UI response", () => { it.each([ { encoding: "gzip", size: 32 }, @@ -1842,6 +1946,35 @@ describe("saved response limits and deadlines", () => { expect(listeners.size).toBe(0); }); + it("saves nothing when cancellation arrives as the body wait releases its listener", async () => { + const parsed = clickSaving("/api/items"); + const secrets = resolved(parsed); + const { page, listeners } = uiPage([ + { + method: "POST", + url: `${ORIGIN}/api/items`, + status: 201, + body: '{"id":"late-object"}', + }, + ]); + const cancel = new AbortController(); + const remove = cancel.signal.removeEventListener.bind(cancel.signal); + let waitsReleased = 0; + const removeListener = vi + .spyOn(cancel.signal, "removeEventListener") + .mockImplementation((type, listener, options) => { + remove(type, listener, options); + if (++waitsReleased === 2) + cancel.abort("cancelled at body wait completion"); + }); + onTestFinished(() => removeListener.mockRestore()); + await expect( + perform(page, parsed.actions[0]!, secrets, cancel.signal), + ).rejects.toThrow("ended before"); + expect(() => secrets.savedValue("item")).toThrow("No earlier step saved"); + expect(listeners.size).toBe(0); + }); + it("reads a response whose received bytes exceed the character limit but whose text is within it", async () => { const parsed = clickSaving("/api/items"); const secrets = resolved(parsed); @@ -1911,6 +2044,92 @@ it.each(["array", "embedded", "headers"])( }, ); +it.each(["x".repeat(3_000_000), "ē§".repeat(100_000)])( + "refuses repeated query expansion before encoding or browser dispatch (case %#)", + async (value) => { + onTestFinished(() => { + vi.unstubAllGlobals(); + }); + const parsed = scenario([ + requestStep({ + query: Array.from({ length: 1000 }, (_, index) => ({ + name: `repeated-${index}`, + value: { source: "literal", value }, + })), + }), + ]); + const secrets = resolved(parsed); + const page = { ...fakePage({}), evaluate: vi.fn() } as unknown as Page; + const encode = vi + .spyOn(globalThis, "encodeURIComponent") + .mockImplementation(() => { + throw new Error("Query encoding ran before expansion admission"); + }); + onTestFinished(() => encode.mockRestore()); + await expect(perform(page, parsed.actions[0]!, secrets)).rejects.toThrow( + "serialization allowance", + ); + expect(encode).not.toHaveBeenCalled(); + expect(page.evaluate).not.toHaveBeenCalled(); + }, +); + +it("refuses repeated goto query expansion before form encoding or browser navigation", async () => { + const value = "x".repeat(3_000_000); + const parsed = scenario([ + { + ...gotoStep(`${ORIGIN}/items?literal=a%20b#kept`), + destination: { + url: `${ORIGIN}/items?literal=a%20b#kept`, + query: Array.from({ length: 1000 }, (_, index) => ({ + name: `repeated-${index}`, + value: { source: "literal", value }, + })), + }, + }, + ]); + const secrets = resolved(parsed); + const goto = vi.fn(async () => null); + const append = vi + .spyOn(URLSearchParams.prototype, "append") + .mockImplementation(() => { + throw new Error("Form encoding ran before expansion admission"); + }); + onTestFinished(() => append.mockRestore()); + await expect( + perform({ goto } as unknown as Page, parsed.actions[0]!, secrets), + ).rejects.toThrow("serialization allowance"); + expect(append).not.toHaveBeenCalled(); + expect(goto).not.toHaveBeenCalled(); +}); + +it("preserves goto form query normalization, Unicode, replacement characters and the fragment", async () => { + const parsed = scenario([ + { + ...gotoStep(`${ORIGIN}/items?literal=a%20b#kept`), + destination: { + url: `${ORIGIN}/items?literal=a%20b#kept`, + query: [ + { + name: "q", + value: { source: "literal", value: "a bē§šŸ˜€!~'\ud800" }, + }, + ], + }, + }, + ]); + const goto = vi.fn(async () => null); + await perform( + { goto } as unknown as Page, + parsed.actions[0]!, + resolved(parsed), + ); + expect(goto).toHaveBeenCalledWith( + `${ORIGIN}/items?literal=a+b&q=a+b%E7%A7%81%F0%9F%98%80%21%7E%27%EF%BF%BD#kept`, + expect.anything(), + ); +}); + it("refuses repeated saved URL expansion before browser navigation", async () => { const parsed = scenario([ requestStep({ save: [{ name: "large", json_pointer: "/value" }] }), diff --git a/src/browser/PlaywrightScenarioRequest.ts b/src/browser/PlaywrightScenarioRequest.ts index 073bdf7f20..6a2c909e31 100644 --- a/src/browser/PlaywrightScenarioRequest.ts +++ b/src/browser/PlaywrightScenarioRequest.ts @@ -1,5 +1,3 @@ -import { constants as bufferConstants } from "node:buffer"; -import { getHeapStatistics } from "node:v8"; import { jsonStringByteLength } from "../domain/jsonStringByteLength.js"; import { jsonValueByteLength } from "../domain/jsonValueByteLength.js"; import type { JsonValue } from "../domain/jsonValue.js"; @@ -24,6 +22,11 @@ import { type BrowserScenarioSaveResponse, } from "../domain/browserScenarioValues.js"; import type { BrowserScenarioSecrets } from "./BrowserScenarioSecrets.js"; +import { + scenarioSerializationAllowance, + scenarioQueryLength, + uriComponentLength, +} from "./BrowserScenarioSerialization.js"; type RequestAction = Extract; @@ -160,18 +163,12 @@ const headerValue = async ( return readPageValue(step, value, label); }; -/** Append declared query entries after any literal query the path carries, leaving that query as written. */ +/** Append admitted query entries after any literal query the path carries, leaving that query as written. */ const withQuery = ( url: URL, entries: readonly (readonly [string, string])[], ): string => { if (entries.length === 0) return url.href; - const added = entries - .map( - ([name, value]) => - `${encodeURIComponent(name)}=${encodeURIComponent(value)}`, - ) - .join("&"); // fetch drops the fragment; cut it at its "#" even when nothing follows it. const hash = url.href.indexOf("#"); const base = hash < 0 ? url.href : url.href.slice(0, hash); @@ -180,6 +177,16 @@ const withQuery = ( : base.endsWith("?") || base.endsWith("&") ? "" : "&"; + const budget = + scenarioSerializationAllowance() - base.length - separator.length; + if (scenarioQueryLength(entries, budget, "uri-component") > budget) + throw requestEnvelopeError(); + const added = entries + .map( + ([name, value]) => + `${encodeURIComponent(name)}=${encodeURIComponent(value)}`, + ) + .join("&"); return `${base}${separator}${added}`; }; @@ -315,7 +322,7 @@ export const performScenarioRequest = async (input: { ); const step = { page, url, stepId: action.step_id, secrets }; const headers: [string, string][] = []; - const envelopeLimit = requestSerializationAllowance(); + const envelopeLimit = scenarioSerializationAllowance(); let headerBytes = 2; const headerValues = new Map(); const appendHeader = (name: string, value: string): void => { @@ -466,6 +473,7 @@ export const performScenarioRequest = async (input: { } } if (action.save.length > 0) { + assertResponseSaveActive(action.step_id, deadline, signal); saveValues(action.step_id, response.body, action.save, secrets); if (observedRequest !== undefined && !ambiguousRequest) secrets.markResponseSecretsReady(observedRequest); @@ -481,7 +489,7 @@ export const withSavedPlaceholders = ( secrets: BrowserScenarioSecrets, placeholder: RegExp = SAVED_PLACEHOLDER, ): string => { - const limit = requestSerializationAllowance(); + const limit = scenarioSerializationAllowance(); let expandedUnits = text.length; if (expandedUnits > limit) throw new ScenarioRequestError( @@ -493,8 +501,7 @@ export const withSavedPlaceholders = ( let replacement = encoded.get(name); if (replacement === undefined) { const raw = String(secrets.savedValue(name)); - // A UTF-16 unit needs at most nine ASCII characters under URI encoding. - if (raw.length * 9 > limit) + if (uriComponentLength(raw, limit) > limit) throw new ScenarioRequestError( "Saved URL encoding exceeds the current heap/string serialization allowance.", ); @@ -553,6 +560,21 @@ const namesResponse = ( ); }; +/** Publication must still belong to the step after its last asynchronous boundary. */ +const assertResponseSaveActive = ( + stepId: string, + deadline: number | undefined, + signal: AbortSignal | undefined, +): void => { + if ( + signal?.aborted === true || + (deadline !== undefined && Date.now() >= deadline) + ) + throw new ScenarioRequestError( + `Step ${stepId} ended before its saved response was read, so nothing was saved.`, + ); +}; + /** Settle as the work does, or fail the step once its deadline passes or it is cancelled, so late work saves nothing. */ const beforeStepEnds = ( work: Promise, @@ -647,9 +669,11 @@ export const awaitSavedResponse = ( let response: Awaited; try { response = await beforeStepEnds(waiting, stepId, deadline, signal); - } catch { + } catch (cause: unknown) { + if (cause instanceof ScenarioRequestError) throw cause; throw new ScenarioRequestError( - `Step ${stepId}: no ${spec.method} ${spec.path} response arrived before the step's timeout, so nothing was saved.`, + `Step ${stepId}: no ${spec.method} ${spec.path} response could be read, so nothing was saved: ${cause instanceof Error ? cause.message : String(cause)}`, + { cause }, ); } const assertUnambiguous = (): void => { @@ -672,6 +696,7 @@ export const awaitSavedResponse = ( if (!body.ok) throw new ScenarioRequestError(`Step ${stepId}: ${body.error.message}`); assertUnambiguous(); + assertResponseSaveActive(stepId, deadline, signal); saveValues(stepId, body.value, spec.save, secrets); secrets.markResponseSecretsReady(response.request()); }; @@ -683,22 +708,12 @@ const requestEnvelopeError = (): ScenarioRequestError => "Expanded request envelope exceeds the current heap/string serialization allowance; reduce repeated values or raise the parent Node heap and restart.", ); -/** Allow for the expanded tree, stringify, Playwright transport and browser-side copies. */ -const REQUEST_JSON_ALLOCATION_FACTOR = 16; -const requestSerializationAllowance = (): number => - Math.min( - bufferConstants.MAX_STRING_LENGTH, - Math.floor( - getHeapStatistics().total_available_size / REQUEST_JSON_ALLOCATION_FACTOR, - ), - ); - /** Count serialization expansion without allocating substituted strings or their serialized copies. */ const admitSavedJson = ( value: JsonValue, secrets: BrowserScenarioSecrets, ): void => { - const limit = requestSerializationAllowance(); + const limit = scenarioSerializationAllowance(); let bytes = 0; const savedSizes = new Map(); const charge = (size: number): void => {