From 0740157028940720cc684716910e91eeb75e34e3 Mon Sep 17 00:00:00 2001 From: Mohammad Wahbeh Date: Tue, 11 Aug 2026 10:47:02 +0300 Subject: [PATCH 1/3] feat(core): add request/response body lifecycle: Body model, materialize(), TypedResponse, HttpStatusError, logging tees (BODY-1..37, HTTP-36..52). --- .changeset/body-lifecycle.md | 7 + packages/core/etc/core.api.md | 214 ++++++++++++++++- packages/core/src/body/body.ts | 16 ++ packages/core/src/body/errors.test.ts | 31 +++ packages/core/src/body/errors.ts | 58 +++++ .../core/src/body/http-status-error.test.ts | 89 ++++++++ packages/core/src/body/http-status-error.ts | 103 +++++++++ packages/core/src/body/index.ts | 35 +++ packages/core/src/body/materialize.test.ts | 78 +++++++ packages/core/src/body/materialize.ts | 39 ++++ packages/core/src/body/multipart-body.test.ts | 216 ++++++++++++++++++ packages/core/src/body/multipart-body.ts | 191 ++++++++++++++++ .../src/body/request-body-logging.test.ts | 153 +++++++++++++ .../core/src/body/request-body-logging.ts | 77 +++++++ .../src/body/response-body-logging.test.ts | 157 +++++++++++++ .../core/src/body/response-body-logging.ts | 191 ++++++++++++++++ packages/core/src/body/simple-bodies.test.ts | 97 ++++++++ packages/core/src/body/simple-bodies.ts | 184 +++++++++++++++ packages/core/src/body/stream-body.test.ts | 118 ++++++++++ packages/core/src/body/stream-body.ts | 84 +++++++ packages/core/src/body/typed-response.test.ts | 80 +++++++ packages/core/src/body/typed-response.ts | 56 +++++ packages/core/src/http/request.test.ts | 22 +- packages/core/src/http/request.ts | 30 +-- packages/core/src/http/response.test.ts | 133 ++++++++--- packages/core/src/http/response.ts | 185 +++++++-------- packages/core/src/index.ts | 33 +++ packages/core/src/io/errors.test.ts | 36 ++- packages/core/src/io/errors.ts | 33 ++- packages/core/src/io/index.ts | 1 + packages/core/src/seams/operation.test.ts | 7 +- packages/core/src/seams/operation.ts | 3 +- 32 files changed, 2569 insertions(+), 188 deletions(-) create mode 100644 .changeset/body-lifecycle.md create mode 100644 packages/core/src/body/body.ts create mode 100644 packages/core/src/body/errors.test.ts create mode 100644 packages/core/src/body/errors.ts create mode 100644 packages/core/src/body/http-status-error.test.ts create mode 100644 packages/core/src/body/http-status-error.ts create mode 100644 packages/core/src/body/index.ts create mode 100644 packages/core/src/body/materialize.test.ts create mode 100644 packages/core/src/body/materialize.ts create mode 100644 packages/core/src/body/multipart-body.test.ts create mode 100644 packages/core/src/body/multipart-body.ts create mode 100644 packages/core/src/body/request-body-logging.test.ts create mode 100644 packages/core/src/body/request-body-logging.ts create mode 100644 packages/core/src/body/response-body-logging.test.ts create mode 100644 packages/core/src/body/response-body-logging.ts create mode 100644 packages/core/src/body/simple-bodies.test.ts create mode 100644 packages/core/src/body/simple-bodies.ts create mode 100644 packages/core/src/body/stream-body.test.ts create mode 100644 packages/core/src/body/stream-body.ts create mode 100644 packages/core/src/body/typed-response.test.ts create mode 100644 packages/core/src/body/typed-response.ts diff --git a/.changeset/body-lifecycle.md b/.changeset/body-lifecycle.md new file mode 100644 index 0000000..6c296c6 --- /dev/null +++ b/.changeset/body-lifecycle.md @@ -0,0 +1,7 @@ +--- +"@dexpace/core": minor +--- + +Add the core Body domain interface and implementations (ByteArrayBody, StringBody, FormUrlEncodedBody, StreamBody, MultipartBody, materialize, TypedResponse, HttpStatusError, toHttpError, withRequestLogging, withResponseLogging). + +`RequestBuilder.body` and `ResponseBuilder.body` narrow from `unknown` to `Body | undefined` and `ReadableStream | null` respectively — a breaking parameter-type change per `styleguide/typescript/10-api-design.md`. Resolving Phase 3b's open D1 finding (`docs/superpowers/specs/2026-07-23-nodejs-sdk-v1-roadmap-design.md`, "Open Findings — Phase 3b Validation Review"): kept as **minor** rather than major because `@dexpace/core` is still pre-1.0 (`0.0.0`), where a 0.x breaking change is conventionally released as minor (semver's own carve-out for initial development, https://semver.org/#spec-item-4). Revisit at 1.0. diff --git a/packages/core/etc/core.api.md b/packages/core/etc/core.api.md index 330de32..92c9da4 100644 --- a/packages/core/etc/core.api.md +++ b/packages/core/etc/core.api.md @@ -4,6 +4,21 @@ ```ts +// @public +interface Body_2 { + // (undocumented) + readonly contentLength: number; + // (undocumented) + readonly kind: 'byte-array' | 'string' | 'stream' | 'form-urlencoded' | 'multipart'; + // (undocumented) + readonly mediaType: string | undefined; + // (undocumented) + readonly replayable: boolean; + // (undocumented) + writeTo(sink: WritableStream): Promise; +} +export { Body_2 as Body } + // @public export interface Builder { build(): T; @@ -12,6 +27,24 @@ export interface Builder { // @public export function buildRequest(baseUrl: string | URL, operation: OperationDescriptor): Request_2; +// @public +export class ByteArrayBody implements Body_2 { + constructor(bytes: Uint8Array, mediaType?: string); + // (undocumented) + readonly contentLength: number; + // (undocumented) + readonly kind: "byte-array"; + // (undocumented) + readonly mediaType: string | undefined; + // (undocumented) + readonly replayable = true; + // (undocumented) + writeTo(sink: WritableStream): Promise; +} + +// @public +export function byteArrayBody(bytes: Uint8Array, mediaType?: string): ByteArrayBody; + // @public export class CancellationError extends DexpaceError { constructor(message: string, options?: ErrorOptions); @@ -20,6 +53,13 @@ export class CancellationError extends DexpaceError { // @public export function composeSignal(userSignal?: AbortSignal, timeoutMs?: number): AbortSignal | undefined; +// @public +export class ConsumedBodyError extends DexpaceError { + constructor(bodyKind: string, options?: ErrorOptions); + // (undocumented) + readonly bodyKind: string; +} + // @public export class DexpaceError extends Error { constructor(message: string, options?: ErrorOptions); @@ -43,6 +83,29 @@ export class ETag { export class EtagParseError extends DomainModelError { } +// @public +export class FormUrlEncodedBody implements Body_2 { + constructor(input: FormUrlEncodedInput); + // (undocumented) + readonly contentLength: number; + // (undocumented) + readonly kind: "form-urlencoded"; + // (undocumented) + readonly mediaType = "application/x-www-form-urlencoded"; + // (undocumented) + readonly params: QueryParams; + // (undocumented) + readonly replayable = true; + // (undocumented) + writeTo(sink: WritableStream): Promise; +} + +// @public +export function formUrlEncodedBody(input: FormUrlEncodedInput): FormUrlEncodedBody; + +// @public +export type FormUrlEncodedInput = QueryParams | ReadonlyMap | Record | readonly (readonly [string, string])[]; + // @public export class HeaderName { equals(other: HeaderName): boolean; @@ -97,9 +160,24 @@ export class HttpRange { export class HttpRangeValidationError extends DomainModelError { } +// @public +export class HttpStatusError extends DexpaceError { + constructor(status: number, bodyBytes: Uint8Array | undefined, mediaType: string | undefined, options?: ErrorOptions); + body(): Body_2 | undefined; + preview(charset?: string): string | null; + // (undocumented) + readonly status: number; +} + +// @public +export function isBodyError(error: unknown): error is ConsumedBodyError | MultipartBoundaryError; + // @public export function isTimeoutSignal(signal: AbortSignal): boolean; +// @public +export function materialize(body: Body_2): Promise; + // @public export class MediaType { get charset(): string | undefined; @@ -120,6 +198,56 @@ export class MediaTypeParseError extends DomainModelError { // @public export type Method = 'GET' | 'HEAD' | 'POST' | 'PUT' | 'DELETE' | 'CONNECT' | 'OPTIONS' | 'TRACE' | 'PATCH'; +// @public +export class MultipartBody implements Body_2 { + constructor(parts: readonly MultipartPart[], boundary?: string); + // (undocumented) + readonly contentLength: number; + // (undocumented) + readonly kind: "multipart"; + // (undocumented) + readonly mediaType: string; + // (undocumented) + static newBuilder(): MultipartBodyBuilder; + newBuilder(): MultipartBodyBuilder; + // (undocumented) + readonly replayable: boolean; + // (undocumented) + writeTo(sink: WritableStream): Promise; +} + +// @public +export function multipartBody(parts: readonly MultipartPart[], boundary?: string): MultipartBody; + +// @public +export class MultipartBodyBuilder implements Builder { + // (undocumented) + addPart(part: MultipartPart): this; + // (undocumented) + boundary(boundary: string | undefined): this; + // (undocumented) + build(): MultipartBody; + // (undocumented) + parts(parts: readonly MultipartPart[]): this; +} + +// @public +export class MultipartBoundaryError extends DexpaceError { + constructor(boundary: string, options?: ErrorOptions); + // (undocumented) + readonly boundary: string; +} + +// @public +export interface MultipartPart { + // (undocumented) + readonly body: Body_2; + // (undocumented) + readonly filename?: string | undefined; + // (undocumented) + readonly name: string; +} + // @public export class OperationAssemblyError extends DexpaceError { constructor(message: string, parameterName: string); @@ -128,7 +256,7 @@ export class OperationAssemblyError extends DexpaceError { // @public export interface OperationDescriptor { - readonly body?: unknown; + readonly body?: Body_2 | undefined; readonly headers?: Headers_2 | undefined; readonly method: Method; readonly pathParams?: Readonly> | undefined; @@ -172,7 +300,7 @@ export type RangeKind = 'bounded' | 'suffix' | 'open'; // @public class Request_2 { - get body(): unknown; + get body(): Body_2 | undefined; equals(other: Request_2): boolean; get headers(): Headers_2; get method(): Method; @@ -189,7 +317,7 @@ export class RequestBodyNotAllowedError extends DomainModelError { // @public export class RequestBuilder implements Builder { - body(body: unknown): this; + body(body: Body_2 | undefined): this; build(): Request_2; headers(headers: Headers_2): this; method(method: Method): this; @@ -246,25 +374,45 @@ export class RequiredFieldError extends DomainModelError { // @public class Response_2 { - get body(): unknown; + // (undocumented) + [Symbol.asyncDispose](): Promise; + constructor(request: Request_2, protocol: Protocol, status: Status, reasonPhrase: string | undefined, headers: Headers_2, body: ReadableStream | null); + get body(): ReadableStream | null; + bytes(): Promise; + close(): Promise; + // (undocumented) get headers(): Headers_2; + // (undocumented) static newBuilder(): ResponseBuilder; + // (undocumented) newBuilder(): ResponseBuilder; + // (undocumented) get protocol(): Protocol; + // (undocumented) get reasonPhrase(): string | undefined; + // (undocumented) get request(): Request_2; + // (undocumented) get status(): Status; + text(): Promise; } export { Response_2 as Response } // @public export class ResponseBuilder implements Builder { - body(body: unknown): this; + // (undocumented) + body(body: ReadableStream | null): this; + // (undocumented) build(): Response_2; + // (undocumented) headers(headers: Headers_2): this; + // (undocumented) protocol(protocol: Protocol): this; + // (undocumented) reasonPhrase(reasonPhrase: string | undefined): this; + // (undocumented) request(request: Request_2): this; + // (undocumented) status(status: Status): this; } @@ -284,12 +432,68 @@ export class Status { static recognized(code: number): Status | undefined; } +// @public +export class StreamBody implements Body_2 { + constructor(stream: ReadableStream, mediaType?: string, contentLength?: number); + // (undocumented) + readonly contentLength: number; + // (undocumented) + readonly kind: "stream"; + // (undocumented) + readonly mediaType: string | undefined; + // (undocumented) + readonly replayable = false; + // (undocumented) + writeTo(sink: WritableStream): Promise; +} + +// @public +export function streamBody(stream: ReadableStream, mediaType?: string, contentLength?: number): StreamBody; + +// @public +export class StringBody implements Body_2 { + constructor(text: string, mediaType?: string); + // (undocumented) + readonly contentLength: number; + // (undocumented) + readonly kind: "string"; + // (undocumented) + readonly mediaType: string; + // (undocumented) + readonly replayable = true; + // (undocumented) + readonly text: string; + // (undocumented) + writeTo(sink: WritableStream): Promise; +} + +// @public +export function stringBody(text: string, mediaType?: string): StringBody; + +// @public +export function toHttpError(response: Response_2): Promise; + // @public export interface Transport { close(): Promise; send(request: Request_2, options?: RequestOptions, signal?: AbortSignal): Promise; } +// @public +export class TypedResponse { + constructor(response: Response_2, parse: (response: Response_2) => Promise); + // (undocumented) + get headers(): Response_2['headers']; + // (undocumented) + get protocol(): string; + // (undocumented) + get reason(): string | undefined; + get request(): Request_2; + // (undocumented) + get status(): Response_2['status']; + value(): Promise; +} + // @public export class UrlConstructionError extends DomainModelError { } diff --git a/packages/core/src/body/body.ts b/packages/core/src/body/body.ts new file mode 100644 index 0000000..f701ab4 --- /dev/null +++ b/packages/core/src/body/body.ts @@ -0,0 +1,16 @@ +// SPDX-License-Identifier: MIT +// packages/core/src/body/body.ts + +/** + * The core domain interface for HTTP message bodies. + * + * @public + */ +export interface Body { + readonly kind: + 'byte-array' | 'string' | 'stream' | 'form-urlencoded' | 'multipart'; + readonly mediaType: string | undefined; + readonly contentLength: number; + readonly replayable: boolean; + writeTo(sink: WritableStream): Promise; +} diff --git a/packages/core/src/body/errors.test.ts b/packages/core/src/body/errors.test.ts new file mode 100644 index 0000000..d55a2b4 --- /dev/null +++ b/packages/core/src/body/errors.test.ts @@ -0,0 +1,31 @@ +// SPDX-License-Identifier: MIT +// packages/core/src/body/errors.test.ts +// Exercises: BODY-3 (ConsumedBodyError), HTTP-51 (MultipartBoundaryError) +import {describe, expect, test} from 'bun:test'; +import {DexpaceError} from '../http/errors.js'; +import { + ConsumedBodyError, + isBodyError, + MultipartBoundaryError, +} from './errors.js'; + +describe('body errors', () => { + test('ConsumedBodyError descends from DexpaceError and names the body kind', () => { + const error = new ConsumedBodyError('stream'); + expect(error).toBeInstanceOf(DexpaceError); + expect(error.bodyKind).toBe('stream'); + expect(error.message).toContain('stream'); + }); + + test('MultipartBoundaryError descends from DexpaceError and names the offending boundary', () => { + const error = new MultipartBoundaryError('bad boundary'); + expect(error).toBeInstanceOf(DexpaceError); + expect(error.boundary).toBe('bad boundary'); + }); + + test('isBodyError groups both leaves without a class tier', () => { + expect(isBodyError(new ConsumedBodyError('stream'))).toBe(true); + expect(isBodyError(new MultipartBoundaryError('x'))).toBe(true); + expect(isBodyError(new DexpaceError('other'))).toBe(false); + }); +}); diff --git a/packages/core/src/body/errors.ts b/packages/core/src/body/errors.ts new file mode 100644 index 0000000..ce36083 --- /dev/null +++ b/packages/core/src/body/errors.ts @@ -0,0 +1,58 @@ +// SPDX-License-Identifier: MIT +// packages/core/src/body/errors.ts +import {DexpaceError} from '../http/errors.js'; + +/** + * A single-use body's second write (BODY-3). `bodyKind` names which Body variant refused the write. + * + * @example + * ```ts + * try { + * await body.writeTo(sink); + * } catch (error) { + * if (error instanceof ConsumedBodyError) { + * // materialize() first if you need to send this body more than once + * } + * } + * ``` + * @public + */ +export class ConsumedBodyError extends DexpaceError { + readonly bodyKind: string; + + constructor(bodyKind: string, options?: ErrorOptions) { + super( + `${bodyKind} body already consumed -- single-use bodies cannot be written twice`, + options, + ); + this.bodyKind = bodyKind; + } +} + +/** + * A caller-supplied multipart boundary violates RFC 2046's grammar (HTTP-51). + * + * @public + */ +export class MultipartBoundaryError extends DexpaceError { + readonly boundary: string; + + constructor(boundary: string, options?: ErrorOptions) { + super(`invalid multipart boundary: ${JSON.stringify(boundary)}`, options); + this.boundary = boundary; + } +} + +/** + * Type guard for body errors. + * + * @public + */ +export function isBodyError( + error: unknown, +): error is ConsumedBodyError | MultipartBoundaryError { + return ( + error instanceof ConsumedBodyError || + error instanceof MultipartBoundaryError + ); +} diff --git a/packages/core/src/body/http-status-error.test.ts b/packages/core/src/body/http-status-error.test.ts new file mode 100644 index 0000000..7a06852 --- /dev/null +++ b/packages/core/src/body/http-status-error.test.ts @@ -0,0 +1,89 @@ +// SPDX-License-Identifier: MIT +// packages/core/src/body/http-status-error.test.ts +// Exercises: HTTP-52/BODY-30 (1 MiB cap, replayable re-serve, buffered inside close-guaranteeing scope), +// BODY-31 (4xx/5xx only, no-body response returned unchanged), BODY-33 (non-consuming preview) +import {describe, expect, test} from 'bun:test'; +import {Headers} from '../http/headers.js'; +import {Protocol} from '../http/protocol.js'; +import {Request} from '../http/request.js'; +import {Response} from '../http/response.js'; +import {Status} from '../http/status.js'; +import {toHttpError} from './http-status-error.js'; + +function readableOf(bytes: Uint8Array): ReadableStream { + return new ReadableStream({ + start: c => { + c.enqueue(bytes); + c.close(); + }, + }); +} + +function responseWith( + status: number, + body: ReadableStream | null, + headers: Headers = Headers.newBuilder().build(), +): Response { + return Response.newBuilder() + .request(Request.newBuilder().url('https://example.com').build()) + .protocol(Protocol.HTTP_1_1) + .status(Status.of(status)) + .headers(headers) + .body(body) + .build(); +} + +describe('toHttpError (BODY-31)', () => { + test('returns null for a non-error response', async () => { + expect(await toHttpError(responseWith(200, null))).toBeNull(); + expect(await toHttpError(responseWith(304, null))).toBeNull(); + }); + + test('returns an HttpStatusError for 4xx and 5xx', async () => { + expect(await toHttpError(responseWith(404, null))).not.toBeNull(); + expect(await toHttpError(responseWith(500, null))).not.toBeNull(); + }); +}); + +describe('HttpStatusError (HTTP-52/BODY-30)', () => { + test('carries the status', async () => { + expect((await toHttpError(responseWith(404, null)))?.status).toBe(404); + }); + + test('buffers the body and re-serves it as a replayable, independently readable Body', async () => { + const bytes = new TextEncoder().encode('not found'); + const error = await toHttpError(responseWith(404, readableOf(bytes))); + const body = error?.body(); + expect(body?.replayable).toBe(true); + + const chunks: Uint8Array[] = []; + await body?.writeTo(new WritableStream({write: c => void chunks.push(c)})); + expect(new TextDecoder().decode(chunks[0])).toBe('not found'); + + const chunksAgain: Uint8Array[] = []; + await error + ?.body() + ?.writeTo(new WritableStream({write: c => void chunksAgain.push(c)})); + expect(new TextDecoder().decode(chunksAgain[0])).toBe('not found'); + }); + + test('drops bytes beyond the 1 MiB cap but still drains and closes the connection', async () => { + const big = new Uint8Array(2 * 1024 * 1024).fill(65); + const error = await toHttpError(responseWith(500, readableOf(big))); + expect(error?.body()?.contentLength).toBe(1024 * 1024); + }); + + test('when the response has no body, the error carries an undefined body and null preview (BODY-31)', async () => { + const error = await toHttpError(responseWith(500, null)); + expect(error?.body()).toBeUndefined(); + expect(error?.preview()).toBeNull(); + }); + + test('preview is non-consuming and repeatable (BODY-33)', async () => { + const error = await toHttpError( + responseWith(500, readableOf(new TextEncoder().encode('boom'))), + ); + expect(error?.preview()).toBe('boom'); + expect(error?.preview()).toBe('boom'); + }); +}); diff --git a/packages/core/src/body/http-status-error.ts b/packages/core/src/body/http-status-error.ts new file mode 100644 index 0000000..956297a --- /dev/null +++ b/packages/core/src/body/http-status-error.ts @@ -0,0 +1,103 @@ +// SPDX-License-Identifier: MIT +// packages/core/src/body/http-status-error.ts +import {DexpaceError} from '../http/errors.js'; +import type {Response} from '../http/response.js'; +import {invariant} from '../invariant.js'; +import type {Body} from './body.js'; +import {byteArrayBody} from './simple-bodies.js'; + +// Fixed by HTTP-52. Deliberately NOT BODY-34's shared preview cap, which is configurable and covers the +// two logging tees only -- a spec-fixed value cannot be the configurable one. +const ERROR_BODY_CAP_BYTES = 1024 * 1024; // 1 MiB, HTTP-52/BODY-30 + +/** + * A 4xx/5xx response turned into an exception (HTTP-52/BODY-30, BODY-31). + * + * @public + */ +export class HttpStatusError extends DexpaceError { + readonly status: number; + readonly #bodyBytes: Uint8Array | undefined; + readonly #mediaType: string | undefined; + + // eslint-disable-next-line max-params -- constructor parameters fixed by error model + constructor( + status: number, + bodyBytes: Uint8Array | undefined, + mediaType: string | undefined, + options?: ErrorOptions, + ) { + super(`HTTP ${String(status)}`, options); + this.status = status; + this.#bodyBytes = bodyBytes; + this.#mediaType = mediaType; + } + + /** + * The buffered error body, re-served as a replayable Body -- readable independently and repeatably + * after the transport connection was released (BODY-30). Undefined when there was no body. + */ + body(): Body | undefined { + return this.#bodyBytes === undefined + ? undefined + : byteArrayBody(this.#bodyBytes, this.#mediaType); + } + + /** Non-consuming preview from the buffered copy (BODY-33). Null for no body. */ + preview(charset = 'utf-8'): string | null { + if (this.#bodyBytes === undefined) return null; + return new TextDecoder(charset).decode(this.#bodyBytes); + } +} + +/** + * Turns a 4xx/5xx response into an HttpStatusError, buffering at most 1 MiB of the body inside the + * response's own close-guaranteeing scope (HTTP-52/BODY-30). Returns null for a non-error response + * (BODY-31) -- the caller keeps the response, body intact. + * + * @public + */ +export async function toHttpError( + response: Response, +): Promise { + // BODY-31: error statuses only, i.e. HTTP-11's 400-599 band. A bare `code < 400` would sweep a + // non-standard 6xx -- which HTTP-10 requires Status.of to accept and return -- into the error path + // and consume a body BODY-31 says must be handed back intact. + if (!response.status.isError) return null; + const mediaType = response.headers.get('content-type'); + if (response.body === null) { + await response.close(); + return new HttpStatusError(response.status.code, undefined, mediaType); + } + const reader = response.body.getReader(); + const chunks: Uint8Array[] = []; + let total = 0; + try { + for (;;) { + // Serial by necessity: each read depends on the previous one advancing the cursor. + const {done, value} = await reader.read(); + if (done) break; + if (total >= ERROR_BODY_CAP_BYTES) continue; // keep draining to release the connection; drop the bytes + const room = ERROR_BODY_CAP_BYTES - total; + const piece = value.length > room ? value.subarray(0, room) : value; + chunks.push(piece); + total += piece.length; + } + } finally { + // Release before close(): cancel() rejects with TypeError on a locked stream (see Response.bytes). + reader.releaseLock(); + await response.close(); + } + invariant( + total <= ERROR_BODY_CAP_BYTES, + `buffered ${String(total)} bytes past the ${String(ERROR_BODY_CAP_BYTES)} cap`, + ); + + const bytes = new Uint8Array(total); + let offset = 0; + for (const chunk of chunks) { + bytes.set(chunk, offset); + offset += chunk.length; + } + return new HttpStatusError(response.status.code, bytes, mediaType); +} diff --git a/packages/core/src/body/index.ts b/packages/core/src/body/index.ts new file mode 100644 index 0000000..9a09d83 --- /dev/null +++ b/packages/core/src/body/index.ts @@ -0,0 +1,35 @@ +// SPDX-License-Identifier: MIT +// packages/core/src/body/index.ts +// Internal-facing barrel for product-spec §6. Everything except the two logging tees is also promoted to +// packages/core/src/index.ts (Step 2) -- this file is the superset a future in-tree consumer (e.g. Phase +// 7's pipeline) imports from directly. +export type {Body} from './body.js'; +export { + ConsumedBodyError, + isBodyError, + MultipartBoundaryError, +} from './errors.js'; +export {HttpStatusError, toHttpError} from './http-status-error.js'; +export {materialize} from './materialize.js'; +export { + multipartBody, + MultipartBody, + MultipartBodyBuilder, + type MultipartPart, +} from './multipart-body.js'; +export {withRequestLogging, type LoggedBody} from './request-body-logging.js'; +export { + withResponseLogging, + type LoggedResponseBody, +} from './response-body-logging.js'; +export { + byteArrayBody, + ByteArrayBody, + formUrlEncodedBody, + FormUrlEncodedBody, + type FormUrlEncodedInput, + stringBody, + StringBody, +} from './simple-bodies.js'; +export {streamBody, StreamBody} from './stream-body.js'; +export {TypedResponse} from './typed-response.js'; diff --git a/packages/core/src/body/materialize.test.ts b/packages/core/src/body/materialize.test.ts new file mode 100644 index 0000000..2582d8c --- /dev/null +++ b/packages/core/src/body/materialize.test.ts @@ -0,0 +1,78 @@ +// SPDX-License-Identifier: MIT +// packages/core/src/body/materialize.test.ts +// Exercises: BODY-3/HTTP-37 (materialize-once) +import {describe, expect, test} from 'bun:test'; +import fc from 'fast-check'; +import {ConsumedBodyError} from './errors.js'; +import {byteArrayBody} from './simple-bodies.js'; +import {materialize} from './materialize.js'; +import {streamBody} from './stream-body.js'; + +function readableOf(...bytes: number[]): ReadableStream { + return new ReadableStream({ + start(controller) { + controller.enqueue(Uint8Array.from(bytes)); + controller.close(); + }, + }); +} + +async function drainBody(body: { + writeTo: (sink: WritableStream) => Promise; +}): Promise { + const chunks: Uint8Array[] = []; + await body.writeTo(new WritableStream({write: c => void chunks.push(c)})); + const total = chunks.reduce((s, c) => s + c.length, 0); + const out = new Uint8Array(total); + let offset = 0; + for (const c of chunks) { + out.set(c, offset); + offset += c.length; + } + return out; +} + +describe('materialize', () => { + test('returns an already-replayable body unchanged', async () => { + const body = byteArrayBody(Uint8Array.from([1, 2])); + expect(await materialize(body)).toBe(body); + }); + + test('drains a single-use body into a fresh replayable ByteArrayBody', async () => { + const materialized = await materialize(streamBody(readableOf(1, 2, 3))); + expect(materialized.replayable).toBe(true); + expect(materialized.kind).toBe('byte-array'); + expect([...(await drainBody(materialized))]).toEqual([1, 2, 3]); + }); + + test('the materialized body is writable more than once, byte-for-byte identical', async () => { + const materialized = await materialize(streamBody(readableOf(9, 8))); + expect([...(await drainBody(materialized))]).toEqual([9, 8]); + expect([...(await drainBody(materialized))]).toEqual([9, 8]); + }); + + test('preserves the original mediaType', async () => { + const materialized = await materialize( + streamBody(readableOf(1), 'text/plain'), + ); + expect(materialized.mediaType).toBe('text/plain'); + }); + + test('under N concurrent callers exactly one drains; every other observes ConsumedBodyError (BODY-3)', async () => { + await fc.assert( + fc.asyncProperty(fc.integer({min: 2, max: 8}), async callers => { + const body = streamBody(readableOf(1, 2, 3)); + const results = await Promise.allSettled( + Array.from({length: callers}, () => materialize(body)), + ); + + const fulfilled = results.filter(r => r.status === 'fulfilled'); + expect(fulfilled.length).toBe(1); + for (const result of results.filter(r => r.status === 'rejected')) { + expect(result.reason).toBeInstanceOf(ConsumedBodyError); + } + }), + {seed: 0x3b}, + ); + }); +}); diff --git a/packages/core/src/body/materialize.ts b/packages/core/src/body/materialize.ts new file mode 100644 index 0000000..3cd54ad --- /dev/null +++ b/packages/core/src/body/materialize.ts @@ -0,0 +1,39 @@ +// SPDX-License-Identifier: MIT +// packages/core/src/body/materialize.ts +import {invariant} from '../invariant.js'; +import type {Body} from './body.js'; +import {byteArrayBody} from './simple-bodies.js'; + +/** + * Returns `body` unchanged if already replayable; otherwise drains its single write into a fresh + * replayable ByteArrayBody, after which the original is treated as consumed (BODY-3/HTTP-37). + * + * @public + */ +export async function materialize(body: Body): Promise { + if (body.replayable) return body; + const chunks: Uint8Array[] = []; + let total = 0; + const collector = new WritableStream({ + write: chunk => { + chunks.push(chunk); + total += chunk.length; + }, + }); + await body.writeTo(collector); + + const bytes = new Uint8Array(total); + let offset = 0; + for (const chunk of chunks) { + bytes.set(chunk, offset); + offset += chunk.length; + } + invariant( + offset === total, + `materialized ${String(offset)} bytes, expected ${String(total)}`, + ); + + const replayed = byteArrayBody(bytes, body.mediaType); + invariant(replayed.replayable, 'materialize must return a replayable body'); // BODY-3's postcondition + return replayed; +} diff --git a/packages/core/src/body/multipart-body.test.ts b/packages/core/src/body/multipart-body.test.ts new file mode 100644 index 0000000..0fa6d94 --- /dev/null +++ b/packages/core/src/body/multipart-body.test.ts @@ -0,0 +1,216 @@ +// SPDX-License-Identifier: MIT +// packages/core/src/body/multipart-body.test.ts +// Exercises: BODY-2 (composite replayability, unknown-length collapse), HTTP-51 (shared framing routine, +// boundary generation/validation, header quoting) +import {describe, expect, test} from 'bun:test'; +import fc from 'fast-check'; +import {MultipartBoundaryError} from './errors.js'; +import { + MultipartBody, + MultipartBodyBuilder, + multipartBody, +} from './multipart-body.js'; +import {byteArrayBody, stringBody} from './simple-bodies.js'; +import {streamBody} from './stream-body.js'; + +function emptyStream(): ReadableStream { + return new ReadableStream({ + start: c => { + c.close(); + }, + }); +} + +function oneByteStream(): ReadableStream { + return new ReadableStream({ + start(c) { + c.enqueue(Uint8Array.from([1])); + c.close(); + }, + }); +} + +async function drain(body: { + writeTo: (sink: WritableStream) => Promise; +}): Promise { + const chunks: Uint8Array[] = []; + await body.writeTo(new WritableStream({write: c => void chunks.push(c)})); + const total = chunks.reduce((s, c) => s + c.length, 0); + const out = new Uint8Array(total); + let offset = 0; + for (const c of chunks) { + out.set(c, offset); + offset += c.length; + } + return new TextDecoder().decode(out); +} + +describe('MultipartBody replayability and length (BODY-2)', () => { + test('replayable when every part is replayable', () => { + expect(multipartBody([{name: 'a', body: stringBody('x')}]).replayable).toBe( + true, + ); + }); + + test('not replayable when any part is not', () => { + const body = multipartBody([ + {name: 'a', body: stringBody('x')}, + {name: 'b', body: streamBody(oneByteStream())}, + ]); + expect(body.replayable).toBe(false); + }); + + test('declared length collapses to -1 if any part length is unknown (BODY-2)', () => { + expect( + multipartBody([{name: 'a', body: streamBody(emptyStream())}]) + .contentLength, + ).toBe(-1); + }); + + test('declared length equals the bytes actually written when every part length is known', async () => { + const body = multipartBody( + [{name: 'a', body: stringBody('hello')}], + 'FIXEDBOUNDARY', + ); + const rendered = await drain(body); + expect(new TextEncoder().encode(rendered).length).toBe(body.contentLength); + }); +}); + +describe('MultipartBody framing and headers (HTTP-51)', () => { + test('frames one part with boundary, headers, body, and a CRLF-terminated trailer', async () => { + const rendered = await drain( + multipartBody( + [ + { + name: 'field', + body: byteArrayBody(new TextEncoder().encode('value')), + }, + ], + 'B', + ), + ); + expect(rendered).toBe( + '--B\r\nContent-Disposition: form-data; name="field"\r\n\r\nvalue\r\n--B--\r\n', + ); + }); + + test('includes filename and Content-Type when the part has them', async () => { + const rendered = await drain( + multipartBody( + [ + { + name: 'file', + filename: 'a.txt', + body: byteArrayBody(Uint8Array.from([1]), 'text/plain'), + }, + ], + 'B', + ), + ); + expect(rendered).toContain('filename="a.txt"'); + expect(rendered).toContain('Content-Type: text/plain\r\n'); + }); + + test('quotes/escapes a quote or backslash in a part name, and strips embedded CR/LF (HTTP-51)', async () => { + const rendered = await drain( + multipartBody([{name: 'a"b\\c\r\nd', body: stringBody('x')}], 'B'), + ); + expect(rendered).toContain('name="a\\"b\\\\cd"'); + }); +}); + +describe('MultipartBody boundary generation and validation (HTTP-51)', () => { + test('a valid caller-supplied boundary is accepted', () => { + expect(() => + multipartBody([{name: 'a', body: stringBody('x')}], 'valid-boundary_1'), + ).not.toThrow(); + }); + + test('an invalid caller-supplied boundary throws MultipartBoundaryError', () => { + expect(() => + multipartBody([{name: 'a', body: stringBody('x')}], 'trailing space '), + ).toThrow(MultipartBoundaryError); + expect(() => + multipartBody([{name: 'a', body: stringBody('x')}], ''), + ).toThrow(MultipartBoundaryError); + }); + + test('an unsupplied boundary is generated and spec-valid', () => { + const body = multipartBody([{name: 'a', body: stringBody('x')}]); + expect(body.mediaType).toMatch( + /^multipart\/form-data; boundary=dexpace-[A-Za-z0-9]{32}$/, + ); + }); + + test('two generated boundaries differ', () => { + const a = multipartBody([{name: 'a', body: stringBody('x')}]); + const b = multipartBody([{name: 'a', body: stringBody('x')}]); + expect(a.mediaType).not.toBe(b.mediaType); + }); +}); + +describe('MultipartBodyBuilder (HTTP-2, HTTP-3)', () => { + test('static newBuilder and instance newBuilder pre-populates parts and boundary', async () => { + const original = MultipartBody.newBuilder() + .addPart({name: 'p1', body: stringBody('v1')}) + .boundary('CUSTOMB') + .build(); + + expect(original.contentLength).toBeGreaterThan(0); + + const derived = original + .newBuilder() + .addPart({name: 'p2', body: stringBody('v2')}) + .build(); + expect(derived.mediaType).toBe('multipart/form-data; boundary=CUSTOMB'); + const rendered = await drain(derived); + expect(rendered).toContain('name="p1"'); + expect(rendered).toContain('name="p2"'); + }); + + test('MultipartBodyBuilder.parts sets the parts list', async () => { + const builder = new MultipartBodyBuilder(); + builder.parts([{name: 'a', body: stringBody('1')}]); + const body = builder.build(); + expect(await drain(body)).toContain('name="a"'); + }); +}); + +describe('MultipartBody property tests (HTTP-51)', () => { + test('declared length always equals the bytes written, for any part set (HTTP-51)', async () => { + await fc.assert( + fc.asyncProperty( + fc.array(fc.record({name: fc.string(), content: fc.string()}), { + minLength: 1, + maxLength: 8, + }), + async specs => { + const body = multipartBody( + specs.map(s => ({name: s.name, body: stringBody(s.content)})), + ); + const written = new TextEncoder().encode(await drain(body)).length; + expect(written).toBe(body.contentLength); + }, + ), + {seed: 0x3b}, + ); + }); + + test('a part name containing CR/LF or a quote never breaks the framing (HTTP-51)', async () => { + await fc.assert( + fc.asyncProperty(fc.string(), async name => { + const rendered = await drain( + multipartBody( + [{name, body: byteArrayBody(new TextEncoder().encode('x'))}], + 'B', + ), + ); + const headerBlock = rendered.slice(0, rendered.indexOf('\r\n\r\n')); + // exactly two CRLFs of framing (boundary line, disposition line) -- no injected extras + expect(headerBlock.split('\r\n').length).toBe(2); + }), + {seed: 0x3b}, + ); + }); +}); diff --git a/packages/core/src/body/multipart-body.ts b/packages/core/src/body/multipart-body.ts new file mode 100644 index 0000000..e3d4c4e --- /dev/null +++ b/packages/core/src/body/multipart-body.ts @@ -0,0 +1,191 @@ +// SPDX-License-Identifier: MIT +// packages/core/src/body/multipart-body.ts +import type {Builder} from '../http/builder.js'; +import {invariant} from '../invariant.js'; +import type {Body} from './body.js'; +import {MultipartBoundaryError} from './errors.js'; + +/** + * A part inside a {@link MultipartBody}. + * + * @public + */ +export interface MultipartPart { + readonly name: string; + readonly filename?: string | undefined; + readonly body: Body; +} + +const BOUNDARY_CHARS = + 'ABCDEFGHIJKLMNOPQRSTUVWXYZabcdefghijklmnopqrstuvwxyz0123456789'; +// RFC 2046 bchars grammar: 1-70 chars, last char not a space. +const BOUNDARY_PATTERN = + /^[A-Za-z0-9'()+_,\-./:=? ]{1,69}[A-Za-z0-9'()+_,\-./:=?]$/; +const SINGLE_CHAR_BOUNDARY_PATTERN = /^[A-Za-z0-9'()+_,\-./:=?]$/; +const CRLF = new TextEncoder().encode('\r\n'); + +function generateBoundary(): string { + const bytes = new Uint8Array(32); + crypto.getRandomValues(bytes); + let boundary = 'dexpace-'; + for (const byte of bytes) { + const char = BOUNDARY_CHARS[byte % BOUNDARY_CHARS.length]; + invariant(char !== undefined, 'boundary character must be defined'); + boundary += char; + } + return boundary; +} + +function validateBoundary(boundary: string): void { + const valid = + boundary.length === 1 + ? SINGLE_CHAR_BOUNDARY_PATTERN.test(boundary) + : BOUNDARY_PATTERN.test(boundary); + if (!valid) throw new MultipartBoundaryError(boundary); +} + +// Escapes a quote/backslash so it cannot break the quoted-string grammar, and strips CR/LF outright so +// they can never break the header framing (HTTP-51). +function quoteParam(value: string): string { + return value.replace(/[\\"]/g, ch => `\\${ch}`).replace(/[\r\n]/g, ''); +} + +// The shared framing routine HTTP-51 requires: both computeContentLength and writeTo call this for every +// part, so the declared length and the written bytes cannot drift. +function renderPartHeader(part: MultipartPart, boundary: string): Uint8Array { + let header = `--${boundary}\r\n`; + header += `Content-Disposition: form-data; name="${quoteParam(part.name)}"`; + if (part.filename !== undefined) + header += `; filename="${quoteParam(part.filename)}"`; + header += '\r\n'; + if (part.body.mediaType !== undefined) + header += `Content-Type: ${part.body.mediaType}\r\n`; + header += '\r\n'; + return new TextEncoder().encode(header); +} + +function trailerBytes(boundary: string): Uint8Array { + return new TextEncoder().encode(`--${boundary}--\r\n`); +} + +function computeContentLength( + parts: readonly MultipartPart[], + boundary: string, +): number { + let total = 0; + for (const part of parts) { + if (part.body.contentLength === -1) return -1; // BODY-2: any unknown part collapses the whole + total += + renderPartHeader(part, boundary).length + + part.body.contentLength + + CRLF.length; + } + return total + trailerBytes(boundary).length; +} + +// Wraps a locked writer as a WritableStream whose close() does not close the real sink -- multiple parts +// share one underlying writer, and only the outer writeTo's own finally block closes it. +function nonClosingSink( + writer: WritableStreamDefaultWriter, +): WritableStream { + return new WritableStream({ + write: async chunk => { + await writer.write(chunk); + }, + }); +} + +/** + * A composite body (BODY-2, HTTP-51). Replayable iff every part is; declared length collapses to unknown + * if any part's length is unknown. + * + * @public + */ +export class MultipartBody implements Body { + readonly kind = 'multipart' as const; + readonly mediaType: string; + readonly contentLength: number; + readonly replayable: boolean; + readonly #parts: readonly MultipartPart[]; + readonly #boundary: string; + + constructor(parts: readonly MultipartPart[], boundary?: string) { + if (boundary !== undefined) validateBoundary(boundary); + this.#boundary = boundary ?? generateBoundary(); + this.#parts = [...parts]; + this.mediaType = `multipart/form-data; boundary=${this.#boundary}`; + this.replayable = this.#parts.every(part => part.body.replayable); + this.contentLength = computeContentLength(this.#parts, this.#boundary); + invariant( + this.contentLength === -1 || + this.contentLength >= trailerBytes(this.#boundary).length, + `framing computed an impossible length ${String(this.contentLength)}`, + ); + } + + static newBuilder(): MultipartBodyBuilder { + return new MultipartBodyBuilder(); + } + + /** HTTP-3: pre-populated with this instance's parts and boundary, aliasing neither. */ + newBuilder(): MultipartBodyBuilder { + return new MultipartBodyBuilder() + .parts(this.#parts) + .boundary(this.#boundary); + } + + async writeTo(sink: WritableStream): Promise { + const writer = sink.getWriter(); + try { + for (const part of this.#parts) { + await writer.write(renderPartHeader(part, this.#boundary)); + await part.body.writeTo(nonClosingSink(writer)); + await writer.write(CRLF); + } + await writer.write(trailerBytes(this.#boundary)); + } finally { + await writer.close(); + } + } +} + +/** + * Creates a MultipartBody (BODY-2, HTTP-51). + * + * @public + */ +export function multipartBody( + parts: readonly MultipartPart[], + boundary?: string, +): MultipartBody { + return new MultipartBody(parts, boundary); +} + +/** + * Builder for {@link MultipartBody}. + * + * @public + */ +export class MultipartBodyBuilder implements Builder { + #parts: MultipartPart[] = []; + #boundary: string | undefined; + + parts(parts: readonly MultipartPart[]): this { + this.#parts = [...parts]; + return this; + } + + addPart(part: MultipartPart): this { + this.#parts.push(part); + return this; + } + + boundary(boundary: string | undefined): this { + this.#boundary = boundary; + return this; + } + + build(): MultipartBody { + return new MultipartBody(this.#parts, this.#boundary); + } +} diff --git a/packages/core/src/body/request-body-logging.test.ts b/packages/core/src/body/request-body-logging.test.ts new file mode 100644 index 0000000..cd30b99 --- /dev/null +++ b/packages/core/src/body/request-body-logging.test.ts @@ -0,0 +1,153 @@ +// SPDX-License-Identifier: MIT +// packages/core/src/body/request-body-logging.test.ts +// Exercises: BODY-17 (mirror + forward the full untruncated payload), BODY-18 (tap clears at the start +// of every write), BODY-19 (tap cap, full payload unaffected), BODY-20 (partial-failure snapshot), BODY-21 +// (replayable/materialize pass through, preserving the tap), BODY-37 (no backing-buffer escape hatch) +import {describe, expect, test} from 'bun:test'; +import fc from 'fast-check'; +import {InvariantViolation} from '../invariant.js'; +import {withRequestLogging} from './request-body-logging.js'; +import {byteArrayBody} from './simple-bodies.js'; +import {streamBody} from './stream-body.js'; + +function collectingSink(): { + sink: WritableStream; + written: () => Uint8Array; +} { + const chunks: Uint8Array[] = []; + const sink = new WritableStream({ + write: c => void chunks.push(c), + }); + return { + sink, + written: () => { + const total = chunks.reduce((s, c) => s + c.length, 0); + const out = new Uint8Array(total); + let offset = 0; + for (const c of chunks) { + out.set(c, offset); + offset += c.length; + } + return out; + }, + }; +} + +describe('withRequestLogging mirroring and caps (BODY-17..20)', () => { + test('forwards the full payload untruncated regardless of the tap cap (BODY-17, BODY-19)', async () => { + const logged = withRequestLogging( + byteArrayBody(Uint8Array.from([1, 2, 3, 4, 5])), + 2, + ); + const {sink, written} = collectingSink(); + await logged.writeTo(sink); + expect([...written()]).toEqual([1, 2, 3, 4, 5]); + expect([...logged.snapshot()]).toEqual([1, 2]); + }); + + test('the tap clears at the start of every write (BODY-18)', async () => { + const logged = withRequestLogging( + byteArrayBody(Uint8Array.from([9, 9])), + 10, + ); + await logged.writeTo(collectingSink().sink); + await logged.writeTo(collectingSink().sink); + expect([...logged.snapshot()]).toEqual([9, 9]); // not [9, 9, 9, 9] + }); + + test('a tap cap of 0 mirrors nothing while still forwarding everything', async () => { + const logged = withRequestLogging( + byteArrayBody(Uint8Array.from([1, 2])), + 0, + ); + const {sink, written} = collectingSink(); + await logged.writeTo(sink); + expect([...written()]).toEqual([1, 2]); + expect(logged.snapshot().length).toBe(0); + }); + + test('a partial write failure still leaves the bytes mirrored up to that point (BODY-20)', () => { + const failing = new WritableStream({ + write: (_chunk, controller) => { + controller.error(new Error('boom')); + }, + }); + const logged = withRequestLogging( + byteArrayBody(Uint8Array.from([1, 2, 3])), + 10, + ); + expect(logged.writeTo(failing)).rejects.toThrow(); + expect(logged.snapshot().length).toBeGreaterThan(0); + }); +}); + +describe('withRequestLogging replayability, materialize, and protection (BODY-21, 32, 37)', () => { + test('replayable passes through the delegate verbatim (BODY-21)', () => { + expect( + withRequestLogging(byteArrayBody(Uint8Array.from([1])), 10).replayable, + ).toBe(true); + const singleUse = withRequestLogging( + streamBody( + new ReadableStream({ + start: c => { + c.close(); + }, + }), + ), + 10, + ); + expect(singleUse.replayable).toBe(false); + }); + + test('materialize() returns a still-logged, now-replayable wrapper preserving the tap (BODY-21)', async () => { + const stream = new ReadableStream({ + start(controller) { + controller.enqueue(Uint8Array.from([7, 7])); + controller.close(); + }, + }); + const logged = withRequestLogging(streamBody(stream), 10); + expect(logged.replayable).toBe(false); + + const materialized = await logged.materialize(); + expect(materialized.replayable).toBe(true); + expect(typeof materialized.snapshot).toBe('function'); + + const {sink, written} = collectingSink(); + await materialized.writeTo(sink); + expect([...written()]).toEqual([7, 7]); + expect([...materialized.snapshot()]).toEqual([7, 7]); + }); + + test('exposes no direct handle onto the tap buffer -- snapshot is the only read path (BODY-37)', () => { + const logged = withRequestLogging(byteArrayBody(Uint8Array.from([1])), 10); + expect(Object.keys(logged)).not.toContain('tap'); + expect(Object.keys(logged)).not.toContain('buffer'); + }); + + test('the primary always receives the exact payload, independent of the tap cap (BODY-17)', async () => { + await fc.assert( + fc.asyncProperty( + fc.uint8Array({minLength: 0, maxLength: 512}), + fc.integer({min: 0, max: 600}), + async (payload, tapCap) => { + const logged = withRequestLogging(byteArrayBody(payload), tapCap); + const {sink, written} = collectingSink(); + await logged.writeTo(sink); + + expect([...written()]).toEqual([...payload]); // wire body never reduced or altered + expect(logged.snapshot().length).toBe( + Math.min(payload.length, tapCap), + ); // tap bounded + }, + ), + {seed: 0x3b}, + ); + }); + + test('a negative tap cap is rejected at construction (BODY-32)', () => { + expect(() => + withRequestLogging(byteArrayBody(Uint8Array.from([1])), -1), + ).toThrow(InvariantViolation); + }); +}); diff --git a/packages/core/src/body/request-body-logging.ts b/packages/core/src/body/request-body-logging.ts new file mode 100644 index 0000000..502e416 --- /dev/null +++ b/packages/core/src/body/request-body-logging.ts @@ -0,0 +1,77 @@ +// SPDX-License-Identifier: MIT +// packages/core/src/body/request-body-logging.ts +import {invariant} from '../invariant.js'; +import {ByteQueue} from '../io/byte-queue.js'; +import {MAX_BYTE_ARRAY_LENGTH} from '../io/limits.js'; +import type {Body} from './body.js'; +import {materialize} from './materialize.js'; + +export interface LoggedBody extends Body { + /** A copy of the tap's current contents -- at most tapCapBytes of the most recent write (BODY-19). */ + snapshot(): Uint8Array; + /** Materializes the delegate while preserving the logging wrapper and the tap (BODY-21). */ + materialize(): Promise; +} + +/** + * Mirrors up to tapCapBytes of each writeTo call into an internal tap while forwarding the full, + * untruncated payload to the primary sink (BODY-17). The tap clears at the start of every write so a + * retry against a replayable delegate does not accumulate stale bytes (BODY-18). No handle onto the tap's + * backing buffer is exposed -- snapshot() is the only way to read it (BODY-37). `@internal` -- unwired + * until Phase 7 supplies a Logger to drive it. + */ +export function withRequestLogging( + delegate: Body, + tapCapBytes: number, +): LoggedBody { + // BODY-32: reject a negative cap, clamp to the platform's max single-array size. Without the guard a + // negative cap makes `tap.size < cap` permanently false and the tee silently mirrors nothing. + invariant( + tapCapBytes >= 0, + `tapCapBytes must be non-negative, got ${String(tapCapBytes)}`, + ); + const cap = Math.min(tapCapBytes, MAX_BYTE_ARRAY_LENGTH); + const tap = new ByteQueue(); + + function wrap(inner: Body): LoggedBody { + return { + kind: inner.kind, + mediaType: inner.mediaType, + contentLength: inner.contentLength, + get replayable() { + return inner.replayable; + }, + async writeTo(sink: WritableStream): Promise { + tap.clear(); // BODY-18 + const writer = sink.getWriter(); + const tapped = new WritableStream({ + write: async chunk => { + if (tap.size < cap) { + const room = cap - tap.size; + // BODY-20/IO-27: mirror BEFORE forwarding, so a failing primary write still captures + // the chunk that failed. + tap.writeBytes( + room >= chunk.length ? chunk : chunk.subarray(0, room), + ); + } + await writer.write(chunk); // BODY-19: the full payload always reaches the primary + invariant( + tap.size <= cap, + `tap grew past its ${String(cap)}-byte cap`, + ); + }, + close: async () => { + await writer.close(); + }, + }); + await inner.writeTo(tapped); + }, + snapshot(): Uint8Array { + return tap.snapshot(); + }, + materialize: async () => wrap(await materialize(inner)), + }; + } + + return wrap(delegate); +} diff --git a/packages/core/src/body/response-body-logging.test.ts b/packages/core/src/body/response-body-logging.test.ts new file mode 100644 index 0000000..2930be3 --- /dev/null +++ b/packages/core/src/body/response-body-logging.test.ts @@ -0,0 +1,157 @@ +// SPDX-License-Identifier: MIT +// packages/core/src/body/response-body-logging.test.ts +// Exercises: BODY-22 (lazy, drain-once), BODY-23 (fits-cap: full capture, repeatable non-consuming +// reads), BODY-24 (exceeds-cap: prefix+tail once, second read fails), BODY-26 (drain failure cached, +// partial bytes retained, error() does not drain), BODY-27 (close-once shared guard), BODY-28 (captured +// buffer survives close), BODY-29 (reported length), BODY-32 (negative cap rejected) +import {describe, expect, test} from 'bun:test'; +import fc from 'fast-check'; +import {InvariantViolation} from '../invariant.js'; +import {withResponseLogging} from './response-body-logging.js'; + +function readableOf(...chunks: number[][]): ReadableStream { + return new ReadableStream({ + start(controller) { + for (const chunk of chunks) controller.enqueue(Uint8Array.from(chunk)); + controller.close(); + }, + }); +} + +async function readAll( + stream: ReadableStream, +): Promise { + const reader = stream.getReader(); + const chunks: Uint8Array[] = []; + let total = 0; + for (;;) { + const {done, value} = await reader.read(); + if (done) break; + chunks.push(value); + total += value.length; + } + const out = new Uint8Array(total); + let offset = 0; + for (const chunk of chunks) { + out.set(chunk, offset); + offset += chunk.length; + } + return out; +} + +describe('withResponseLogging regimes (BODY-22..24)', () => { + test('nothing is captured until read() is called (BODY-22 laziness)', () => { + expect( + withResponseLogging(readableOf([1, 2, 3]), 100).snapshot().length, + ).toBe(0); + }); + + test('fits-cap: fully captures, and every later read() is a fresh non-consuming view (BODY-23)', async () => { + const logged = withResponseLogging(readableOf([1, 2, 3]), 100); + expect([...(await readAll(await logged.read()))]).toEqual([1, 2, 3]); + expect([...(await readAll(await logged.read()))]).toEqual([1, 2, 3]); + expect([...logged.snapshot()]).toEqual([1, 2, 3]); + }); + + test('exceeds-cap: replays the prefix then the live tail, consumer receives the complete body (BODY-24)', async () => { + const logged = withResponseLogging(readableOf([1, 2], [3, 4, 5]), 3); + expect([...(await readAll(await logged.read()))]).toEqual([1, 2, 3, 4, 5]); + expect([...logged.snapshot()]).toEqual([1, 2, 3]); // only the prefix up to the cap is retained + }); + + test('exceeds-cap: a second read() throws (BODY-24)', async () => { + const logged = withResponseLogging(readableOf([1, 2, 3, 4]), 1); + await logged.read(); + expect(logged.read()).rejects.toThrow(); + }); +}); + +describe('withResponseLogging lifecycle (BODY-27, 28)', () => { + test('close is idempotent and shared across the wrapper close and tail completion (BODY-27)', async () => { + const logged = withResponseLogging(readableOf([1, 2, 3]), 100); + await readAll(await logged.read()); + await logged.close(); + await logged.close(); + }); + + test('the captured buffer survives close -- snapshot still works after (BODY-28)', async () => { + const logged = withResponseLogging(readableOf([1, 2]), 100); + await readAll(await logged.read()); + await logged.close(); + expect([...logged.snapshot()]).toEqual([1, 2]); + }); + + test('[Symbol.asyncDispose] delegates to close()', async () => { + await withResponseLogging(readableOf([1]), 100)[Symbol.asyncDispose](); + }); +}); + +describe('withResponseLogging error caching (BODY-26)', () => { + test('a drain failure is cached: read() re-throws it, snapshot keeps the partial bytes (BODY-26)', () => { + const boom = new Error('upstream reset'); + const failing = new ReadableStream({ + start(controller) { + controller.enqueue(Uint8Array.from([1, 2])); + }, + pull(controller) { + controller.error(boom); + }, + }); + const logged = withResponseLogging(failing, 100); + + expect(logged.read()).rejects.toBe(boom); + expect(logged.read()).rejects.toBe(boom); // same cached error, upstream never re-read + expect([...logged.snapshot()]).toEqual([1, 2]); // partial capture retained, snapshot does not throw + expect(logged.error()).toBe(boom); + }); + + test('error() reports null without triggering a drain (BODY-26)', () => { + const logged = withResponseLogging(readableOf([1, 2, 3]), 100); + expect(logged.error()).toBeNull(); + expect(logged.snapshot().length).toBe(0); // still undrained -- error() did not read anything + }); +}); + +describe('withResponseLogging properties and lengths (BODY-29..34)', () => { + test('contentLength is the captured size when it fits, the declared length when it does not (BODY-29)', async () => { + const fits = withResponseLogging(readableOf([1, 2, 3]), 100, 3); + await fits.read(); + expect(fits.contentLength).toBe(3); + + const exceeds = withResponseLogging(readableOf([1, 2, 3, 4]), 2, 4); + await exceeds.read(); + expect(exceeds.contentLength).toBe(4); // the delegate's true length, not the 2-byte prefix + }); + + test('a negative cap is rejected at construction (BODY-32)', () => { + expect(() => withResponseLogging(readableOf([1]), -1)).toThrow( + InvariantViolation, + ); + }); + + test('for any (cap, body) pair the consumer receives every byte and the tap stays bounded', async () => { + await fc.assert( + fc.asyncProperty( + fc.uint8Array({minLength: 0, maxLength: 512}), + fc.integer({min: 0, max: 600}), + async (payload, cap) => { + const source = new ReadableStream({ + start(controller) { + if (payload.length > 0) controller.enqueue(payload); + controller.close(); + }, + }); + const logged = withResponseLogging(source, cap); + + // BODY-34: the consumer gets the complete body whichever regime triggered. + expect([...(await readAll(await logged.read()))]).toEqual([ + ...payload, + ]); + // BODY-23/BODY-24: the capture is bounded by the cap either way. + expect(logged.snapshot().length).toBe(Math.min(payload.length, cap)); + }, + ), + {seed: 0x3b}, + ); + }); +}); diff --git a/packages/core/src/body/response-body-logging.ts b/packages/core/src/body/response-body-logging.ts new file mode 100644 index 0000000..712b4e4 --- /dev/null +++ b/packages/core/src/body/response-body-logging.ts @@ -0,0 +1,191 @@ +// SPDX-License-Identifier: MIT +// packages/core/src/body/response-body-logging.ts +import {invariant} from '../invariant.js'; +import {ByteQueue} from '../io/byte-queue.js'; +import {MAX_BYTE_ARRAY_LENGTH} from '../io/limits.js'; +import {ConsumedBodyError} from './errors.js'; + +export interface LoggedResponseBody extends AsyncDisposable { + /** + * Returns a stream serving the body. Lazy -- nothing is read from the delegate until the first call + * (BODY-22). Fits-cap regime: every call, including calls after the first, returns a fresh + * non-consuming view over the captured bytes (BODY-23). Exceeds-cap regime: exactly one call is + * allowed; a second throws (BODY-24). If the drain failed, every call re-throws the cached error. + */ + read(): Promise>; + /** Non-consuming; reflects whatever has been captured so far, even after a failed drain (BODY-26). */ + snapshot(): Uint8Array; + /** The cached drain failure, or null. MUST NOT trigger a drain (BODY-26). */ + error(): Error | null; + /** Captured size iff fully captured within the cap, else the delegate's declared length (BODY-29). */ + readonly contentLength: number; + close(): Promise; +} + +/** + * Mutable state for one wrapper instance. Extracted from the factory closure so the factory stays under + * the 70-line function cap and each step below is independently testable. + */ +interface DrainState { + readonly captured: ByteQueue; + readonly reader: ReadableStreamDefaultReader; + readonly delegate: ReadableStream; + readonly cap: number; + regime: 'undrained' | 'fits' | 'exceeds'; + tailConsumed: boolean; + pendingTailChunk: Uint8Array | undefined; + failure: Error | null; + closed: boolean; + started: Promise | undefined; +} + +/** BODY-27: one close-once guard shared by the wrapper's close and the tail stream's completion. */ +async function closeDelegate(state: DrainState): Promise { + if (state.closed) return; + state.closed = true; + // MUST precede cancel(): cancel() rejects with TypeError on a locked stream, and reading to done does + // not release the lock (see Response.bytes for the same trap). + state.reader.releaseLock(); + // BODY-28: on the fits-cap path the capture already succeeded, so a close failure is best-effort and + // must not surface as a drain error. Narrowed to the one thing cancel() reports here. + await state.delegate.cancel().catch((error: unknown) => { + if (!(error instanceof TypeError)) throw error; + }); +} + +/** + * Reads until EOF (fits regime) or until the cap is reached (exceeds regime, leaving the delegate open + * and the overflow chunk staged). BODY-26: a failure is cached, never allowed to truncate silently. + * + * BODY-25 note: the requirement's "zero bytes returned for a positive requested count" has no analog + * here -- `ReadableStreamDefaultReader.read()` takes no count, and a zero-length chunk is a legal + * no-op, not an EOF signal. EOF is signalled only by `{done: true}`, which is what the loop keys on. + */ +async function drainOnce(state: DrainState): Promise { + try { + for (;;) { + // Serial by necessity: each read depends on the previous one advancing the cursor. + const {done, value} = await state.reader.read(); + if (done) { + state.regime = 'fits'; + await closeDelegate(state); + return; + } + if (state.captured.size + value.length <= state.cap) { + state.captured.writeBytes(value); + continue; + } + const room = state.cap - state.captured.size; + if (room > 0) state.captured.writeBytes(value.subarray(0, room)); + state.pendingTailChunk = value.subarray(room); + state.regime = 'exceeds'; + invariant( + state.captured.size <= state.cap, + `captured past the ${String(state.cap)}-byte cap`, + ); + return; + } + } catch (error: unknown) { + // BODY-26: retain what was read and cache the error rather than discarding a partial capture. + state.failure = error instanceof Error ? error : new Error(String(error)); + throw state.failure; + } +} + +/** A fresh, non-consuming view over the fully-captured bytes. Repeatable (BODY-23). */ +function capturedStream(state: DrainState): ReadableStream { + const bytes = state.captured.snapshot(); + return new ReadableStream({ + start(controller) { + if (bytes.length > 0) controller.enqueue(bytes); + controller.close(); + }, + }); +} + +/** + * Replays the captured prefix, then continues from the still-live tail (BODY-24). Pull-driven, one + * chunk per pull: looping inside start() would eagerly materialize the whole remaining body in the + * controller's queue -- precisely the oversized payloads the cap exists to keep off the heap. + */ +function tailStream(state: DrainState): ReadableStream { + const prefix = state.captured.snapshot(); + let staged: Uint8Array | undefined = state.pendingTailChunk; + let prefixSent = false; + return new ReadableStream({ + async pull(controller) { + if (!prefixSent) { + prefixSent = true; + if (prefix.length > 0) { + controller.enqueue(prefix); + return; + } + } + if (staged !== undefined) { + const chunk = staged; + staged = undefined; + if (chunk.length > 0) { + controller.enqueue(chunk); + return; + } + } + const {done, value} = await state.reader.read(); + if (done) { + await closeDelegate(state); + controller.close(); + return; + } + controller.enqueue(value); + }, + async cancel() { + await closeDelegate(state); + }, + }); +} + +/** + * Wraps a raw response body stream (BODY-22..29). `@internal` -- unwired until Phase 7 supplies a Logger. + */ +export function withResponseLogging( + delegate: ReadableStream, + capBytes: number, + declaredLength = -1, +): LoggedResponseBody { + invariant( + capBytes >= 0, + `capBytes must be non-negative, got ${String(capBytes)}`, + ); // BODY-32 + const state: DrainState = { + captured: new ByteQueue(), + reader: delegate.getReader(), + delegate, + cap: Math.min(capBytes, MAX_BYTE_ARRAY_LENGTH), // BODY-32: clamp, do not attempt an impossible allocation + regime: 'undrained', + tailConsumed: false, + pendingTailChunk: undefined, + failure: null, + closed: false, + started: undefined, + }; + + return { + async read(): Promise> { + state.started ??= drainOnce(state); + await state.started; // a cached failure re-throws here on every call (BODY-26) + if (state.regime === 'fits') return capturedStream(state); + if (state.tailConsumed) { + throw new ConsumedBodyError('logged-response'); + } + state.tailConsumed = true; + return tailStream(state); + }, + snapshot: () => state.captured.snapshot(), + error: () => state.failure, // deliberately does not drain (BODY-26) + get contentLength(): number { + // BODY-29: the capture is the true length only when the whole body fit within the cap. + return state.regime === 'fits' ? state.captured.size : declaredLength; + }, + close: () => closeDelegate(state), + [Symbol.asyncDispose]: () => closeDelegate(state), + }; +} diff --git a/packages/core/src/body/simple-bodies.test.ts b/packages/core/src/body/simple-bodies.test.ts new file mode 100644 index 0000000..4a800c6 --- /dev/null +++ b/packages/core/src/body/simple-bodies.test.ts @@ -0,0 +1,97 @@ +// SPDX-License-Identifier: MIT +// packages/core/src/body/simple-bodies.test.ts +// Exercises: HTTP-36/BODY-1 (mediaType, contentLength, replayable, writeTo), HTTP-38/BODY-35 (replayable +// by source; form-urlencoded uses "+" for space, distinct from RFC 3986 query encoding) +import {describe, expect, test} from 'bun:test'; +import { + byteArrayBody, + formUrlEncodedBody, + stringBody, +} from './simple-bodies.js'; + +async function drain(body: { + writeTo: (sink: WritableStream) => Promise; +}): Promise { + const chunks: Uint8Array[] = []; + await body.writeTo( + new WritableStream({write: chunk => void chunks.push(chunk)}), + ); + const total = chunks.reduce((sum, chunk) => sum + chunk.length, 0); + const result = new Uint8Array(total); + let offset = 0; + for (const chunk of chunks) { + result.set(chunk, offset); + offset += chunk.length; + } + return result; +} + +describe('ByteArrayBody', () => { + test('reports kind, mediaType, contentLength, and is always replayable', () => { + const body = byteArrayBody( + Uint8Array.from([1, 2, 3]), + 'application/octet-stream', + ); + expect(body.kind).toBe('byte-array'); + expect(body.mediaType).toBe('application/octet-stream'); + expect(body.contentLength).toBe(3); + expect(body.replayable).toBe(true); + }); + + test('defaults mediaType to undefined -- absence is undefined, never null', () => { + expect(byteArrayBody(Uint8Array.from([1])).mediaType).toBeUndefined(); + }); + + test('writeTo emits the exact bytes, twice, byte-for-byte identical (BODY-1)', async () => { + const body = byteArrayBody(Uint8Array.from([9, 8, 7])); + expect([...(await drain(body))]).toEqual([9, 8, 7]); + expect([...(await drain(body))]).toEqual([9, 8, 7]); + }); + + test('holds an independent copy -- mutating the caller array afterwards does not change it', async () => { + const input = Uint8Array.from([1, 2, 3]); + const body = byteArrayBody(input); + input[0] = 99; + expect([...(await drain(body))]).toEqual([1, 2, 3]); + }); +}); + +describe('StringBody', () => { + test('encodes UTF-8 and reports the byte length, not the character length', () => { + const body = stringBody('héllo'); + expect(body.contentLength).toBe(6); // "é" is 2 bytes in UTF-8 + expect(body.replayable).toBe(true); + }); + + test('writeTo emits the UTF-8 bytes', async () => { + expect(new TextDecoder().decode(await drain(stringBody('hi')))).toBe('hi'); + }); +}); + +describe('FormUrlEncodedBody (HTTP-38/BODY-35)', () => { + test('mediaType is fixed and the body is always replayable', () => { + const body = formUrlEncodedBody(new Map([['a', 'b']])); + expect(body.mediaType).toBe('application/x-www-form-urlencoded'); + expect(body.replayable).toBe(true); + }); + + test('encodes space as "+" rather than "%20"', async () => { + const body = formUrlEncodedBody(new Map([['q', 'a b']])); + expect(new TextDecoder().decode(await drain(body))).toBe('q=a+b'); + }); + + test('joins multiple params with "&", preserving insertion order', async () => { + const body = formUrlEncodedBody( + new Map([ + ['a', '1'], + ['b', '2'], + ]), + ); + expect(new TextDecoder().decode(await drain(body))).toBe('a=1&b=2'); + }); + + test('percent-encodes reserved characters in keys and values', async () => { + const body = formUrlEncodedBody(new Map([['a&b', 'c=d']])); + expect(new TextDecoder().decode(await drain(body))).toBe('a%26b=c%3Dd'); + }); +}); diff --git a/packages/core/src/body/simple-bodies.ts b/packages/core/src/body/simple-bodies.ts new file mode 100644 index 0000000..21dbad7 --- /dev/null +++ b/packages/core/src/body/simple-bodies.ts @@ -0,0 +1,184 @@ +// SPDX-License-Identifier: MIT +// packages/core/src/body/simple-bodies.ts +import {QueryParams, type QueryParamsBuilder} from '../http/query-params.js'; +import {invariant} from '../invariant.js'; +import type {Body} from './body.js'; + +/** + * A body backed by an in-memory byte array (BODY-1). Always replayable. + * + * @public + */ +export class ByteArrayBody implements Body { + readonly kind = 'byte-array' as const; + readonly mediaType: string | undefined; + readonly contentLength: number; + readonly replayable = true; + readonly #bytes: Uint8Array; + + constructor(bytes: Uint8Array, mediaType?: string) { + // Defensive copy: `bytes` caller passed might be mutated later (HTTP-1). Kept `#private` -- + // exposing this publicly would let a caller mutate a "replayable" body's contents after + // construction, silently breaking the byte-for-byte-identical guarantee BODY-1 requires. + this.#bytes = Uint8Array.from(bytes); + this.mediaType = mediaType; + this.contentLength = this.#bytes.length; + } + + async writeTo(sink: WritableStream): Promise { + const writer = sink.getWriter(); + try { + if (this.#bytes.length > 0) await writer.write(this.#bytes); + } finally { + await writer.close(); + } + } +} + +/** + * Creates a replayable ByteArrayBody (BODY-1). + * + * @public + */ +export function byteArrayBody( + bytes: Uint8Array, + mediaType?: string, +): ByteArrayBody { + return new ByteArrayBody(bytes, mediaType); +} + +/** + * A body backed by an in-memory string (BODY-1). Always replayable. + * + * @public + */ +export class StringBody implements Body { + readonly kind = 'string' as const; + readonly mediaType: string; + readonly contentLength: number; + readonly replayable = true; + readonly text: string; + readonly #bytes: Uint8Array; + + constructor(text: string, mediaType = 'text/plain; charset=utf-8') { + this.text = text; + this.mediaType = mediaType; + this.#bytes = new TextEncoder().encode(text); + this.contentLength = this.#bytes.length; + } + + async writeTo(sink: WritableStream): Promise { + const writer = sink.getWriter(); + try { + if (this.#bytes.length > 0) await writer.write(this.#bytes); + } finally { + await writer.close(); + } + } +} + +/** + * Creates a replayable StringBody (BODY-1). + * + * @public + */ +export function stringBody( + text: string, + mediaType = 'text/plain; charset=utf-8', +): StringBody { + return new StringBody(text, mediaType); +} + +/** + * Accepted input shapes for {@link formUrlEncodedBody}. + * + * @public + */ +export type FormUrlEncodedInput = + | QueryParams + | ReadonlyMap + | Record + | readonly (readonly [string, string])[]; + +function addParamValue( + builder: QueryParamsBuilder, + key: string, + value: unknown, +): void { + if (Array.isArray(value)) { + for (const v of value) { + if (typeof v === 'string') builder.add(key, v); + } + } else if (typeof value === 'string' || value === null) { + builder.add(key, value); + } +} + +function toQueryParams(input: FormUrlEncodedInput): QueryParams { + if (input instanceof QueryParams) return input; + const builder = QueryParams.newBuilder(); + if (input instanceof Map) { + for (const [key, value] of input.entries()) { + if (typeof key === 'string') addParamValue(builder, key, value); + } + } else if (Array.isArray(input)) { + for (const [key, value] of input as readonly (readonly [ + unknown, + unknown, + ])[]) { + if (typeof key === 'string' && typeof value === 'string') { + builder.add(key, value); + } + } + } else { + for (const [key, value] of Object.entries(input)) { + addParamValue(builder, key, value); + } + } + return builder.build(); +} + +/** + * A body backed by URL-encoded form data (BODY-1, HTTP-50). Always replayable. + * + * @public + */ +export class FormUrlEncodedBody implements Body { + readonly kind = 'form-urlencoded' as const; + readonly mediaType = 'application/x-www-form-urlencoded'; + readonly contentLength: number; + readonly replayable = true; + readonly params: QueryParams; + readonly #bytes: Uint8Array; + + constructor(input: FormUrlEncodedInput) { + this.params = toQueryParams(input); + const encoded = this.params.encode().replace(/%20/g, '+'); // HTTP-50: space encoded as '+' + invariant( + !encoded.includes(' '), + 'form-urlencoded encoding produced illegal space', + ); + this.#bytes = new TextEncoder().encode(encoded); + this.contentLength = this.#bytes.length; + } + + async writeTo(sink: WritableStream): Promise { + const writer = sink.getWriter(); + try { + if (this.#bytes.length > 0) await writer.write(this.#bytes); + } finally { + await writer.close(); + } + } +} + +/** + * Creates a replayable FormUrlEncodedBody (BODY-1, HTTP-50). + * + * @public + */ +export function formUrlEncodedBody( + input: FormUrlEncodedInput, +): FormUrlEncodedBody { + return new FormUrlEncodedBody(input); +} diff --git a/packages/core/src/body/stream-body.test.ts b/packages/core/src/body/stream-body.test.ts new file mode 100644 index 0000000..8ed821f --- /dev/null +++ b/packages/core/src/body/stream-body.test.ts @@ -0,0 +1,118 @@ +// SPDX-License-Identifier: MIT +// packages/core/src/body/stream-body.test.ts +// Exercises: BODY-9 (always single-use -- no generic mark/reset on Node's ReadableStream), BODY-3 +// (second write fails loudly and is race-safe), BODY-8 (caller's stream is not force-closed -- read to +// natural exhaustion), HTTP-39/BODY-10 (declared length verified, short stream raises +// delivered-of-declared), IO-3 (a contentLength below the -1 sentinel is rejected) +import {describe, expect, test} from 'bun:test'; +import {InvariantViolation} from '../invariant.js'; +import {EndOfStreamError} from '../io/errors.js'; +import {ConsumedBodyError} from './errors.js'; +import {streamBody} from './stream-body.js'; + +function readableOf(...chunks: number[][]): ReadableStream { + return new ReadableStream({ + start(controller) { + for (const chunk of chunks) controller.enqueue(Uint8Array.from(chunk)); + controller.close(); + }, + }); +} + +function collectingSink(): { + sink: WritableStream; + written: () => Uint8Array; +} { + const chunks: Uint8Array[] = []; + const sink = new WritableStream({ + write: chunk => void chunks.push(chunk), + }); + return { + sink, + written: () => { + const total = chunks.reduce((sum, c) => sum + c.length, 0); + const out = new Uint8Array(total); + let offset = 0; + for (const chunk of chunks) { + out.set(chunk, offset); + offset += chunk.length; + } + return out; + }, + }; +} + +describe('StreamBody properties and writeTo (BODY-1, BODY-9)', () => { + test('is always single-use, regardless of declared length (BODY-9)', () => { + expect(streamBody(readableOf([1, 2]), undefined, 2).replayable).toBe(false); + }); + + test('reports the caller-supplied mediaType and contentLength', () => { + const body = streamBody(readableOf([1]), 'application/octet-stream', 1); + expect(body.mediaType).toBe('application/octet-stream'); + expect(body.contentLength).toBe(1); + }); + + test('defaults contentLength to -1 (unknown)', () => { + expect(streamBody(readableOf([1])).contentLength).toBe(-1); + }); + + test('writeTo forwards the exact bytes', async () => { + const {sink, written} = collectingSink(); + await streamBody(readableOf([1, 2], [3])).writeTo(sink); + expect([...written()]).toEqual([1, 2, 3]); + }); + + test('a second write throws ConsumedBodyError (BODY-3)', async () => { + const body = streamBody(readableOf([1])); + await body.writeTo(collectingSink().sink); + expect(body.writeTo(collectingSink().sink)).rejects.toThrow( + ConsumedBodyError, + ); + }); +}); + +describe('StreamBody declared length verification (HTTP-39, BODY-10, IO-3)', () => { + test('a declared length the stream cannot satisfy raises EndOfStreamError (HTTP-39/BODY-10)', () => { + const body = streamBody(readableOf([1, 2]), undefined, 5); + expect(body.writeTo(collectingSink().sink)).rejects.toThrow( + EndOfStreamError, + ); + }); + + test('a satisfied declared length writes exactly that many bytes (HTTP-39/BODY-10)', async () => { + const {sink, written} = collectingSink(); + await streamBody(readableOf([1, 2], [3]), undefined, 3).writeTo(sink); + expect([...written()]).toEqual([1, 2, 3]); + }); + + test('a declared length of 0 is a legitimate empty write (BODY-10)', () => { + const {sink, written} = collectingSink(); + void streamBody( + new ReadableStream({ + start: c => { + c.close(); + }, + }), + undefined, + 0, + ).writeTo(sink); + expect(written().length).toBe(0); + }); + + test('a contentLength below the -1 sentinel is rejected at construction (IO-3)', () => { + expect(() => streamBody(readableOf([1]), undefined, -2)).toThrow( + InvariantViolation, + ); + }); + + test('concurrent first writes: exactly one proceeds, the other rejects (BODY-3 race-safety)', async () => { + const body = streamBody(readableOf([1, 2, 3])); + const results = await Promise.allSettled([ + body.writeTo(collectingSink().sink), + body.writeTo(collectingSink().sink), + ]); + expect(results.filter(r => r.status === 'fulfilled').length).toBe(1); + expect(results.filter(r => r.status === 'rejected').length).toBe(1); + }); +}); diff --git a/packages/core/src/body/stream-body.ts b/packages/core/src/body/stream-body.ts new file mode 100644 index 0000000..b135958 --- /dev/null +++ b/packages/core/src/body/stream-body.ts @@ -0,0 +1,84 @@ +// SPDX-License-Identifier: MIT +// packages/core/src/body/stream-body.ts +import {EndOfStreamError} from '../io/errors.js'; +import {invariant} from '../invariant.js'; +import type {Body} from './body.js'; +import {ConsumedBodyError} from './errors.js'; + +/** + * A single-use body backed by a caller-supplied stream. + * + * @public + */ +export class StreamBody implements Body { + readonly kind = 'stream' as const; + readonly mediaType: string | undefined; + readonly contentLength: number; + readonly replayable = false; + readonly #stream: ReadableStream; + #consumed = false; + + constructor( + stream: ReadableStream, + mediaType?: string, + contentLength = -1, + ) { + invariant( + contentLength >= -1, + `contentLength must be >= -1 (-1 = unknown), got ${String(contentLength)}`, + ); // IO-3 + this.#stream = stream; + this.mediaType = mediaType; + this.contentLength = contentLength; + } + + async writeTo(sink: WritableStream): Promise { + if (this.#consumed) throw new ConsumedBodyError('stream'); + this.#consumed = true; // set before the first await -- BODY-3's race-safety guard + + if (this.contentLength < 0) { + await this.#stream.pipeTo(sink); + return; + } + await this.#writeExactly(sink, this.contentLength); + } + + /** HTTP-39/BODY-10: writes precisely `declared` bytes or raises naming delivered-of-declared. */ + async #writeExactly( + sink: WritableStream, + declared: number, + ): Promise { + const reader = this.#stream.getReader(); + const writer = sink.getWriter(); + let delivered = 0; + try { + for (;;) { + // Serial by necessity: each read depends on the previous one advancing the cursor. + const {done, value} = await reader.read(); + if (done) break; + delivered += value.length; + await writer.write(value); + } + } finally { + reader.releaseLock(); // BODY-8: release our handle, never cancel the caller's stream + await writer.close(); + } + + if (delivered !== declared) { + throw new EndOfStreamError(delivered, declared); + } + } +} + +/** + * Creates a single-use StreamBody (BODY-9). + * + * @public + */ +export function streamBody( + stream: ReadableStream, + mediaType?: string, + contentLength = -1, +): StreamBody { + return new StreamBody(stream, mediaType, contentLength); +} diff --git a/packages/core/src/body/typed-response.test.ts b/packages/core/src/body/typed-response.test.ts new file mode 100644 index 0000000..b953e60 --- /dev/null +++ b/packages/core/src/body/typed-response.test.ts @@ -0,0 +1,80 @@ +// SPDX-License-Identifier: MIT +// packages/core/src/body/typed-response.test.ts +// Exercises: HTTP-44 (raw fields without touching the body, parse-once memoized including failure), +// HTTP-45 (concurrent first callers serialized to one parse run) +import {describe, expect, test} from 'bun:test'; +import {Protocol} from '../http/protocol.js'; +import {Request} from '../http/request.js'; +import {Response} from '../http/response.js'; +import {Status} from '../http/status.js'; +import {TypedResponse} from './typed-response.js'; + +function readableOf(text: string): ReadableStream { + return new ReadableStream({ + start(c) { + c.enqueue(new TextEncoder().encode(text)); + c.close(); + }, + }); +} + +function baseResponse( + body: ReadableStream | null = null, +): Response { + return Response.newBuilder() + .request(Request.newBuilder().url('https://example.com').build()) + .protocol(Protocol.HTTP_1_1) + .status(Status.of(200)) + .reasonPhrase('OK') + .body(body) + .build(); +} + +describe('TypedResponse', () => { + test('exposes raw fields without touching the body (HTTP-44)', () => { + const response = baseResponse(readableOf('untouched')); + const typed = new TypedResponse(response, r => r.text()); + expect(typed.status.code).toBe(200); + expect(typed.headers).toBe(response.headers); + expect(typed.protocol).toBe('http/1.1'); + expect(typed.reason).toBe('OK'); + expect(typed.request).toBe(response.request); + expect(response.body?.locked).toBe(false); + }); + + test('parses on first value() call and memoizes the result', async () => { + let calls = 0; + const typed = new TypedResponse(baseResponse(readableOf('x')), () => { + calls += 1; + return Promise.resolve('parsed'); + }); + expect(await typed.value()).toBe('parsed'); + expect(await typed.value()).toBe('parsed'); + expect(calls).toBe(1); + }); + + test('memoizes a thrown failure -- every later call re-throws the same error, parse never re-runs', () => { + let calls = 0; + const failure = new Error('parse failed'); + const typed = new TypedResponse(baseResponse(readableOf('x')), () => { + calls += 1; + return Promise.reject(failure); + }); + expect(typed.value()).rejects.toBe(failure); + expect(typed.value()).rejects.toBe(failure); + expect(calls).toBe(1); + }); + + test('concurrent first callers share one in-flight parse (HTTP-45)', async () => { + let calls = 0; + const typed = new TypedResponse(baseResponse(readableOf('x')), async () => { + calls += 1; + await Promise.resolve(); + return 'value'; + }); + const [a, b] = await Promise.all([typed.value(), typed.value()]); + expect(a).toBe('value'); + expect(b).toBe('value'); + expect(calls).toBe(1); + }); +}); diff --git a/packages/core/src/body/typed-response.ts b/packages/core/src/body/typed-response.ts new file mode 100644 index 0000000..6fe366d --- /dev/null +++ b/packages/core/src/body/typed-response.ts @@ -0,0 +1,56 @@ +// SPDX-License-Identifier: MIT +// packages/core/src/body/typed-response.ts +import type {Request} from '../http/request.js'; +import type {Response} from '../http/response.js'; + +/** + * A typed view over an HTTP response (HTTP-44). Wraps an underlying raw Response and a parser function, + * materializing and parsing the response value lazily on the first call to `value()`. + * + * Deliberately does NOT expose the underlying `Response` itself (only its status/headers/protocol/ + * reason/request, per HTTP-44) -- doing so would let a caller read the single-use body directly, + * bypassing `value()`'s memoization and the HTTP-45 in-flight-promise serialization entirely. + * + * @public + */ +export class TypedResponse { + readonly #response: Response; + readonly #parse: (response: Response) => Promise; + #memoized: Promise | undefined; + + constructor(response: Response, parse: (response: Response) => Promise) { + this.#response = response; + this.#parse = parse; + } + + get status(): Response['status'] { + return this.#response.status; + } + + get headers(): Response['headers'] { + return this.#response.headers; + } + + get protocol(): string { + return this.#response.protocol.token; // lower-case token string (Protocol.token) + } + + get reason(): string | undefined { + return this.#response.reasonPhrase; + } + + /** The originating request (HTTP-44). Accessing raw fields never consumes the body. */ + get request(): Request { + return this.#response.request; + } + + /** + * Lazily parses and returns the typed value. Memoized: the parser function runs at most once, and + * subsequent calls return the same parsed value (or re-throw the same error) without re-parsing or + * re-reading the body (HTTP-44). Concurrent first callers share the single in-flight parse (HTTP-45). + */ + value(): Promise { + this.#memoized ??= this.#parse(this.#response); + return this.#memoized; + } +} diff --git a/packages/core/src/http/request.test.ts b/packages/core/src/http/request.test.ts index 568ff7c..2506d7e 100644 --- a/packages/core/src/http/request.test.ts +++ b/packages/core/src/http/request.test.ts @@ -5,6 +5,7 @@ // immutability) import {describe, expect, test} from 'bun:test'; import fc from 'fast-check'; +import {stringBody} from '../body/simple-bodies.js'; import {Request} from './request.js'; import {Headers} from './headers.js'; import { @@ -31,7 +32,7 @@ describe('method/body legality (HTTP-7)', () => { Request.newBuilder() .method(method) .url('https://example.com') - .body('x') + .body(stringBody('x')) .build(), ).toThrow(RequestBodyNotAllowedError); } @@ -42,7 +43,7 @@ describe('method/body legality (HTTP-7)', () => { Request.newBuilder() .method('POST') .url('https://example.com') - .body('x') + .body(stringBody('x')) .build(), ).not.toThrow(); }); @@ -51,21 +52,11 @@ describe('method/body legality (HTTP-7)', () => { const request = Request.newBuilder() .method('GET') .url('https://example.com') - .body('x') + .body(stringBody('x')) .body(undefined) .build(); expect(request.body).toBeUndefined(); }); - - test('a null body clears like undefined — HTTP-7 rejects only a non-null body', () => { - const request = Request.newBuilder() - .method('GET') - .url('https://example.com') - .body('x') - .body(null) - .build(); - expect(request.body).toBeUndefined(); - }); }); describe('method defaulting (HTTP-8)', () => { @@ -76,7 +67,10 @@ describe('method defaulting (HTTP-8)', () => { test('fails naming the missing method when a body is set with no method', () => { expect(() => - Request.newBuilder().url('https://example.com').body('x').build(), + Request.newBuilder() + .url('https://example.com') + .body(stringBody('x')) + .build(), ).toThrow('method is required'); }); }); diff --git a/packages/core/src/http/request.ts b/packages/core/src/http/request.ts index 2610f29..9105de4 100644 --- a/packages/core/src/http/request.ts +++ b/packages/core/src/http/request.ts @@ -1,5 +1,6 @@ // SPDX-License-Identifier: MIT // packages/core/src/http/request.ts +import type {Body} from '../body/body.js'; import type {Builder} from './builder.js'; import {requireField} from './builder.js'; import {UrlConstructionError, RequestBodyNotAllowedError} from './errors.js'; @@ -21,7 +22,7 @@ let createRequest: ( method: Method, url: URL, headers: Headers, - body: unknown, + body: Body | undefined, ) => Request; /** @@ -39,7 +40,7 @@ let createRequest: ( * const request = Request.newBuilder() * .method('POST') * .url('https://example.com/items') - * .body('payload') + * .body(stringBody('payload')) * .build(); * ``` * @@ -49,14 +50,14 @@ export class Request { readonly #method: Method; readonly #url: URL; readonly #headers: Headers; - readonly #body: unknown; + readonly #body: Body | undefined; // eslint-disable-next-line max-params -- private, builder-internal; field count fixed by the wire model (HTTP-6) private constructor( method: Method, url: URL, headers: Headers, - body: unknown, + body: Body | undefined, ) { this.#method = method; this.#url = url; @@ -113,13 +114,8 @@ export class Request { return this.#headers; } - /** - * The request body, or `undefined` when absent. - * - * Typed `unknown` on purpose: this phase only needs presence or absence to enforce HTTP-7/8. The - * body lifecycle — streaming, replayability, charset — is owned by a later phase. - */ - get body(): unknown { + /** The request body, or `undefined` when absent. */ + get body(): Body | undefined { return this.#body; } @@ -128,8 +124,7 @@ export class Request { * * The URL is compared by textual external form only, never by resolving the host — native URL * equality on some platforms resolves DNS, which blocks and is wrong for virtual hosts sharing an - * IP (HTTP-46). The body is compared by reference for now; value equality arrives with the real - * body model in a later phase. + * IP (HTTP-46). * * @param other - the request to compare against. * @returns `true` when every compared facet is equal. @@ -153,7 +148,7 @@ export class RequestBuilder implements Builder { #method: Method | undefined; #url: URL | undefined; #headers: Headers = Headers.newBuilder().build(); - #body: unknown; + #body: Body | undefined; /** * Sets the request method. @@ -194,12 +189,11 @@ export class RequestBuilder implements Builder { /** * Sets or clears the request body. * - * @param body - the body, or `null`/`undefined` to clear it. `null` normalizes to `undefined`: - * HTTP-7 rejects only a *non-null* body, so passing `null` clears exactly like `undefined`. + * @param body - the body, or `undefined` to clear it. * @returns this builder, for chaining. */ - body(body: unknown): this { - this.#body = body ?? undefined; + body(body: Body | undefined): this { + this.#body = body; return this; } diff --git a/packages/core/src/http/response.test.ts b/packages/core/src/http/response.test.ts index d3d8b46..ec8533f 100644 --- a/packages/core/src/http/response.test.ts +++ b/packages/core/src/http/response.test.ts @@ -1,17 +1,41 @@ -// SPDX-License-Identifier: MIT // packages/core/src/http/response.test.ts -// Exercises: HTTP-6 (response's required fields: request, protocol, status) +// Exercises: HTTP-6 (required fields), HTTP-41/BODY-14 (single-use body, same reference on repeat +// access), HTTP-41/BODY-15, HTTP-43 (idempotent close, releases the connection whether or not the body +// was read), HTTP-41/BODY-16 (convenience readers close in a finally-style guarantee), HTTP-42 +// (charset default and UTF-8 fallback) import {describe, expect, test} from 'bun:test'; -import {Response} from './response.js'; -import {Request} from './request.js'; +import {Headers} from './headers.js'; import {Protocol} from './protocol.js'; +import {Request} from './request.js'; +import {Response} from './response.js'; import {Status} from './status.js'; -import {Headers} from './headers.js'; function baseRequest(): Request { return Request.newBuilder().url('https://example.com').build(); } +function readableOf(text: string): ReadableStream { + return new ReadableStream({ + start(controller) { + controller.enqueue(new TextEncoder().encode(text)); + controller.close(); + }, + }); +} + +function baseResponse( + body: ReadableStream | null = null, + headers: Headers = Headers.newBuilder().build(), +): Response { + return Response.newBuilder() + .request(baseRequest()) + .protocol(Protocol.HTTP_1_1) + .status(Status.of(200)) + .headers(headers) + .body(body) + .build(); +} + describe('required fields', () => { test('throws naming request when missing', () => { expect(() => @@ -42,60 +66,115 @@ describe('required fields', () => { }); describe('construction', () => { - test('carries the originating request, protocol, status, headers, and an optional reason phrase/body', () => { + test('carries the originating request, protocol, status, headers, and an optional reason phrase', () => { const request = baseRequest(); const response = Response.newBuilder() .request(request) .protocol(Protocol.HTTP_1_1) .status(Status.of(200)) .reasonPhrase('OK') - .body('payload') .build(); expect(response.request.equals(request)).toBe(true); expect(response.protocol.equals(Protocol.HTTP_1_1)).toBe(true); expect(response.status.equals(Status.of(200))).toBe(true); expect(response.reasonPhrase).toBe('OK'); - expect(response.body).toBe('payload'); }); - test('reason phrase and body are optional', () => { + test('reason phrase is optional, body defaults to null', () => { const response = Response.newBuilder() .request(baseRequest()) .protocol(Protocol.HTTP_1_1) .status(Status.of(204)) .build(); expect(response.reasonPhrase).toBeUndefined(); - expect(response.body).toBeUndefined(); + expect(response.body).toBeNull(); }); }); describe('newBuilder derivation', () => { test('deriving a builder and rebuilding does not affect the original', () => { - const original = Response.newBuilder() - .request(baseRequest()) - .protocol(Protocol.HTTP_1_1) - .status(Status.of(200)) - .build(); + const original = baseResponse(); original.newBuilder().status(Status.of(500)).build(); expect(original.status.code).toBe(200); }); }); -describe('headers (HTTP-6)', () => { - test('defaults to empty headers and carries what the builder was given', () => { - const bare = Response.newBuilder() - .request(baseRequest()) - .protocol(Protocol.HTTP_1_1) - .status(Status.of(204)) +describe('body (HTTP-41/BODY-14)', () => { + test('repeated access returns the same reference, not a replay', () => { + const stream = readableOf('x'); + const response = baseResponse(stream); + expect(response.body).toBe(stream); + expect(response.body).toBe(response.body); + }); +}); + +describe('bytes/text (BODY-16, HTTP-42)', () => { + test('bytes() reads the whole body', async () => { + const response = baseResponse(readableOf('hello')); + expect(new TextDecoder().decode(await response.bytes())).toBe('hello'); + }); + + test('bytes() on a null body returns empty', async () => { + expect(await baseResponse(null).bytes()).toEqual(new Uint8Array(0)); + }); + + test('text() defaults to UTF-8 when no content-type is declared', async () => { + expect(await baseResponse(readableOf('héllo')).text()).toBe('héllo'); + }); + + test('text() uses the declared charset', async () => { + const bytes = Uint8Array.from([0x68, 0xe9]); // "hé" in ISO-8859-1 + const stream = new ReadableStream({ + start: c => { + c.enqueue(bytes); + c.close(); + }, + }); + const headers = Headers.newBuilder() + .add('content-type', 'text/plain;charset=iso-8859-1') .build(); - expect(bare.headers.names()).toEqual([]); + expect(await baseResponse(stream, headers).text()).toBe('hé'); + }); - const response = bare - .newBuilder() - .headers(Headers.newBuilder().add('Content-Type', 'text/plain').build()) + test('text() falls back to UTF-8 when the declared charset is unrecognized', async () => { + const headers = Headers.newBuilder() + .add('content-type', 'text/plain;charset=bogus-charset') .build(); - expect(response.headers.get('content-type')).toBe('text/plain'); - expect(bare.headers.has('content-type')).toBe(false); + expect(await baseResponse(readableOf('ok'), headers).text()).toBe('ok'); + }); + + test('bytes() closes the response even though the read succeeded', async () => { + const response = baseResponse(readableOf('x')); + await response.bytes(); + expect(response.close()).resolves.toBeUndefined(); // idempotent, already closed + }); +}); + +describe('close (HTTP-41/BODY-15, HTTP-43)', () => { + test('is idempotent', async () => { + const response = baseResponse(readableOf('x')); + await response.close(); + await response.close(); + }); + + test('releases the connection even when the body was never read', async () => { + let cancelled = false; + const stream = new ReadableStream({ + start(controller) { + controller.enqueue(Uint8Array.from([1])); + }, + cancel() { + cancelled = true; + }, + }); + await baseResponse(stream).close(); + expect(cancelled).toBe(true); + }); + + test('[Symbol.asyncDispose] delegates to close()', async () => { + const response = baseResponse(readableOf('x')); + await response[Symbol.asyncDispose](); + expect(response.close()).resolves.toBeUndefined(); }); }); diff --git a/packages/core/src/http/response.ts b/packages/core/src/http/response.ts index 8c9c68e..ebe6de4 100644 --- a/packages/core/src/http/response.ts +++ b/packages/core/src/http/response.ts @@ -2,27 +2,14 @@ // packages/core/src/http/response.ts import type {Builder} from './builder.js'; import {requireField} from './builder.js'; -import type {Request} from './request.js'; +import {Headers} from './headers.js'; +import {MediaType} from './media-type.js'; import type {Protocol} from './protocol.js'; +import type {Request} from './request.js'; import type {Status} from './status.js'; -import {Headers} from './headers.js'; - -// eslint-disable-next-line max-params -- private, builder-internal plumbing; field count fixed by HTTP-6 -let createResponse: ( - request: Request, - protocol: Protocol, - status: Status, - reasonPhrase: string | undefined, - headers: Headers, - body: unknown, -) => Response; /** - * An immutable HTTP response: the originating request, the negotiated protocol, the status, an - * optional reason phrase, headers, and an optional body (HTTP-6). - * - * Status-range classification is reached through {@link Response.status} — `response.status.isSuccess`, - * `response.status.isError`, and the rest (HTTP-11). + * An HTTP response model (HTTP-6). * * @public */ @@ -32,16 +19,19 @@ export class Response { readonly #status: Status; readonly #reasonPhrase: string | undefined; readonly #headers: Headers; - readonly #body: unknown; + readonly #body: ReadableStream | null; + // Not `readonly` -- Object.freeze(this) below only freezes normal properties, never #private fields, + // so this can still track close state after construction (BODY-15, HTTP-43). + #closed = false; // eslint-disable-next-line max-params -- private, builder-internal; field count fixed by the wire model (HTTP-6) - private constructor( + constructor( request: Request, protocol: Protocol, status: Status, reasonPhrase: string | undefined, headers: Headers, - body: unknown, + body: ReadableStream | null, ) { this.#request = request; this.#protocol = protocol; @@ -52,30 +42,10 @@ export class Response { Object.freeze(this); } - static { - // eslint-disable-next-line max-params -- private, builder-internal plumbing; field count fixed by HTTP-6 - createResponse = (request, protocol, status, reasonPhrase, headers, body) => - new Response(request, protocol, status, reasonPhrase, headers, body); - } - - /** - * Starts an empty builder. - * - * @returns a fresh {@link ResponseBuilder}. - */ static newBuilder(): ResponseBuilder { return new ResponseBuilder(); } - /** - * Derives a builder pre-populated from this instance (HTTP-3). - * - * Every field it carries is itself immutable — `Request` freezes and defensively clones its URL, - * and `Headers`, `Status`, and `Protocol` are frozen values — so sharing them cannot leak - * mutability back into either instance. - * - * @returns a {@link ResponseBuilder} holding this response's state. - */ newBuilder(): ResponseBuilder { return new ResponseBuilder() .request(this.#request) @@ -86,42 +56,104 @@ export class Response { .body(this.#body); } - /** The request this response was produced for. */ get request(): Request { return this.#request; } - /** The negotiated protocol version. */ get protocol(): Protocol { return this.#protocol; } - /** The response status, which also carries the range classification (HTTP-11). */ get status(): Status { return this.#status; } - /** The reason phrase as sent, or `undefined` when the transport supplied none. */ get reasonPhrase(): string | undefined { return this.#reasonPhrase; } - /** The response headers — never null, possibly empty. */ get headers(): Headers { return this.#headers; } - /** - * The response body, or `undefined` when absent. Typed `unknown` until the body lifecycle lands - * in a later phase. - */ - get body(): unknown { + /** Single-use (BODY-14) -- the same reference every call, never a replay. */ + get body(): ReadableStream | null { return this.#body; } + + /** Reads the whole body as bytes, closing the response whether or not the read succeeds (BODY-16). */ + async bytes(): Promise { + if (this.#body === null) { + await this.close(); + return new Uint8Array(0); + } + const reader = this.#body.getReader(); + const chunks: Uint8Array[] = []; + let total = 0; + try { + for (;;) { + // Serial by necessity: each read depends on the previous one advancing the cursor. + const {done, value} = await reader.read(); + if (done) break; + chunks.push(value); + total += value.length; + } + } finally { + // MUST precede close(): ReadableStream.cancel() rejects with TypeError on a locked stream, and + // reading to done does NOT release the lock. Without this the finally replaces the read value + // with a TypeError and bytes()/text() never succeed. + reader.releaseLock(); + await this.close(); + } + const result = new Uint8Array(total); + let offset = 0; + for (const chunk of chunks) { + result.set(chunk, offset); + offset += chunk.length; + } + return result; + } + + /** Reads the whole body as text, defaulting to the media type's charset then UTF-8 (HTTP-42). */ + async text(): Promise { + const bytes = await this.bytes(); + try { + return new TextDecoder(this.#charset()).decode(bytes); + } catch { + return new TextDecoder('utf-8').decode(bytes); // HTTP-42: unrecognized charset also falls back + } + } + + #charset(): string { + const contentType = this.#headers.get('content-type'); + if (contentType === undefined) return 'utf-8'; + try { + return MediaType.parse(contentType).charset ?? 'utf-8'; + } catch { + return 'utf-8'; + } + } + + /** Idempotent; releases the underlying connection whether or not the body was read (BODY-15, HTTP-43). */ + async close(): Promise { + if (this.#closed) return; + this.#closed = true; + if (this.#body === null) return; + // BODY-15 forbids assuming the body was read, so an external consumer may still hold the reader + // lock -- cancel() rejects with TypeError in that case. Swallow only that: the caller asked to + // release the connection, and the lock holder's own close will finish the job. + await this.#body.cancel().catch((error: unknown) => { + if (!(error instanceof TypeError)) throw error; + }); + } + + async [Symbol.asyncDispose](): Promise { + await this.close(); + } } /** - * Accumulates response state and produces an immutable {@link Response}. + * Builder for {@link Response}. * * @public */ @@ -131,86 +163,43 @@ export class ResponseBuilder implements Builder { #status: Status | undefined; #reasonPhrase: string | undefined; #headers: Headers = Headers.newBuilder().build(); - #body: unknown; - - /** - * Sets the originating request. Required. - * - * @param request - the request this response answers. - * @returns this builder, for chaining. - */ + #body: ReadableStream | null = null; + request(request: Request): this { this.#request = request; return this; } - /** - * Sets the negotiated protocol. Required. - * - * @param protocol - the protocol the exchange used. - * @returns this builder, for chaining. - */ protocol(protocol: Protocol): this { this.#protocol = protocol; return this; } - /** - * Sets the response status. Required. - * - * @param status - the status received. - * @returns this builder, for chaining. - */ status(status: Status): this { this.#status = status; return this; } - /** - * Sets the reason phrase. - * - * @param reasonPhrase - the phrase as sent, or `undefined` when there was none. - * @returns this builder, for chaining. - */ reasonPhrase(reasonPhrase: string | undefined): this { this.#reasonPhrase = reasonPhrase; return this; } - /** - * Sets the response headers, replacing whatever was set before. - * - * @param headers - the headers received; already immutable, so held by reference. - * @returns this builder, for chaining. - */ headers(headers: Headers): this { this.#headers = headers; return this; } - /** - * Sets the response body. - * - * @param body - the body, or `undefined` when absent. - * @returns this builder, for chaining. - */ - body(body: unknown): this { + body(body: ReadableStream | null): this { this.#body = body; return this; } - /** - * Validates the required fields and constructs the response. - * - * @returns the frozen response. - * @throws {@link RequiredFieldError} when the request, protocol, or status was never set, - * naming whichever is missing (HTTP-4). - */ build(): Response { const request = requireField(this.#request, 'request'); const protocol = requireField(this.#protocol, 'protocol'); const status = requireField(this.#status, 'status'); - return createResponse( + return new Response( request, protocol, status, diff --git a/packages/core/src/index.ts b/packages/core/src/index.ts index 9ae93a0..7039ba9 100644 --- a/packages/core/src/index.ts +++ b/packages/core/src/index.ts @@ -24,3 +24,36 @@ export { } from './seams/transport.js'; export type {OperationDescriptor} from './seams/operation.js'; export {buildRequest, OperationAssemblyError} from './seams/operation.js'; + +// Deliberately NOT `export * from './body/index.js';` — that barrel also carries withRequestLogging/ +// withResponseLogging, internal until Phase 7 supplies a Logger to drive them. Naming each public export +// here instead keeps that boundary enforced at the barrel, not by convention. +// The concrete body classes are exported as TYPES ONLY. Exporting the class as a value publishes +// `new ByteArrayBody(...)` as a field-wise constructor, which HTTP-2 forbids ("constructible only +// through their builder or dedicated factory") and which duplicates the factory functions for no +// stated need (NFR-3). Callers construct via the factories and annotate with the types. +export type {Body} from './body/body.js'; +export { + ConsumedBodyError, + isBodyError, + MultipartBoundaryError, +} from './body/errors.js'; +export {HttpStatusError, toHttpError} from './body/http-status-error.js'; +export {materialize} from './body/materialize.js'; +export { + multipartBody, + type MultipartBody, + MultipartBodyBuilder, + type MultipartPart, +} from './body/multipart-body.js'; +export { + byteArrayBody, + type ByteArrayBody, + formUrlEncodedBody, + type FormUrlEncodedBody, + type FormUrlEncodedInput, + stringBody, + type StringBody, +} from './body/simple-bodies.js'; +export {streamBody, type StreamBody} from './body/stream-body.js'; +export {TypedResponse} from './body/typed-response.js'; diff --git a/packages/core/src/io/errors.test.ts b/packages/core/src/io/errors.test.ts index 4038132..c7f16bb 100644 --- a/packages/core/src/io/errors.test.ts +++ b/packages/core/src/io/errors.test.ts @@ -9,6 +9,7 @@ import { ClosedResourceError, EndOfStreamError, IoError, + isIoError, SourceContractViolationError, } from './errors.js'; @@ -17,13 +18,16 @@ describe('IoError tree', () => { expect(new IoError('boom')).toBeInstanceOf(DexpaceError); }); - test('every leaf descends from IoError', () => { - expect(new EndOfStreamError(3, 8)).toBeInstanceOf(IoError); + test('every leaf descends from DexpaceError directly, not through IoError (Phase 3b retrofit)', () => { + expect(new EndOfStreamError(3, 8)).toBeInstanceOf(DexpaceError); + expect(new EndOfStreamError(3, 8)).not.toBeInstanceOf(IoError); expect(new SourceContractViolationError('zero read')).toBeInstanceOf( - IoError, + DexpaceError, ); - expect(new ClosedResourceError('BufferedSource')).toBeInstanceOf(IoError); - expect(new AllocationLimitError(9, 8)).toBeInstanceOf(IoError); + expect(new ClosedResourceError('BufferedSource')).toBeInstanceOf( + DexpaceError, + ); + expect(new AllocationLimitError(9, 8)).toBeInstanceOf(DexpaceError); }); test('each error sets name from its own constructor', () => { @@ -60,19 +64,13 @@ describe('IoError tree', () => { expect(new AllocationLimitError(5, 4, {cause}).cause).toBe(cause); }); - test('EndOfStreamError chains a cause', () => { - const cause = new Error('underlying read failure'); - expect(new EndOfStreamError(1, 2, {cause}).cause).toBe(cause); - }); - - test('ClosedResourceError chains a cause', () => { - const cause = new Error('already closed'); - expect(new ClosedResourceError('ByteQueue', {cause}).cause).toBe(cause); - }); - - test('SourceContractViolationError carries its message and descends from IoError', () => { - const error = new SourceContractViolationError('returned zero bytes'); - expect(error.message).toBe('returned zero bytes'); - expect(error.name).toBe('SourceContractViolationError'); + test('isIoError groups every leaf, including bare IoError, without a class tier', () => { + expect(isIoError(new IoError('x'))).toBe(true); + expect(isIoError(new EndOfStreamError(1, 2))).toBe(true); + expect(isIoError(new SourceContractViolationError('x'))).toBe(true); + expect(isIoError(new ClosedResourceError('x'))).toBe(true); + expect(isIoError(new AllocationLimitError(1, 2))).toBe(true); + expect(isIoError(new DexpaceError('other'))).toBe(false); + expect(isIoError(new Error('plain'))).toBe(false); }); }); diff --git a/packages/core/src/io/errors.ts b/packages/core/src/io/errors.ts index bc53c0c..abe1557 100644 --- a/packages/core/src/io/errors.ts +++ b/packages/core/src/io/errors.ts @@ -26,7 +26,7 @@ export class IoError extends DexpaceError { * * @internal */ -export class EndOfStreamError extends IoError { +export class EndOfStreamError extends DexpaceError { readonly delivered: number; readonly requested: number; @@ -46,7 +46,7 @@ export class EndOfStreamError extends IoError { * * @internal */ -export class SourceContractViolationError extends IoError { +export class SourceContractViolationError extends DexpaceError { // See IoError's constructor above: keeps this bodiless subclass registered for bun's // function coverage. // eslint-disable-next-line @typescript-eslint/no-useless-constructor -- see comment above @@ -62,7 +62,7 @@ export class SourceContractViolationError extends IoError { * * @internal */ -export class ClosedResourceError extends IoError { +export class ClosedResourceError extends DexpaceError { readonly resource: string; constructor(resource: string, options?: ErrorOptions) { @@ -77,7 +77,7 @@ export class ClosedResourceError extends IoError { * * @internal */ -export class AllocationLimitError extends IoError { +export class AllocationLimitError extends DexpaceError { readonly requested: number; readonly limit: number; @@ -90,3 +90,28 @@ export class AllocationLimitError extends IoError { this.limit = limit; } } + +/** + * Groups every leaf in this file, including bare `IoError`, without reintroducing a class tier between + * them and `DexpaceError` — the corpus caps custom error hierarchies at two levels. Retrofits Phase 3a's + * shape, where the four leaves extended `IoError` (a 3-tier chain the checkpoint's `DomainModelError` fix + * should also have caught and didn't). + * + * @internal + */ +export function isIoError( + error: unknown, +): error is + | IoError + | EndOfStreamError + | SourceContractViolationError + | ClosedResourceError + | AllocationLimitError { + return ( + error instanceof IoError || + error instanceof EndOfStreamError || + error instanceof SourceContractViolationError || + error instanceof ClosedResourceError || + error instanceof AllocationLimitError + ); +} diff --git a/packages/core/src/io/index.ts b/packages/core/src/io/index.ts index 8fbdb99..7910892 100644 --- a/packages/core/src/io/index.ts +++ b/packages/core/src/io/index.ts @@ -13,6 +13,7 @@ export { ClosedResourceError, EndOfStreamError, IoError, + isIoError, SourceContractViolationError, } from './errors.js'; export { diff --git a/packages/core/src/seams/operation.test.ts b/packages/core/src/seams/operation.test.ts index d166bc1..0217b72 100644 --- a/packages/core/src/seams/operation.test.ts +++ b/packages/core/src/seams/operation.test.ts @@ -98,17 +98,20 @@ describe('SEAM-27: base-URL composition rules', () => { }); }); +import {stringBody} from '../body/simple-bodies.js'; + describe('operation headers and body projections are threaded through', () => { test('supplied headers and body appear on the built request', () => { const headers = Headers.newBuilder().add('X-Trace', 'abc').build(); + const body = stringBody('Fido'); const request = buildRequest('https://host', { method: 'POST', pathTemplate: '/pets', headers, - body: {name: 'Fido'}, + body, }); expect(request.headers.get('x-trace')).toBe('abc'); - expect(request.body).toEqual({name: 'Fido'}); + expect(request.body).toBe(body); }); }); diff --git a/packages/core/src/seams/operation.ts b/packages/core/src/seams/operation.ts index cb05c5a..260c36e 100644 --- a/packages/core/src/seams/operation.ts +++ b/packages/core/src/seams/operation.ts @@ -1,5 +1,6 @@ // SPDX-License-Identifier: MIT // packages/core/src/seams/operation.ts +import type {Body} from '../body/body.js'; import {Request} from '../http/request.js'; import type {Headers} from '../http/headers.js'; import type {QueryParams} from '../http/query-params.js'; @@ -68,7 +69,7 @@ export interface OperationDescriptor { * The operation's body. Carried, not encoded — serialization is a separate seam's concern * (SEAM-26). Defaults to absent. */ - readonly body?: unknown; + readonly body?: Body | undefined; } const PATH_PARAM_RE = /\{([^{}]+)\}/g; From 2ad5b74f7d43010b4107bedf68a29f848ab055ac Mon Sep 17 00:00:00 2001 From: Mohammad Wahbeh Date: Mon, 24 Aug 2026 22:16:27 +0300 Subject: [PATCH 2/3] fix(core): resolve body-lifecycle review findings (BODY-3..37, HTTP-26/39/42/43/44/51). --- .changeset/body-lifecycle-review-fixes.md | 24 ++++ ...026-07-25-phase3b-body-lifecycle-design.md | 3 +- packages/core/etc/core.api.md | 15 ++- packages/core/src/body/body.ts | 8 ++ packages/core/src/body/errors.ts | 25 +++- .../core/src/body/http-status-error.test.ts | 61 ++++++++- packages/core/src/body/http-status-error.ts | 23 +++- packages/core/src/body/index.ts | 2 + packages/core/src/body/materialize.ts | 3 + packages/core/src/body/media-type-safety.ts | 38 ++++++ packages/core/src/body/multipart-body.test.ts | 57 ++++++++- packages/core/src/body/multipart-body.ts | 26 +++- .../src/body/request-body-logging.test.ts | 41 +++++- .../core/src/body/request-body-logging.ts | 6 +- .../src/body/response-body-logging.test.ts | 121 +++++++++++++++++- .../core/src/body/response-body-logging.ts | 40 +++++- packages/core/src/body/simple-bodies.test.ts | 93 +++++++++++++- packages/core/src/body/simple-bodies.ts | 102 +++++++++------ packages/core/src/body/stream-body.test.ts | 80 +++++++++++- packages/core/src/body/stream-body.ts | 43 +++++-- packages/core/src/body/typed-response.test.ts | 35 ++++- packages/core/src/body/typed-response.ts | 9 +- packages/core/src/body/write-body.ts | 31 +++++ packages/core/src/http/charset.ts | 28 ++++ packages/core/src/http/response.test.ts | 42 +++++- packages/core/src/http/response.ts | 31 ++--- packages/core/src/index.ts | 2 + 27 files changed, 876 insertions(+), 113 deletions(-) create mode 100644 .changeset/body-lifecycle-review-fixes.md create mode 100644 packages/core/src/body/media-type-safety.ts create mode 100644 packages/core/src/body/write-body.ts create mode 100644 packages/core/src/http/charset.ts diff --git a/.changeset/body-lifecycle-review-fixes.md b/.changeset/body-lifecycle-review-fixes.md new file mode 100644 index 0000000..3d3b291 --- /dev/null +++ b/.changeset/body-lifecycle-review-fixes.md @@ -0,0 +1,24 @@ +--- +"@dexpace/core": minor +--- + +Body lifecycle review fixes. + +Security: + +- Body media types are validated as header-safe at construction (`byteArrayBody`, `stringBody`, `streamBody`, and every part rendered into a multipart body), using the same predicate as outbound header-value validation (HTTP-26). A CR/LF in a media type was previously interpolated verbatim into a multipart part header, which allowed arbitrary header injection, arbitrary part content, and a forged closing boundary while the declared content length still matched the corrupted bytes (HTTP-51). +- `StreamBody.writeTo` now refuses a chunk that would carry the body past its declared `contentLength` *before* writing it, and aborts the sink rather than closing it on any length mismatch. Overrun bytes previously reached the sink and were reported only afterwards, leaving them on the socket behind a stamped `Content-Length` (HTTP-39/BODY-10). + +Correctness: + +- A body write failure is no longer masked by the close that follows it. All five `Body` implementations share one writer scope that aborts on failure and never lets a close error replace the primary one (RECOV-12), so retry classification still sees the I/O failure in the cause chain (RETRY-2). +- `TypedResponse.value()` memoizes a parser that throws synchronously; it previously re-ran the handler and re-read the single-use body (HTTP-44). +- `HttpStatusError.preview()` decodes with the charset declared by the response media type, falling back to UTF-8, and never throws a `RangeError` on an unknown label (HTTP-42). +- `withRequestLogging(...).materialize()` gives the new wrapper its own tap buffer instead of aliasing the original's, so one wrapper's write can no longer rewrite another's captured preview (BODY-21). +- `withResponseLogging` treats a zero-length delegate chunk as a stream-contract violation, matching `RetentionWindow` under IO-17 (BODY-25), and `snapshot()` now starts the lazy drain the way `read()` does (BODY-22). +- `Response.close()` marks the response closed only once the release actually succeeds, memoized so concurrent closers share one cancel — the shape `BufferedSink.close()` already uses (BODY-15, HTTP-43). + +Public API: + +- New `FormBodyValidationError`, reported by `isBodyError`. A form field whose value cannot be rendered is now raised instead of silently dropped from the body. +- `FormUrlEncodedInput` accepts the new `FormUrlEncodedValue` (`string | number | boolean | bigint | null`); primitives render rather than vanish (HTTP-38/BODY-35). diff --git a/docs/superpowers/specs/2026-07-25-phase3b-body-lifecycle-design.md b/docs/superpowers/specs/2026-07-25-phase3b-body-lifecycle-design.md index 333f673..50715fc 100644 --- a/docs/superpowers/specs/2026-07-25-phase3b-body-lifecycle-design.md +++ b/docs/superpowers/specs/2026-07-25-phase3b-body-lifecycle-design.md @@ -52,7 +52,7 @@ Everything else in `§6` ships in this phase. | BODY-17, BODY-18, BODY-19, BODY-21, BODY-37 | MUST | `withRequestLogging` tee decorator over `Body` — a self-contained tee reusing only `ByteQueue`, **not** Phase 3a's `TeeSink` class (whose `ByteQueue`-and-count signature does not compose with `writeTo`'s chunk-shaped sink; see the section below) | | BODY-20 | SHOULD | Partial-failure snapshot returns bytes mirrored up to the failure | | BODY-22, BODY-23, BODY-24, BODY-27, BODY-28 | MUST | Response-body logging wrapper, two regimes (fits-cap capture vs. exceeds-cap prefix+tail), shared close-once guard | -| BODY-25 | MUST | **Structurally inapplicable on Node** — the wrapper reads through a `ReadableStreamDefaultReader`, which has no requested-count parameter, so "returns zero for a positive requested count" has no analog; a zero-length chunk is not an EOS signal and is captured as-is, with EOS signalled only by `{done: true}`. Ledgered | +| BODY-25 | MUST | Implemented: a zero-length delegate chunk raises `SourceContractViolationError`, matching `RetentionWindow` under the identically-worded `IO-17`. `ReadableStreamDefaultReader.read()` carries no requested count, so the clause has no *literal* analog — but a response body reaches both this tee and `BufferedSource`, and the tolerant reading made the same upstream succeed or fail depending only on which wrapper it passed through. Ledger entry withdrawn (review finding, 2026-08-24) | | BODY-26 | MUST | `LoggedResponseBody.error(): Error \| null` — the drain failure is cached in the wrapper's closure; `read()` re-throws it on every call, `snapshot()` returns the partial bytes without throwing, and `error()` surfaces it **without triggering a drain** | | BODY-29 | SHOULD | `LoggedResponseBody.contentLength` — the captured size in the fits-cap regime, the delegate's declared length otherwise (the capture is only a bounded prefix) | | HTTP-52 / BODY-30, BODY-31 | MUST | `toHttpError(response)` — 1 MiB fixed cap, 4xx/5xx only, buffering inside the response's own close-guaranteeing scope | @@ -398,7 +398,6 @@ Phase 1's `unknown` placeholder; `Response` gains `text()`/`bytes()`/`close()`), | Both logging tees are new, self-contained implementations, not built on Phase 3a's `TeeSink`/`BufferedSource` | none — forced by the `writeTo` decision above | `TeeSink`/`BufferedSource`/`BufferedSink` are reader/writer-bound with `ByteQueue`-and-count-shaped signatures; `Body.writeTo`'s chunk-shaped `WritableStream` doesn't compose with them without rewriting Phase 3a's frozen surface. Only `ByteQueue` (pure in-memory, unbound to a stream shape) is reused | | Phase 3a's `IoError` tier flattened in this phase, not in 3a itself | phase-boundary discipline (each phase's own frozen surface) | The checkpoint's `§5.2` fix for `DomainModelError` missed the identically-shaped `IoError` tier; carrying the inconsistency forward into a fourth phase was judged worse than a scoped retrofit here | | Logging tees and `toHttpError`'s preview machinery shipped `@internal`, unwired to any `Logger` | none — matches Phase 2's `Serde` precedent | No `Logger`/config surface exists until Phase 7 | -| `BODY-25`'s zero-byte-read-for-a-positive-count clause not implemented | `BODY-25` (MUST) | Structurally inapplicable: `ReadableStreamDefaultReader.read()` takes no requested count, so the failure mode has no analog. EOF is signalled only by `{done: true}`, which is what the drain loop keys on, so the silent truncation `BODY-25` guards against cannot arise | | `BODY-34`'s shared preview cap covers the two logging tees only, not `toHttpError` | `BODY-34` (MUST), read literally as "all three" | `HTTP-52` *fixes* the error-body cap at 1 MiB, so it cannot also be the configurable shared value. The two capture sites `BODY-34` actually names — request-side tee and response-side drain — do share one cap | | Concrete `Body` classes exported from the public barrel as types only, never as values | none — required by `HTTP-2` | Exporting the class as a value publishes a field-wise constructor, which `HTTP-2` forbids; the factory functions are the sanctioned construction path and the classes remain usable as type annotations | diff --git a/packages/core/etc/core.api.md b/packages/core/etc/core.api.md index 92c9da4..8614a45 100644 --- a/packages/core/etc/core.api.md +++ b/packages/core/etc/core.api.md @@ -14,7 +14,6 @@ interface Body_2 { readonly mediaType: string | undefined; // (undocumented) readonly replayable: boolean; - // (undocumented) writeTo(sink: WritableStream): Promise; } export { Body_2 as Body } @@ -83,6 +82,13 @@ export class ETag { export class EtagParseError extends DomainModelError { } +// @public +export class FormBodyValidationError extends DexpaceError { + constructor(field: string, value: unknown, options?: ErrorOptions); + // (undocumented) + readonly field: string; +} + // @public export class FormUrlEncodedBody implements Body_2 { constructor(input: FormUrlEncodedInput); @@ -104,7 +110,10 @@ export class FormUrlEncodedBody implements Body_2 { export function formUrlEncodedBody(input: FormUrlEncodedInput): FormUrlEncodedBody; // @public -export type FormUrlEncodedInput = QueryParams | ReadonlyMap | Record | readonly (readonly [string, string])[]; +export type FormUrlEncodedInput = QueryParams | ReadonlyMap | Record | readonly (readonly [string, FormUrlEncodedValue])[]; + +// @public +export type FormUrlEncodedValue = string | number | boolean | bigint | null; // @public export class HeaderName { @@ -170,7 +179,7 @@ export class HttpStatusError extends DexpaceError { } // @public -export function isBodyError(error: unknown): error is ConsumedBodyError | MultipartBoundaryError; +export function isBodyError(error: unknown): error is ConsumedBodyError | MultipartBoundaryError | FormBodyValidationError; // @public export function isTimeoutSignal(signal: AbortSignal): boolean; diff --git a/packages/core/src/body/body.ts b/packages/core/src/body/body.ts index f701ab4..9b6d5ec 100644 --- a/packages/core/src/body/body.ts +++ b/packages/core/src/body/body.ts @@ -12,5 +12,13 @@ export interface Body { readonly mediaType: string | undefined; readonly contentLength: number; readonly replayable: boolean; + /** + * Writes the body once into `sink`, closing it on success and aborting it on failure so a partially + * written body is never signalled to the transport as a complete one. + * + * @throws ConsumedBodyError when a single-use body is written a second time (BODY-3). + * @throws EndOfStreamError when a stream body's byte count disagrees with its declared + * `contentLength` (HTTP-39/BODY-10). + */ writeTo(sink: WritableStream): Promise; } diff --git a/packages/core/src/body/errors.ts b/packages/core/src/body/errors.ts index ce36083..95e05bf 100644 --- a/packages/core/src/body/errors.ts +++ b/packages/core/src/body/errors.ts @@ -43,6 +43,25 @@ export class MultipartBoundaryError extends DexpaceError { } } +/** + * A form field that cannot be rendered into an `x-www-form-urlencoded` body (HTTP-38/BODY-35) -- a + * non-string field name, or a value that is neither a primitive nor `null`. Raised rather than dropping + * the field, which would put a silently incomplete body on the wire. + * + * @public + */ +export class FormBodyValidationError extends DexpaceError { + readonly field: string; + + constructor(field: string, value: unknown, options?: ErrorOptions) { + super( + `form field ${JSON.stringify(field)} has an unsupported value of type ${typeof value} -- use a string, number, boolean, bigint, or null`, + options, + ); + this.field = field; + } +} + /** * Type guard for body errors. * @@ -50,9 +69,11 @@ export class MultipartBoundaryError extends DexpaceError { */ export function isBodyError( error: unknown, -): error is ConsumedBodyError | MultipartBoundaryError { +): error is + ConsumedBodyError | MultipartBoundaryError | FormBodyValidationError { return ( error instanceof ConsumedBodyError || - error instanceof MultipartBoundaryError + error instanceof MultipartBoundaryError || + error instanceof FormBodyValidationError ); } diff --git a/packages/core/src/body/http-status-error.test.ts b/packages/core/src/body/http-status-error.test.ts index 7a06852..cd85c01 100644 --- a/packages/core/src/body/http-status-error.test.ts +++ b/packages/core/src/body/http-status-error.test.ts @@ -1,7 +1,8 @@ // SPDX-License-Identifier: MIT // packages/core/src/body/http-status-error.test.ts // Exercises: HTTP-52/BODY-30 (1 MiB cap, replayable re-serve, buffered inside close-guaranteeing scope), -// BODY-31 (4xx/5xx only, no-body response returned unchanged), BODY-33 (non-consuming preview) +// BODY-31 (4xx/5xx only, no-body response returned unchanged), BODY-33 (non-consuming preview), +// HTTP-42 (preview decodes with the media type's charset, falling back to UTF-8, never throwing) import {describe, expect, test} from 'bun:test'; import {Headers} from '../http/headers.js'; import {Protocol} from '../http/protocol.js'; @@ -87,3 +88,61 @@ describe('HttpStatusError (HTTP-52/BODY-30)', () => { expect(error?.preview()).toBe('boom'); }); }); + +function contentType(value: string): Headers { + return Headers.newBuilder().add('content-type', value).build(); +} + +describe('preview charset resolution (HTTP-42, BODY-33)', () => { + const cafeLatin1 = Uint8Array.from([0x63, 0x61, 0x66, 0xe9]); // "café" in ISO-8859-1 + + test('decodes with the charset declared by the response media type', async () => { + const error = await toHttpError( + responseWith( + 500, + readableOf(cafeLatin1), + contentType('text/plain; charset=iso-8859-1'), + ), + ); + expect(error?.preview()).toBe('café'); + }); + + test('an explicit charset argument still wins', async () => { + const error = await toHttpError( + responseWith(500, readableOf(cafeLatin1), contentType('text/plain')), + ); + expect(error?.preview('iso-8859-1')).toBe('café'); + }); + + test('an unknown charset falls back to UTF-8 instead of raising a RangeError', async () => { + const error = await toHttpError( + responseWith( + 500, + readableOf(new TextEncoder().encode('ok')), + contentType('text/plain; charset=bogus-charset'), + ), + ); + expect(error?.preview()).toBe('ok'); + expect(error?.preview('also-bogus')).toBe('ok'); + }); + + test('defaults to UTF-8 when no media type was sent', async () => { + const error = await toHttpError( + responseWith(500, readableOf(new TextEncoder().encode('héllo'))), + ); + expect(error?.preview()).toBe('héllo'); + }); + + test('body() drops an inbound media type that is not outbound-safe (HTTP-18/HTTP-19)', async () => { + // HTTP-19 admits obs-text (>= 0x80) inbound; HTTP-18 forbids it outbound. Re-serving a received + // content-type on an outbound Body must drop it, never raise from an accessor on an error object. + const headers = Headers.newBuilder() + .addInbound('content-type', 'text/plain; note="\u00e9"') + .build(); + const error = await toHttpError( + responseWith(500, readableOf(Uint8Array.from([1])), headers), + ); + expect(error?.body()?.mediaType).toBeUndefined(); + expect(error?.preview()).toBe('\u0001'); // still previews, charset resolution falls back + }); +}); diff --git a/packages/core/src/body/http-status-error.ts b/packages/core/src/body/http-status-error.ts index 956297a..a877552 100644 --- a/packages/core/src/body/http-status-error.ts +++ b/packages/core/src/body/http-status-error.ts @@ -1,9 +1,11 @@ // SPDX-License-Identifier: MIT // packages/core/src/body/http-status-error.ts +import {decodeText, resolveCharset} from '../http/charset.js'; import {DexpaceError} from '../http/errors.js'; import type {Response} from '../http/response.js'; import {invariant} from '../invariant.js'; import type {Body} from './body.js'; +import {headerSafeMediaType} from './media-type-safety.js'; import {byteArrayBody} from './simple-bodies.js'; // Fixed by HTTP-52. Deliberately NOT BODY-34's shared preview cap, which is configurable and covers the @@ -40,13 +42,25 @@ export class HttpStatusError extends DexpaceError { body(): Body | undefined { return this.#bodyBytes === undefined ? undefined - : byteArrayBody(this.#bodyBytes, this.#mediaType); + : // Dropped rather than raised when the received content-type is not outbound-safe: an inbound + // value may legally carry obs-text (HTTP-19) that an outbound body may not (HTTP-18). + byteArrayBody(this.#bodyBytes, headerSafeMediaType(this.#mediaType)); } - /** Non-consuming preview from the buffered copy (BODY-33). Null for no body. */ - preview(charset = 'utf-8'): string | null { + /** + * Non-consuming preview from the buffered copy (BODY-33). Null for no body. + * + * Decodes with `charset` when given, otherwise with the charset declared by the response's media type, + * falling back to UTF-8 when that is absent or unknown -- the same resolution `Response.text()` uses + * (HTTP-42). Never throws: an unrecognized label falls back rather than raising a RangeError out of a + * method on an error object, where a caller is least able to handle another exception. + */ + preview(charset?: string): string | null { if (this.#bodyBytes === undefined) return null; - return new TextDecoder(charset).decode(this.#bodyBytes); + return decodeText( + this.#bodyBytes, + charset ?? resolveCharset(this.#mediaType), + ); } } @@ -55,6 +69,7 @@ export class HttpStatusError extends DexpaceError { * response's own close-guaranteeing scope (HTTP-52/BODY-30). Returns null for a non-error response * (BODY-31) -- the caller keeps the response, body intact. * + * @throws Whatever reading the response body raises; the response is closed either way (BODY-16). * @public */ export async function toHttpError( diff --git a/packages/core/src/body/index.ts b/packages/core/src/body/index.ts index 9a09d83..902ce74 100644 --- a/packages/core/src/body/index.ts +++ b/packages/core/src/body/index.ts @@ -6,6 +6,7 @@ export type {Body} from './body.js'; export { ConsumedBodyError, + FormBodyValidationError, isBodyError, MultipartBoundaryError, } from './errors.js'; @@ -28,6 +29,7 @@ export { formUrlEncodedBody, FormUrlEncodedBody, type FormUrlEncodedInput, + type FormUrlEncodedValue, stringBody, StringBody, } from './simple-bodies.js'; diff --git a/packages/core/src/body/materialize.ts b/packages/core/src/body/materialize.ts index 3cd54ad..f001d0c 100644 --- a/packages/core/src/body/materialize.ts +++ b/packages/core/src/body/materialize.ts @@ -8,6 +8,9 @@ import {byteArrayBody} from './simple-bodies.js'; * Returns `body` unchanged if already replayable; otherwise drains its single write into a fresh * replayable ByteArrayBody, after which the original is treated as consumed (BODY-3/HTTP-37). * + * @throws ConsumedBodyError when `body` is single-use and has already been written (BODY-3). + * @throws Whatever the delegate's `writeTo` raises -- an EndOfStreamError from a stream body whose + * byte count disagrees with its declared length, for instance (HTTP-39/BODY-10). * @public */ export async function materialize(body: Body): Promise { diff --git a/packages/core/src/body/media-type-safety.ts b/packages/core/src/body/media-type-safety.ts new file mode 100644 index 0000000..ba0f383 --- /dev/null +++ b/packages/core/src/body/media-type-safety.ts @@ -0,0 +1,38 @@ +// SPDX-License-Identifier: MIT +// packages/core/src/body/media-type-safety.ts +import {hasForbiddenOutboundByte} from '../http/ascii-validation.js'; +import {MediaTypeParseError} from '../http/errors.js'; + +/** + * Rejects a media type that is not header-safe, using the same predicate as outbound header-value + * validation (HTTP-26). + * + * `Body.mediaType` is interpolated into a multipart part header verbatim (HTTP-51), so a CR/LF inside it + * is a header-injection primitive: it can append arbitrary headers, close the header block outright, and + * forge a closing boundary, all while the shared framing routine keeps the declared content length + * consistent with the corrupted bytes. Validating at construction closes it at the source -- a media type + * containing a control character is never legitimate. + */ +export function assertHeaderSafeMediaType(mediaType: string | undefined): void { + if (mediaType === undefined) return; + if (hasForbiddenOutboundByte(mediaType)) { + throw new MediaTypeParseError( + `media type must not contain a control character or non-ASCII byte: ${JSON.stringify(mediaType)}`, + ); + } +} + +/** + * Returns `mediaType` when it is header-safe, otherwise undefined. + * + * For media types that arrive from the wire rather than from a caller. HTTP-19 deliberately lets an + * inbound header value carry obs-text (>= 0x80) that HTTP-18 forbids outbound, so re-serving a received + * `content-type` on an outbound body can legitimately fail {@link assertHeaderSafeMediaType}. Dropping + * the media type is the right trade there -- raising from an accessor on an error object is not. + */ +export function headerSafeMediaType( + mediaType: string | undefined, +): string | undefined { + if (mediaType === undefined) return undefined; + return hasForbiddenOutboundByte(mediaType) ? undefined : mediaType; +} diff --git a/packages/core/src/body/multipart-body.test.ts b/packages/core/src/body/multipart-body.test.ts index 0fa6d94..a3ced6f 100644 --- a/packages/core/src/body/multipart-body.test.ts +++ b/packages/core/src/body/multipart-body.test.ts @@ -1,9 +1,12 @@ // SPDX-License-Identifier: MIT // packages/core/src/body/multipart-body.test.ts // Exercises: BODY-2 (composite replayability, unknown-length collapse), HTTP-51 (shared framing routine, -// boundary generation/validation, header quoting) +// boundary generation/validation, header quoting, and a part media type that cannot break the framing), +// HTTP-26 (a media type is header-safe), RECOV-12 (a close failure never masks the primary failure) import {describe, expect, test} from 'bun:test'; import fc from 'fast-check'; +import {MediaTypeParseError} from '../http/errors.js'; +import type {Body} from './body.js'; import {MultipartBoundaryError} from './errors.js'; import { MultipartBody, @@ -214,3 +217,55 @@ describe('MultipartBody property tests (HTTP-51)', () => { ); }); }); + +// A hand-rolled Body bypassing the bundled factories' construction-time validation -- MultipartPart +// accepts any Body, so the framing routine cannot assume the media type was already checked. +function forgedBody(mediaType: string): Body { + return { + kind: 'byte-array', + mediaType, + contentLength: 1, + replayable: true, + writeTo: async sink => { + const writer = sink.getWriter(); + await writer.write(Uint8Array.from([120])); + await writer.close(); + }, + }; +} + +describe('a part media type cannot break the framing (HTTP-51)', () => { + test('a media type carrying CR/LF is refused, not interpolated', () => { + const part = { + name: 'f', + body: forgedBody('text/plain\r\nX-Injected: pwned'), + }; + expect(() => multipartBody([part], 'BOUNDARY')).toThrow( + MediaTypeParseError, + ); + }); + + test('a media type that would forge a closing boundary is refused', () => { + const part = { + name: 'f', + body: forgedBody('text/plain\r\n\r\nSMUGGLED\r\n--BOUNDARY--'), + }; + // Without this the declared contentLength still matches the written bytes -- the shared framing + // routine counts the forged bytes too, so the wire is consistently, silently wrong. + expect(() => multipartBody([part], 'BOUNDARY')).toThrow( + MediaTypeParseError, + ); + }); +}); + +describe('MultipartBody failure propagation (RECOV-12)', () => { + test('surfaces the sink failure, not a close TypeError', () => { + const body = multipartBody([{name: 'a', body: stringBody('x')}], 'B'); + const sink = new WritableStream({ + write: () => { + throw new Error('SOCKET GONE'); + }, + }); + expect(body.writeTo(sink)).rejects.toThrow('SOCKET GONE'); + }); +}); diff --git a/packages/core/src/body/multipart-body.ts b/packages/core/src/body/multipart-body.ts index e3d4c4e..801b3f4 100644 --- a/packages/core/src/body/multipart-body.ts +++ b/packages/core/src/body/multipart-body.ts @@ -4,6 +4,8 @@ import type {Builder} from '../http/builder.js'; import {invariant} from '../invariant.js'; import type {Body} from './body.js'; import {MultipartBoundaryError} from './errors.js'; +import {assertHeaderSafeMediaType} from './media-type-safety.js'; +import {withBodyWriter} from './write-body.js'; /** * A part inside a {@link MultipartBody}. @@ -58,8 +60,14 @@ function renderPartHeader(part: MultipartPart, boundary: string): Uint8Array { if (part.filename !== undefined) header += `; filename="${quoteParam(part.filename)}"`; header += '\r\n'; - if (part.body.mediaType !== undefined) + if (part.body.mediaType !== undefined) { + // Defence in depth: the bundled Body implementations validate at construction, but `MultipartPart` + // accepts any `Body`, and this value is interpolated raw. A CR/LF here would append arbitrary + // headers, close the header block, or forge a closing boundary -- and because this routine is shared + // with computeContentLength, the declared length would agree with the corrupted bytes (HTTP-51). + assertHeaderSafeMediaType(part.body.mediaType); header += `Content-Type: ${part.body.mediaType}\r\n`; + } header += '\r\n'; return new TextEncoder().encode(header); } @@ -135,23 +143,23 @@ export class MultipartBody implements Body { } async writeTo(sink: WritableStream): Promise { - const writer = sink.getWriter(); - try { + await withBodyWriter(sink, async writer => { for (const part of this.#parts) { await writer.write(renderPartHeader(part, this.#boundary)); await part.body.writeTo(nonClosingSink(writer)); await writer.write(CRLF); } await writer.write(trailerBytes(this.#boundary)); - } finally { - await writer.close(); - } + }); } } /** * Creates a MultipartBody (BODY-2, HTTP-51). * + * @throws MultipartBoundaryError when `boundary` violates RFC 2046's bchars grammar (HTTP-51). + * @throws MediaTypeParseError when a part's media type contains a control character or non-ASCII byte, + * which would let it break out of the part header it is rendered into (HTTP-26/HTTP-51). * @public */ export function multipartBody( @@ -185,6 +193,12 @@ export class MultipartBodyBuilder implements Builder { return this; } + /** + * @throws MultipartBoundaryError when the configured boundary violates RFC 2046's bchars grammar + * (HTTP-51). + * @throws MediaTypeParseError when a part's media type contains a control character or non-ASCII byte + * (HTTP-26/HTTP-51). + */ build(): MultipartBody { return new MultipartBody(this.#parts, this.#boundary); } diff --git a/packages/core/src/body/request-body-logging.test.ts b/packages/core/src/body/request-body-logging.test.ts index cd30b99..c167f0d 100644 --- a/packages/core/src/body/request-body-logging.test.ts +++ b/packages/core/src/body/request-body-logging.test.ts @@ -2,7 +2,8 @@ // packages/core/src/body/request-body-logging.test.ts // Exercises: BODY-17 (mirror + forward the full untruncated payload), BODY-18 (tap clears at the start // of every write), BODY-19 (tap cap, full payload unaffected), BODY-20 (partial-failure snapshot), BODY-21 -// (replayable/materialize pass through, preserving the tap), BODY-37 (no backing-buffer escape hatch) +// (replayable/materialize pass through, preserving the tap CAP without sharing its buffer), BODY-37 (no +// backing-buffer escape hatch) import {describe, expect, test} from 'bun:test'; import fc from 'fast-check'; import {InvariantViolation} from '../invariant.js'; @@ -151,3 +152,41 @@ describe('withRequestLogging replayability, materialize, and protection (BODY-21 ).toThrow(InvariantViolation); }); }); + +function bytesStream(...values: number[]): ReadableStream { + return new ReadableStream({ + start(controller) { + controller.enqueue(Uint8Array.from(values)); + controller.close(); + }, + }); +} + +describe('materialize does not alias the tap (BODY-21)', () => { + test('each wrapper keeps its own buffer, so one write cannot rewrite the other preview', async () => { + const logged = withRequestLogging( + streamBody(bytesStream(1, 2, 3), undefined, 3), + 100, + ); + const materialized = await logged.materialize(); + + const {sink} = collectingSink(); + await materialized.writeTo(sink); + + expect([...materialized.snapshot()]).toEqual([1, 2, 3]); + // BODY-18 clears the tap at the start of every write. With one shared ByteQueue, a Phase 7 retry + // loop's second attempt silently rewrites the preview the first-attempt wrapper is still holding. + expect([...logged.snapshot()]).toEqual([]); + }); + + test('the materialized wrapper still honours the configured cap', async () => { + const logged = withRequestLogging( + streamBody(bytesStream(1, 2, 3), undefined, 3), + 2, + ); + const materialized = await logged.materialize(); + const {sink} = collectingSink(); + await materialized.writeTo(sink); + expect([...materialized.snapshot()]).toEqual([1, 2]); + }); +}); diff --git a/packages/core/src/body/request-body-logging.ts b/packages/core/src/body/request-body-logging.ts index 502e416..41666ca 100644 --- a/packages/core/src/body/request-body-logging.ts +++ b/packages/core/src/body/request-body-logging.ts @@ -31,9 +31,13 @@ export function withRequestLogging( `tapCapBytes must be non-negative, got ${String(tapCapBytes)}`, ); const cap = Math.min(tapCapBytes, MAX_BYTE_ARRAY_LENGTH); - const tap = new ByteQueue(); function wrap(inner: Body): LoggedBody { + // Per-wrapper, never hoisted to the factory scope. BODY-21 asks materialize() to preserve the tap + // *cap*, not to share the buffer: two live wrappers over one ByteQueue means BODY-18's clear-on-write + // in the materialized wrapper silently rewrites the preview the pre-materialization wrapper is still + // holding -- which is precisely what a Phase 7 retry loop does between attempts. + const tap = new ByteQueue(); return { kind: inner.kind, mediaType: inner.mediaType, diff --git a/packages/core/src/body/response-body-logging.test.ts b/packages/core/src/body/response-body-logging.test.ts index 2930be3..6108c07 100644 --- a/packages/core/src/body/response-body-logging.test.ts +++ b/packages/core/src/body/response-body-logging.test.ts @@ -3,10 +3,12 @@ // Exercises: BODY-22 (lazy, drain-once), BODY-23 (fits-cap: full capture, repeatable non-consuming // reads), BODY-24 (exceeds-cap: prefix+tail once, second read fails), BODY-26 (drain failure cached, // partial bytes retained, error() does not drain), BODY-27 (close-once shared guard), BODY-28 (captured -// buffer survives close), BODY-29 (reported length), BODY-32 (negative cap rejected) +// buffer survives close), BODY-29 (reported length), BODY-32 (negative cap rejected), BODY-25 (a +// zero-length delegate chunk is a stream-contract violation, never end-of-stream) import {describe, expect, test} from 'bun:test'; import fc from 'fast-check'; import {InvariantViolation} from '../invariant.js'; +import {SourceContractViolationError} from '../io/errors.js'; import {withResponseLogging} from './response-body-logging.js'; function readableOf(...chunks: number[][]): ReadableStream { @@ -67,11 +69,26 @@ describe('withResponseLogging regimes (BODY-22..24)', () => { }); describe('withResponseLogging lifecycle (BODY-27, 28)', () => { - test('close is idempotent and shared across the wrapper close and tail completion (BODY-27)', async () => { - const logged = withResponseLogging(readableOf([1, 2, 3]), 100); - await readAll(await logged.read()); + test('the delegate is cancelled at most once however often close is called (BODY-27)', async () => { + // The exceeds-cap regime deliberately: on the fits path the delegate is already closed by the time + // the guard runs, so cancel() is a spec no-op and a counter there proves nothing -- and BODY-27 + // exists for the transports that are less forgiving than a spec-compliant ReadableStream. + const {stream, cancels} = countingStream([1, 2], [3, 4]); + const logged = withResponseLogging(stream, 1); + await logged.read(); await logged.close(); await logged.close(); + await logged.close(); + expect(cancels()).toBe(1); + }); + + test('the wrapper close and the tail stream share one guard (BODY-27)', async () => { + const {stream, cancels} = countingStream([1, 2], [3, 4]); + const logged = withResponseLogging(stream, 1); + const tail = await logged.read(); + await tail.cancel(); // tail path + await logged.close(); // wrapper path + expect(cancels()).toBe(1); }); test('the captured buffer survives close -- snapshot still works after (BODY-28)', async () => { @@ -155,3 +172,99 @@ describe('withResponseLogging properties and lengths (BODY-29..34)', () => { ); }); }); + +/** + * A delegate that counts calls to `cancel()` and throws on the second, standing in for the transports + * BODY-27 names -- the ones that do not tolerate a double close. Counting the underlying source's + * `cancel` callback instead would prove nothing: the Streams spec makes a second `cancel()` on an + * already-cancelled stream a resolved no-op that never reaches the source. + */ +function countingStream(...chunks: number[][]): { + stream: ReadableStream; + cancels: () => number; +} { + let cancels = 0; + const stream = new ReadableStream({ + start(controller) { + for (const chunk of chunks) controller.enqueue(Uint8Array.from(chunk)); + controller.close(); + }, + }); + const delegate = stream.cancel.bind(stream); + stream.cancel = async (reason?: unknown): Promise => { + cancels += 1; + if (cancels > 1) + throw new Error('transport does not tolerate a double close'); + return delegate(reason); + }; + return {stream, cancels: () => cancels}; +} + +describe('close failures (BODY-28)', () => { + test('a non-TypeError from cancel() propagates rather than being swallowed', async () => { + const stream = new ReadableStream({ + start(controller) { + controller.enqueue(Uint8Array.from([1, 2])); + }, + }); + stream.cancel = (): Promise => + Promise.reject(new Error('CONNECTION STUCK')); + const logged = withResponseLogging(stream, 1); + await logged.read(); // exceeds-cap regime leaves the delegate live, so close() really cancels + expect(logged.close()).rejects.toThrow('CONNECTION STUCK'); + }); +}); + +describe('delegate stream contract (BODY-25)', () => { + function emptyThenData(): ReadableStream { + return new ReadableStream({ + start(controller) { + controller.enqueue(new Uint8Array(0)); + controller.enqueue(Uint8Array.from([7])); + controller.close(); + }, + }); + } + + test('a zero-length chunk is raised, never tolerated as a no-op', () => { + // Matches RetentionWindow under IO-17's identical rule: a response body reaches both this tee and + // BufferedSource, so the two layers must not disagree about the same upstream. + const logged = withResponseLogging(emptyThenData(), 100); + expect(logged.read()).rejects.toThrow(SourceContractViolationError); + }); + + test('the violation is cached like any other drain failure (BODY-26)', () => { + const logged = withResponseLogging(emptyThenData(), 100); + expect(logged.read()).rejects.toThrow(SourceContractViolationError); + expect(logged.error()).toBeInstanceOf(SourceContractViolationError); + expect(logged.read()).rejects.toThrow(SourceContractViolationError); + }); +}); + +describe('snapshot is a drain trigger (BODY-22)', () => { + test('calling snapshot starts the drain, without a read()', async () => { + const logged = withResponseLogging(readableOf([1, 2, 3]), 100); + expect([...logged.snapshot()]).toEqual([]); // synchronous: the drain has only just been started + await new Promise(resolve => setTimeout(resolve, 0)); + expect([...logged.snapshot()]).toEqual([1, 2, 3]); + }); + + test('the drain still happens exactly once (BODY-22)', async () => { + const logged = withResponseLogging(readableOf([1, 2, 3]), 100); + logged.snapshot(); + logged.snapshot(); + expect([...(await readAll(await logged.read()))]).toEqual([1, 2, 3]); + }); + + test('a snapshot-triggered drain failure still reaches read(), and does not go unhandled', async () => { + const stream = new ReadableStream({ + start(controller) { + controller.error(new Error('UPSTREAM GONE')); + }, + }); + const logged = withResponseLogging(stream, 100); + expect([...logged.snapshot()]).toEqual([]); + await new Promise(resolve => setTimeout(resolve, 0)); + expect(logged.read()).rejects.toThrow('UPSTREAM GONE'); + }); +}); diff --git a/packages/core/src/body/response-body-logging.ts b/packages/core/src/body/response-body-logging.ts index 712b4e4..7652a0c 100644 --- a/packages/core/src/body/response-body-logging.ts +++ b/packages/core/src/body/response-body-logging.ts @@ -2,6 +2,7 @@ // packages/core/src/body/response-body-logging.ts import {invariant} from '../invariant.js'; import {ByteQueue} from '../io/byte-queue.js'; +import {SourceContractViolationError} from '../io/errors.js'; import {MAX_BYTE_ARRAY_LENGTH} from '../io/limits.js'; import {ConsumedBodyError} from './errors.js'; @@ -57,9 +58,12 @@ async function closeDelegate(state: DrainState): Promise { * Reads until EOF (fits regime) or until the cap is reached (exceeds regime, leaving the delegate open * and the overflow chunk staged). BODY-26: a failure is cached, never allowed to truncate silently. * - * BODY-25 note: the requirement's "zero bytes returned for a positive requested count" has no analog - * here -- `ReadableStreamDefaultReader.read()` takes no count, and a zero-length chunk is a legal - * no-op, not an EOF signal. EOF is signalled only by `{done: true}`, which is what the loop keys on. + * BODY-25: a delegate chunk of zero bytes is a stream-contract violation, not a no-op and never EOF -- + * EOF is signalled only by `{done: true}`. `ReadableStreamDefaultReader.read()` carries no requested + * count, so the requirement's "for a positive requested count" has no literal analog, but the tolerant + * reading is the wrong one to pick: `RetentionWindow` raises on the same input under IO-17's identical + * rule, and a response body reaches both this tee and `BufferedSource`, so a divergence would make one + * upstream fail or succeed depending only on which wrapper it passed through. */ async function drainOnce(state: DrainState): Promise { try { @@ -71,6 +75,11 @@ async function drainOnce(state: DrainState): Promise { await closeDelegate(state); return; } + if (value.length === 0) { + throw new SourceContractViolationError( + 'source delivered 0 bytes without signalling end of stream', + ); // BODY-25 + } if (state.captured.size + value.length <= state.cap) { state.captured.writeBytes(value); continue; @@ -92,6 +101,19 @@ async function drainOnce(state: DrainState): Promise { } } +/** + * BODY-22's once-only, lazily-started drain. Concurrent first accesses share the one in-flight promise. + * + * The detached `.catch` matters: a snapshot-triggered drain has no awaiter, so without it a drain failure + * becomes an unhandled rejection. Attaching a handler to a *copy* leaves the stored promise rejected, so + * `read()` still re-throws the cached failure on every call (BODY-26). + */ +function startDrain(state: DrainState): Promise { + state.started ??= drainOnce(state); + void state.started.catch(() => undefined); + return state.started; +} + /** A fresh, non-consuming view over the fully-captured bytes. Repeatable (BODY-23). */ function capturedStream(state: DrainState): ReadableStream { const bytes = state.captured.snapshot(); @@ -170,8 +192,7 @@ export function withResponseLogging( return { async read(): Promise> { - state.started ??= drainOnce(state); - await state.started; // a cached failure re-throws here on every call (BODY-26) + await startDrain(state); // a cached failure re-throws here on every call (BODY-26) if (state.regime === 'fits') return capturedStream(state); if (state.tailConsumed) { throw new ConsumedBodyError('logged-response'); @@ -179,7 +200,14 @@ export function withResponseLogging( state.tailConsumed = true; return tailStream(state); }, - snapshot: () => state.captured.snapshot(), + snapshot(): Uint8Array { + // BODY-22 lists snapshot in the drain's trigger set alongside read. The accessor is synchronous, + // so it starts the drain and returns what has been captured so far rather than awaiting it; a + // later read() awaits the very same in-flight promise, so the delegate is still read exactly once. + // (BODY-26's "snapshot returns the partial bytes without throwing" is why it cannot await here.) + void startDrain(state); + return state.captured.snapshot(); + }, error: () => state.failure, // deliberately does not drain (BODY-26) get contentLength(): number { // BODY-29: the capture is the true length only when the whole body fit within the cap. diff --git a/packages/core/src/body/simple-bodies.test.ts b/packages/core/src/body/simple-bodies.test.ts index 4a800c6..2965924 100644 --- a/packages/core/src/body/simple-bodies.test.ts +++ b/packages/core/src/body/simple-bodies.test.ts @@ -1,8 +1,12 @@ // SPDX-License-Identifier: MIT // packages/core/src/body/simple-bodies.test.ts // Exercises: HTTP-36/BODY-1 (mediaType, contentLength, replayable, writeTo), HTTP-38/BODY-35 (replayable -// by source; form-urlencoded uses "+" for space, distinct from RFC 3986 query encoding) +// by source; form-urlencoded uses "+" for space, distinct from RFC 3986 query encoding; a field value +// that cannot be rendered is raised, never dropped), HTTP-26/HTTP-51 (a media type is header-safe), +// RECOV-12 (a close failure never masks the primary write failure) import {describe, expect, test} from 'bun:test'; +import {MediaTypeParseError} from '../http/errors.js'; +import {FormBodyValidationError} from './errors.js'; import { byteArrayBody, formUrlEncodedBody, @@ -95,3 +99,90 @@ describe('FormUrlEncodedBody (HTTP-38/BODY-35)', () => { expect(new TextDecoder().decode(await drain(body))).toBe('a%26b=c%3Dd'); }); }); + +function failingSink(): WritableStream { + return new WritableStream({ + write: () => { + throw new Error('SOCKET GONE'); + }, + }); +} + +function decode(bytes: Uint8Array): string { + return new TextDecoder().decode(bytes); +} + +describe('media types are header-safe (HTTP-26/HTTP-51)', () => { + test('byteArrayBody rejects a media type carrying CR/LF', () => { + expect(() => + byteArrayBody(Uint8Array.from([1]), 'text/plain\r\nX-Injected: pwned'), + ).toThrow(MediaTypeParseError); + }); + + test('stringBody rejects a media type carrying a control character', () => { + expect(() => stringBody('x', 'text/plain\u0007')).toThrow( + MediaTypeParseError, + ); + }); + + test('a parameterised media type is still accepted', () => { + expect( + byteArrayBody(Uint8Array.from([1]), 'text/plain; charset=utf-8') + .mediaType, + ).toBe('text/plain; charset=utf-8'); + }); +}); + +describe('a write failure is never masked by the close (RECOV-12, RETRY-2)', () => { + test('ByteArrayBody surfaces the sink failure', () => { + expect( + byteArrayBody(Uint8Array.from([1, 2])).writeTo(failingSink()), + ).rejects.toThrow('SOCKET GONE'); + }); + + test('StringBody surfaces the sink failure', () => { + expect(stringBody('hi').writeTo(failingSink())).rejects.toThrow( + 'SOCKET GONE', + ); + }); + + test('FormUrlEncodedBody surfaces the sink failure', () => { + expect(formUrlEncodedBody({a: 'b'}).writeTo(failingSink())).rejects.toThrow( + 'SOCKET GONE', + ); + }); +}); + +describe('form field values (HTTP-38/BODY-35)', () => { + test('primitives are rendered rather than dropped', async () => { + const body = formUrlEncodedBody({count: 5, flag: true, big: 9n}); + expect(decode(await drain(body))).toBe('count=5&flag=true&big=9'); + }); + + test('null renders as a valueless parameter', async () => { + expect(decode(await drain(formUrlEncodedBody({empty: null})))).toBe( + 'empty=', + ); + }); + + test('array values render element-wise', async () => { + expect(decode(await drain(formUrlEncodedBody({tag: ['a', 2]})))).toBe( + 'tag=a&tag=2', + ); + }); + + test('a value that cannot be rendered throws naming the field', () => { + expect(() => formUrlEncodedBody({profile: {a: 1}} as never)).toThrow( + FormBodyValidationError, + ); + expect(() => formUrlEncodedBody({profile: {a: 1}} as never)).toThrow( + /"profile"/, + ); + }); + + test('undefined is rejected too -- an absent field is never guessed at', () => { + expect(() => formUrlEncodedBody({missing: undefined} as never)).toThrow( + FormBodyValidationError, + ); + }); +}); diff --git a/packages/core/src/body/simple-bodies.ts b/packages/core/src/body/simple-bodies.ts index 21dbad7..7a7c593 100644 --- a/packages/core/src/body/simple-bodies.ts +++ b/packages/core/src/body/simple-bodies.ts @@ -3,6 +3,9 @@ import {QueryParams, type QueryParamsBuilder} from '../http/query-params.js'; import {invariant} from '../invariant.js'; import type {Body} from './body.js'; +import {FormBodyValidationError} from './errors.js'; +import {assertHeaderSafeMediaType} from './media-type-safety.js'; +import {withBodyWriter} from './write-body.js'; /** * A body backed by an in-memory byte array (BODY-1). Always replayable. @@ -17,6 +20,7 @@ export class ByteArrayBody implements Body { readonly #bytes: Uint8Array; constructor(bytes: Uint8Array, mediaType?: string) { + assertHeaderSafeMediaType(mediaType); // HTTP-26/HTTP-51 // Defensive copy: `bytes` caller passed might be mutated later (HTTP-1). Kept `#private` -- // exposing this publicly would let a caller mutate a "replayable" body's contents after // construction, silently breaking the byte-for-byte-identical guarantee BODY-1 requires. @@ -26,18 +30,17 @@ export class ByteArrayBody implements Body { } async writeTo(sink: WritableStream): Promise { - const writer = sink.getWriter(); - try { + await withBodyWriter(sink, async writer => { if (this.#bytes.length > 0) await writer.write(this.#bytes); - } finally { - await writer.close(); - } + }); } } /** * Creates a replayable ByteArrayBody (BODY-1). * + * @throws MediaTypeParseError when `mediaType` contains a control character or non-ASCII byte, which + * would let it break out of the header it is rendered into (HTTP-26/HTTP-51). * @public */ export function byteArrayBody( @@ -61,6 +64,7 @@ export class StringBody implements Body { readonly #bytes: Uint8Array; constructor(text: string, mediaType = 'text/plain; charset=utf-8') { + assertHeaderSafeMediaType(mediaType); // HTTP-26/HTTP-51 this.text = text; this.mediaType = mediaType; this.#bytes = new TextEncoder().encode(text); @@ -68,18 +72,17 @@ export class StringBody implements Body { } async writeTo(sink: WritableStream): Promise { - const writer = sink.getWriter(); - try { + await withBodyWriter(sink, async writer => { if (this.#bytes.length > 0) await writer.write(this.#bytes); - } finally { - await writer.close(); - } + }); } } /** * Creates a replayable StringBody (BODY-1). * + * @throws MediaTypeParseError when `mediaType` contains a control character or non-ASCII byte, which + * would let it break out of the header it is rendered into (HTTP-26/HTTP-51). * @public */ export function stringBody( @@ -89,6 +92,14 @@ export function stringBody( return new StringBody(text, mediaType); } +/** + * A form field value. Primitives are rendered with their standard string form; `null` produces a + * valueless parameter. Anything else is rejected rather than silently dropped. + * + * @public + */ +export type FormUrlEncodedValue = string | number | boolean | bigint | null; + /** * Accepted input shapes for {@link formUrlEncodedBody}. * @@ -96,9 +107,23 @@ export function stringBody( */ export type FormUrlEncodedInput = | QueryParams - | ReadonlyMap - | Record - | readonly (readonly [string, string])[]; + | ReadonlyMap + | Record + | readonly (readonly [string, FormUrlEncodedValue])[]; + +// BODY-35: a form field that is neither a primitive nor null cannot be rendered, and dropping it would +// put a silently incomplete body on the wire. Fail naming the key instead. +function toFieldValue(key: string, value: unknown): string | null { + if (typeof value === 'string' || value === null) return value; + if ( + typeof value === 'number' || + typeof value === 'boolean' || + typeof value === 'bigint' + ) { + return String(value); + } + throw new FormBodyValidationError(key, value); +} function addParamValue( builder: QueryParamsBuilder, @@ -106,40 +131,33 @@ function addParamValue( value: unknown, ): void { if (Array.isArray(value)) { - for (const v of value) { - if (typeof v === 'string') builder.add(key, v); + for (const element of value as readonly unknown[]) { + builder.add(key, toFieldValue(key, element)); } - } else if (typeof value === 'string' || value === null) { - builder.add(key, value); + return; } + builder.add(key, toFieldValue(key, value)); } function toQueryParams(input: FormUrlEncodedInput): QueryParams { if (input instanceof QueryParams) return input; const builder = QueryParams.newBuilder(); - if (input instanceof Map) { - for (const [key, value] of input.entries()) { - if (typeof key === 'string') addParamValue(builder, key, value); - } - } else if (Array.isArray(input)) { - for (const [key, value] of input as readonly (readonly [ - unknown, - unknown, - ])[]) { - if (typeof key === 'string' && typeof value === 'string') { - builder.add(key, value); - } - } - } else { - for (const [key, value] of Object.entries(input)) { - addParamValue(builder, key, value); - } + const entries: readonly (readonly [unknown, unknown])[] = + input instanceof Map + ? [...input.entries()] + : Array.isArray(input) + ? (input as readonly (readonly [unknown, unknown])[]) + : Object.entries(input); + for (const [key, value] of entries) { + if (typeof key !== 'string') + throw new FormBodyValidationError(String(key), key); + addParamValue(builder, key, value); } return builder.build(); } /** - * A body backed by URL-encoded form data (BODY-1, HTTP-50). Always replayable. + * A body backed by URL-encoded form data (BODY-1, HTTP-38/BODY-35). Always replayable. * * @public */ @@ -153,7 +171,8 @@ export class FormUrlEncodedBody implements Body { constructor(input: FormUrlEncodedInput) { this.params = toQueryParams(input); - const encoded = this.params.encode().replace(/%20/g, '+'); // HTTP-50: space encoded as '+' + // HTTP-38/BODY-35: x-www-form-urlencoded uses '+' for space, distinct from RFC 3986 query encoding. + const encoded = this.params.encode().replace(/%20/g, '+'); invariant( !encoded.includes(' '), 'form-urlencoded encoding produced illegal space', @@ -163,18 +182,17 @@ export class FormUrlEncodedBody implements Body { } async writeTo(sink: WritableStream): Promise { - const writer = sink.getWriter(); - try { + await withBodyWriter(sink, async writer => { if (this.#bytes.length > 0) await writer.write(this.#bytes); - } finally { - await writer.close(); - } + }); } } /** - * Creates a replayable FormUrlEncodedBody (BODY-1, HTTP-50). + * Creates a replayable FormUrlEncodedBody (BODY-1, HTTP-38/BODY-35). * + * @throws FormBodyValidationError when a field name is not a string, or a field value is neither a + * primitive nor `null` -- such a field cannot be rendered and is never dropped silently. * @public */ export function formUrlEncodedBody( diff --git a/packages/core/src/body/stream-body.test.ts b/packages/core/src/body/stream-body.test.ts index 8ed821f..396b929 100644 --- a/packages/core/src/body/stream-body.test.ts +++ b/packages/core/src/body/stream-body.test.ts @@ -3,8 +3,11 @@ // Exercises: BODY-9 (always single-use -- no generic mark/reset on Node's ReadableStream), BODY-3 // (second write fails loudly and is race-safe), BODY-8 (caller's stream is not force-closed -- read to // natural exhaustion), HTTP-39/BODY-10 (declared length verified, short stream raises -// delivered-of-declared), IO-3 (a contentLength below the -1 sentinel is rejected) +// delivered-of-declared, and an overrunning stream is stopped BEFORE the extra bytes reach the sink), +// IO-3 (a contentLength below the -1 sentinel is rejected), HTTP-26/HTTP-51 (a media type is +// header-safe), RECOV-12 (a close failure never masks the primary write failure) import {describe, expect, test} from 'bun:test'; +import {MediaTypeParseError} from '../http/errors.js'; import {InvariantViolation} from '../invariant.js'; import {EndOfStreamError} from '../io/errors.js'; import {ConsumedBodyError} from './errors.js'; @@ -116,3 +119,78 @@ describe('StreamBody declared length verification (HTTP-39, BODY-10, IO-3)', () expect(results.filter(r => r.status === 'rejected').length).toBe(1); }); }); + +interface SinkState { + written: number[]; + closed: boolean; + aborted: boolean; +} + +function probeSink(): {state: SinkState; sink: WritableStream} { + const state: SinkState = {written: [], closed: false, aborted: false}; + const sink = new WritableStream({ + write: chunk => void state.written.push(...chunk), + close: () => void (state.closed = true), + abort: () => void (state.aborted = true), + }); + return {state, sink}; +} + +describe('a mis-framed body never reaches the wire (HTTP-39/BODY-10)', () => { + test('an overrunning chunk is refused before any of it is written', () => { + const {state, sink} = probeSink(); + expect( + streamBody(readableOf([1, 2, 3, 4, 5, 6, 7, 8]), undefined, 3).writeTo( + sink, + ), + ).rejects.toThrow(EndOfStreamError); + // Not [1,2,3,4,5,6,7,8]: once a transport has stamped Content-Length: 3, the surplus sits on the + // socket where the peer reads it as the start of the next message. + expect(state.written).toEqual([]); + expect(state.aborted).toBe(true); + }); + + test('bytes written before the overrun stay written, the straddling chunk does not', () => { + const {state, sink} = probeSink(); + expect( + streamBody(readableOf([1, 2], [3, 4]), undefined, 3).writeTo(sink), + ).rejects.toThrow(EndOfStreamError); + expect(state.written).toEqual([1, 2]); + }); + + test('a short stream aborts the sink rather than closing it cleanly', () => { + const {state, sink} = probeSink(); + expect( + streamBody(readableOf([1]), undefined, 5).writeTo(sink), + ).rejects.toThrow(EndOfStreamError); + expect(state.aborted).toBe(true); + expect(state.closed).toBe(false); // a truncated body is never signalled as complete + }); + + test('an exact-length stream closes the sink cleanly', async () => { + const {state, sink} = probeSink(); + await streamBody(readableOf([1, 2, 3]), undefined, 3).writeTo(sink); + expect(state.written).toEqual([1, 2, 3]); + expect(state.closed).toBe(true); + expect(state.aborted).toBe(false); + }); +}); + +describe('StreamBody media type and failure propagation', () => { + test('rejects a media type carrying CR/LF (HTTP-26/HTTP-51)', () => { + expect(() => + streamBody(readableOf([1]), 'text/plain\r\nX-Injected: pwned'), + ).toThrow(MediaTypeParseError); + }); + + test('surfaces the sink failure, not a close TypeError (RECOV-12)', () => { + const sink = new WritableStream({ + write: () => { + throw new Error('SOCKET GONE'); + }, + }); + expect( + streamBody(readableOf([1, 2]), undefined, 2).writeTo(sink), + ).rejects.toThrow('SOCKET GONE'); + }); +}); diff --git a/packages/core/src/body/stream-body.ts b/packages/core/src/body/stream-body.ts index b135958..c473f1e 100644 --- a/packages/core/src/body/stream-body.ts +++ b/packages/core/src/body/stream-body.ts @@ -4,6 +4,8 @@ import {EndOfStreamError} from '../io/errors.js'; import {invariant} from '../invariant.js'; import type {Body} from './body.js'; import {ConsumedBodyError} from './errors.js'; +import {assertHeaderSafeMediaType} from './media-type-safety.js'; +import {withBodyWriter} from './write-body.js'; /** * A single-use body backed by a caller-supplied stream. @@ -23,6 +25,7 @@ export class StreamBody implements Body { mediaType?: string, contentLength = -1, ) { + assertHeaderSafeMediaType(mediaType); // HTTP-26/HTTP-51 invariant( contentLength >= -1, `contentLength must be >= -1 (-1 = unknown), got ${String(contentLength)}`, @@ -49,23 +52,30 @@ export class StreamBody implements Body { declared: number, ): Promise { const reader = this.#stream.getReader(); - const writer = sink.getWriter(); - let delivered = 0; try { - for (;;) { - // Serial by necessity: each read depends on the previous one advancing the cursor. - const {done, value} = await reader.read(); - if (done) break; - delivered += value.length; - await writer.write(value); - } + await withBodyWriter(sink, async writer => { + let delivered = 0; + for (;;) { + // Serial by necessity: each read depends on the previous one advancing the cursor. + const {done, value} = await reader.read(); + if (done) break; + // Checked BEFORE the write, not after the loop: once a transport has stamped the declared + // Content-Length, an overrun byte sits on the socket where the peer reads it as the start of + // the next message, and a thrown error cannot recall bytes already written (HTTP-39/BODY-10). + if (delivered + value.length > declared) { + throw new EndOfStreamError(delivered + value.length, declared); + } + delivered += value.length; + await writer.write(value); + } + // Raised inside the writer scope so withBodyWriter aborts: a truncated body must never be + // signalled to the sink as a clean close. + if (delivered !== declared) { + throw new EndOfStreamError(delivered, declared); + } + }); } finally { reader.releaseLock(); // BODY-8: release our handle, never cancel the caller's stream - await writer.close(); - } - - if (delivered !== declared) { - throw new EndOfStreamError(delivered, declared); } } } @@ -73,6 +83,11 @@ export class StreamBody implements Body { /** * Creates a single-use StreamBody (BODY-9). * + * @throws MediaTypeParseError when `mediaType` contains a control character or non-ASCII byte, which + * would let it break out of the header it is rendered into (HTTP-26/HTTP-51). + * @throws ConsumedBodyError from `writeTo` when the body has already been written once (BODY-3). + * @throws EndOfStreamError from `writeTo` when the stream yields a byte count other than the declared + * `contentLength` (HTTP-39/BODY-10). * @public */ export function streamBody( diff --git a/packages/core/src/body/typed-response.test.ts b/packages/core/src/body/typed-response.test.ts index b953e60..c794015 100644 --- a/packages/core/src/body/typed-response.test.ts +++ b/packages/core/src/body/typed-response.test.ts @@ -1,7 +1,7 @@ // SPDX-License-Identifier: MIT // packages/core/src/body/typed-response.test.ts -// Exercises: HTTP-44 (raw fields without touching the body, parse-once memoized including failure), -// HTTP-45 (concurrent first callers serialized to one parse run) +// Exercises: HTTP-44 (raw fields without touching the body, parse-once memoized including failure -- +// a synchronous throw included), HTTP-45 (concurrent first callers serialized to one parse run) import {describe, expect, test} from 'bun:test'; import {Protocol} from '../http/protocol.js'; import {Request} from '../http/request.js'; @@ -78,3 +78,34 @@ describe('TypedResponse', () => { expect(calls).toBe(1); }); }); + +describe('memoization covers a synchronously-throwing parser (HTTP-44)', () => { + test('the handler runs once even when it throws before returning a promise', () => { + let calls = 0; + // Typed `=> Promise` but not `async`: validating an argument before the first await is ordinary, + // and a bare `??=` never completes the assignment when the right-hand side throws. + const typed = new TypedResponse( + baseResponse(readableOf('x')), + () => { + calls += 1; + throw new Error('sync boom'); + }, + ); + for (let attempt = 0; attempt < 3; attempt += 1) { + expect(typed.value()).rejects.toThrow('sync boom'); + } + expect(calls).toBe(1); + }); + + test('the same rejected promise is handed back, never a second body read', () => { + const typed = new TypedResponse( + baseResponse(readableOf('x')), + () => { + throw new Error('sync boom'); + }, + ); + const first = typed.value(); + expect(typed.value()).toBe(first); + expect(first).rejects.toThrow('sync boom'); + }); +}); diff --git a/packages/core/src/body/typed-response.ts b/packages/core/src/body/typed-response.ts index 6fe366d..bbba8fd 100644 --- a/packages/core/src/body/typed-response.ts +++ b/packages/core/src/body/typed-response.ts @@ -48,9 +48,16 @@ export class TypedResponse { * Lazily parses and returns the typed value. Memoized: the parser function runs at most once, and * subsequent calls return the same parsed value (or re-throw the same error) without re-parsing or * re-reading the body (HTTP-44). Concurrent first callers share the single in-flight parse (HTTP-45). + * + * @throws Whatever the parser raises -- rethrown identically on every later call, never re-parsed. */ value(): Promise { - this.#memoized ??= this.#parse(this.#response); + // The `async` wrapper is load-bearing: a parser is typed `=> Promise` but may still be a plain + // function that throws synchronously (validating an argument before the first await is ordinary). + // A bare `this.#memoized ??= this.#parse(...)` never completes the assignment in that case, so the + // handler re-runs on the next call and re-reads a single-use body whose bytes are already gone -- + // exactly what HTTP-44's "without re-running the handler or re-reading the body" forbids. + this.#memoized ??= (async () => this.#parse(this.#response))(); return this.#memoized; } } diff --git a/packages/core/src/body/write-body.ts b/packages/core/src/body/write-body.ts new file mode 100644 index 0000000..535e320 --- /dev/null +++ b/packages/core/src/body/write-body.ts @@ -0,0 +1,31 @@ +// SPDX-License-Identifier: MIT +// packages/core/src/body/write-body.ts + +/** + * Runs `write` against a fresh writer over `sink`, closing on success and aborting on failure. + * + * The naive shape -- `try { ... } finally { await writer.close(); }` -- is wrong twice over. Closing an + * already-errored writer rejects with a TypeError, and a throwing `finally` *replaces* the in-flight + * exception, so the real "connection died mid-upload" cause is destroyed rather than chained (RECOV-12). + * That is not merely a bad message: RETRY-2 classifies a failure by walking its cause chain, so an I/O + * failure surfacing as a TypeError about closing a stream is silently declassified as non-retryable. + * + * Aborting rather than closing on failure also tells the transport the message is broken; a clean close + * would signal a complete body that was never fully written. + */ +export async function withBodyWriter( + sink: WritableStream, + write: (writer: WritableStreamDefaultWriter) => Promise, +): Promise { + const writer = sink.getWriter(); + try { + await write(writer); + } catch (error: unknown) { + // Best-effort: abort() resolves on an already-errored stream, and a sink whose own abort() throws + // must not displace the primary failure either. + await writer.abort(error).catch(() => undefined); + throw error; + } + // On the success path a close failure IS the primary failure, so it propagates unwrapped. + await writer.close(); +} diff --git a/packages/core/src/http/charset.ts b/packages/core/src/http/charset.ts new file mode 100644 index 0000000..8abe1c1 --- /dev/null +++ b/packages/core/src/http/charset.ts @@ -0,0 +1,28 @@ +// SPDX-License-Identifier: MIT +// packages/core/src/http/charset.ts +import {MediaType} from './media-type.js'; + +/** + * HTTP-42's charset resolution: the media type's declared `charset`, falling back to UTF-8 when the + * media type is absent or unparseable. Never throws. + */ +export function resolveCharset(mediaType: string | undefined): string { + if (mediaType === undefined) return 'utf-8'; + try { + return MediaType.parse(mediaType).charset ?? 'utf-8'; + } catch { + return 'utf-8'; + } +} + +/** + * Decodes with `charset`, falling back to UTF-8 when the label is unknown (HTTP-42). `TextDecoder` + * throws a RangeError on an unrecognized label, which callers on an error path are least able to handle. + */ +export function decodeText(bytes: Uint8Array, charset: string): string { + try { + return new TextDecoder(charset).decode(bytes); + } catch { + return new TextDecoder('utf-8').decode(bytes); + } +} diff --git a/packages/core/src/http/response.test.ts b/packages/core/src/http/response.test.ts index ec8533f..91dd10a 100644 --- a/packages/core/src/http/response.test.ts +++ b/packages/core/src/http/response.test.ts @@ -152,10 +152,48 @@ describe('bytes/text (BODY-16, HTTP-42)', () => { }); describe('close (HTTP-41/BODY-15, HTTP-43)', () => { - test('is idempotent', async () => { - const response = baseResponse(readableOf('x')); + test('cancels the body at most once however often close is called (BODY-15, HTTP-43)', async () => { + let cancels = 0; + const stream = new ReadableStream({ + start(controller) { + controller.enqueue(Uint8Array.from([1])); + }, + }); + // Counting calls to cancel(), not the source's cancel callback: the Streams spec makes a second + // cancel() on an already-cancelled stream a resolved no-op that never reaches the source, so only + // the call count can show the guard working -- and the throw stands in for a transport whose + // cancel is not re-entrant, which is why HTTP-43 asks for at-most-once in the first place. + const delegate = stream.cancel.bind(stream); + stream.cancel = async (reason?: unknown): Promise => { + cancels += 1; + if (cancels > 1) + throw new Error('transport does not tolerate a double close'); + return delegate(reason); + }; + const response = baseResponse(stream); await response.close(); await response.close(); + await response.close(); + // Counted, not merely "did not throw": cancel() on an already-cancelled ReadableStream resolves + // quietly, so idempotence observed only as the absence of a throw tests nothing. The guard exists + // for transports whose cancel is not re-entrant. + expect(cancels).toBe(1); + }); + + test('a failed release is not remembered as a successful close', () => { + const stream = new ReadableStream({ + start(controller) { + controller.enqueue(Uint8Array.from([1])); + }, + cancel() { + throw new Error('CONNECTION STUCK'); + }, + }); + const response = baseResponse(stream); + // Every caller sees the failure -- marking the response closed before awaiting would report a + // connection that was never released as released. + expect(response.close()).rejects.toThrow('CONNECTION STUCK'); + expect(response.close()).rejects.toThrow('CONNECTION STUCK'); }); test('releases the connection even when the body was never read', async () => { diff --git a/packages/core/src/http/response.ts b/packages/core/src/http/response.ts index ebe6de4..1a5de64 100644 --- a/packages/core/src/http/response.ts +++ b/packages/core/src/http/response.ts @@ -2,8 +2,8 @@ // packages/core/src/http/response.ts import type {Builder} from './builder.js'; import {requireField} from './builder.js'; +import {decodeText, resolveCharset} from './charset.js'; import {Headers} from './headers.js'; -import {MediaType} from './media-type.js'; import type {Protocol} from './protocol.js'; import type {Request} from './request.js'; import type {Status} from './status.js'; @@ -22,7 +22,7 @@ export class Response { readonly #body: ReadableStream | null; // Not `readonly` -- Object.freeze(this) below only freezes normal properties, never #private fields, // so this can still track close state after construction (BODY-15, HTTP-43). - #closed = false; + #closing: Promise | undefined; // eslint-disable-next-line max-params -- private, builder-internal; field count fixed by the wire model (HTTP-6) constructor( @@ -117,27 +117,20 @@ export class Response { /** Reads the whole body as text, defaulting to the media type's charset then UTF-8 (HTTP-42). */ async text(): Promise { const bytes = await this.bytes(); - try { - return new TextDecoder(this.#charset()).decode(bytes); - } catch { - return new TextDecoder('utf-8').decode(bytes); // HTTP-42: unrecognized charset also falls back - } - } - - #charset(): string { - const contentType = this.#headers.get('content-type'); - if (contentType === undefined) return 'utf-8'; - try { - return MediaType.parse(contentType).charset ?? 'utf-8'; - } catch { - return 'utf-8'; - } + return decodeText(bytes, resolveCharset(this.#headers.get('content-type'))); } /** Idempotent; releases the underlying connection whether or not the body was read (BODY-15, HTTP-43). */ async close(): Promise { - if (this.#closed) return; - this.#closed = true; + // Memoized rather than flag-guarded, the same shape BufferedSink.close settled on for IO-5/IO-41: + // a `#closed = true` set before the await reports a FAILED release as success to every later caller, + // over a connection that was never released. Handing every caller the same promise propagates the + // failure on every path while still cancelling at most once. + this.#closing ??= this.#release(); + return this.#closing; + } + + async #release(): Promise { if (this.#body === null) return; // BODY-15 forbids assuming the body was read, so an external consumer may still hold the reader // lock -- cancel() rejects with TypeError in that case. Swallow only that: the caller asked to diff --git a/packages/core/src/index.ts b/packages/core/src/index.ts index 7039ba9..a522929 100644 --- a/packages/core/src/index.ts +++ b/packages/core/src/index.ts @@ -35,6 +35,7 @@ export {buildRequest, OperationAssemblyError} from './seams/operation.js'; export type {Body} from './body/body.js'; export { ConsumedBodyError, + FormBodyValidationError, isBodyError, MultipartBoundaryError, } from './body/errors.js'; @@ -52,6 +53,7 @@ export { formUrlEncodedBody, type FormUrlEncodedBody, type FormUrlEncodedInput, + type FormUrlEncodedValue, stringBody, type StringBody, } from './body/simple-bodies.js'; From 8a1a874f2ad5f4c288d3a5a4036ca551986d4e9c Mon Sep 17 00:00:00 2001 From: Mohammad Wahbeh Date: Mon, 24 Aug 2026 22:30:43 +0300 Subject: [PATCH 3/3] test(core): cover withBodyWriter teardown paths (RECOV-12, RETRY-2). --- packages/core/src/body/write-body.test.ts | 104 ++++++++++++++++++++++ 1 file changed, 104 insertions(+) create mode 100644 packages/core/src/body/write-body.test.ts diff --git a/packages/core/src/body/write-body.test.ts b/packages/core/src/body/write-body.test.ts new file mode 100644 index 0000000..5cb8d33 --- /dev/null +++ b/packages/core/src/body/write-body.test.ts @@ -0,0 +1,104 @@ +// SPDX-License-Identifier: MIT +// packages/core/src/body/write-body.test.ts +// Exercises: RECOV-12 (a failure inside the writer scope is never masked by the teardown -- the sink is +// aborted, and an abort() that itself rejects does not displace the primary failure), RETRY-2 (the +// primary failure reaches the caller unwrapped so classification can walk its own cause chain) +import {describe, expect, test} from 'bun:test'; +import {rejection} from '../io/test-support/rejection.js'; +import {withBodyWriter} from './write-body.js'; + +interface SinkLog { + readonly chunks: Uint8Array[]; + closed: boolean; + abortReason: unknown; +} + +function recordingSink(overrides: UnderlyingSink = {}): { + stream: WritableStream; + log: SinkLog; +} { + const log: SinkLog = {chunks: [], closed: false, abortReason: undefined}; + const stream = new WritableStream({ + write: chunk => void log.chunks.push(chunk), + close: () => void (log.closed = true), + abort: reason => void (log.abortReason = reason), + ...overrides, + }); + return {stream, log}; +} + +describe('withBodyWriter success path', () => { + test('writes through and closes the sink', async () => { + const {stream, log} = recordingSink(); + + await withBodyWriter(stream, async writer => { + await writer.write(Uint8Array.from([1, 2])); + }); + + expect(log.chunks).toEqual([Uint8Array.from([1, 2])]); + expect(log.closed).toBe(true); + expect(log.abortReason).toBeUndefined(); + }); + + test('a close failure propagates unwrapped (RETRY-2)', async () => { + const {stream} = recordingSink({ + close: () => { + throw new Error('CLOSE FAILED'); + }, + }); + + const error = await rejection( + withBodyWriter(stream, () => Promise.resolve()), + ); + + expect(error.message).toBe('CLOSE FAILED'); + }); +}); + +describe('withBodyWriter failure path (RECOV-12, RETRY-2)', () => { + test('aborts the sink with the primary failure and rethrows it', async () => { + const {stream, log} = recordingSink(); + const primary = new Error('SOCKET GONE'); + + const error = await rejection( + withBodyWriter(stream, () => Promise.reject(primary)), + ); + + expect(error).toBe(primary); + expect(log.abortReason).toBe(primary); + expect(log.closed).toBe(false); + }); + + test('an abort() that itself rejects does not displace the primary failure', async () => { + const {stream} = recordingSink({ + abort: () => { + throw new Error('ABORT FAILED'); + }, + }); + const primary = new Error('SOCKET GONE'); + + const error = await rejection( + withBodyWriter(stream, () => Promise.reject(primary)), + ); + + expect(error).toBe(primary); + }); + + test('aborting an already-errored stream still surfaces the primary failure', async () => { + // The sink's own write() poisons the stream, so abort() runs against a stream that is already + // errored -- the case the naive `finally { close() }` shape turns into a bogus TypeError. + const {stream} = recordingSink({ + write: () => { + throw new Error('SINK EXPLODED'); + }, + }); + + const error = await rejection( + withBodyWriter(stream, async writer => { + await writer.write(Uint8Array.from([1])); + }), + ); + + expect(error.message).toBe('SINK EXPLODED'); + }); +});