From 19366ae8edbca98c7cafb10b61e3085049db43a9 Mon Sep 17 00:00:00 2001 From: Nameless 7778777 <7778777@7778777.online> Date: Wed, 12 Aug 2026 12:29:57 +0200 Subject: [PATCH] Fix binary response corruption in plugin fetch() pluginFetch() always did response.text() before relaying the body to a plugin's worker, which silently corrupts any binary payload (there was no way to fetch one intact). Read raw bytes and base64-encode them for transport instead, regardless of content type - PluginResponse gains an arrayBuffer() accessor alongside the existing text()/json(). Also adds a response size cap (100MB) that didn't exist before. Co-Authored-By: Claude Sonnet 5 --- impro-plugin/docs/docs.md | 20 +++++-- impro-plugin/main.d.ts | 18 ++++-- impro-plugin/main.js | 58 ++++++++++++++++--- src/js/plugins/pluginRequests.js | 42 +++++++++++++- .../unit/specs/plugins/pluginRequests.test.js | 38 +++++++++++- 5 files changed, 153 insertions(+), 23 deletions(-) diff --git a/impro-plugin/docs/docs.md b/impro-plugin/docs/docs.md index 3140af25..579a9c3e 100644 --- a/impro-plugin/docs/docs.md +++ b/impro-plugin/docs/docs.md @@ -1268,9 +1268,11 @@ Fetch a raw repo record by `(repo, collection, rkey)`. ### PluginResponse -Response returned from [fetch](#fetch). Body is buffered by the host and -exposed as text or parsed JSON. `status`, `ok`, and `headers` (a `Map`) -mirror the underlying HTTP response. +Response returned from [fetch](#fetch). The host always buffers and +base64-encodes the raw response bytes for transport (so binary bodies +survive intact), and this class decodes that on demand depending on +which accessor is called. `status`, `ok`, and `headers` (a `Map`) mirror +the underlying HTTP response. #### Properties @@ -1282,6 +1284,16 @@ mirror the underlying HTTP response. #### Methods +##### arrayBuffer() + +> **arrayBuffer**(): `Promise`\<`ArrayBuffer`\> + +Resolves with the raw response bytes. + +###### Returns + +`Promise`\<`ArrayBuffer`\> + ##### json() > **json**(): `Promise`\<`unknown`\> @@ -1296,7 +1308,7 @@ Resolves with the response body parsed as JSON. > **text**(): `Promise`\<`string`\> -Resolves with the response body as a string. +Resolves with the response body decoded as UTF-8 text. ###### Returns diff --git a/impro-plugin/main.d.ts b/impro-plugin/main.d.ts index fe60f99b..6cc0f6bb 100644 --- a/impro-plugin/main.d.ts +++ b/impro-plugin/main.d.ts @@ -299,9 +299,11 @@ export class App { showMoreLikeThis(postUri: string, feedUri: string): Promise; } /** - * Response returned from {@link fetch}. Body is buffered by the host and - * exposed as text or parsed JSON. `status`, `ok`, and `headers` (a `Map`) - * mirror the underlying HTTP response. + * Response returned from {@link fetch}. The host always buffers and + * base64-encodes the raw response bytes for transport (so binary bodies + * survive intact), and this class decodes that on demand depending on + * which accessor is called. `status`, `ok`, and `headers` (a `Map`) mirror + * the underlying HTTP response. */ export class PluginResponse { /** @@ -316,7 +318,12 @@ export class PluginResponse { /** @type {Map} */ headers: Map; /** - * Resolves with the response body as a string. + * Resolves with the raw response bytes. + * @returns {Promise} + */ + arrayBuffer(): Promise; + /** + * Resolves with the response body decoded as UTF-8 text. * @returns {Promise} */ text(): Promise; @@ -1425,7 +1432,8 @@ export type HostEventMessage = { */ export type HostMessage = HostCallMessage | HostResultMessage | HostEventMessage; /** - * {@internal} The host's reply to a proxied {@link fetch}. + * {@internal} The host's reply to a proxied {@link fetch}. `body` is + * always the raw response bytes, base64-encoded (see {@link PluginResponse}). */ export type SerializedFetchResponse = { status: number; diff --git a/impro-plugin/main.js b/impro-plugin/main.js index 4ed4f3cb..2e643415 100644 --- a/impro-plugin/main.js +++ b/impro-plugin/main.js @@ -553,12 +553,44 @@ function serializeFetchInit(init) { } /** - * Response returned from {@link fetch}. Body is buffered by the host and - * exposed as text or parsed JSON. `status`, `ok`, and `headers` (a `Map`) - * mirror the underlying HTTP response. + * @param {Uint8Array} bytes + * @returns {string} + */ +function bytesToBase64(bytes) { + // Encoded in fixed-size chunks rather than one + // String.fromCharCode(...bytes) call, which risks "too many + // arguments"/stack errors once bytes gets into the megabytes. + const CHUNK_SIZE = 0x8000; + let binary = ""; + for (let i = 0; i < bytes.length; i += CHUNK_SIZE) { + binary += String.fromCharCode(...bytes.subarray(i, i + CHUNK_SIZE)); + } + return btoa(binary); +} + +/** + * @param {string} base64 + * @returns {ArrayBuffer} + */ +function base64ToArrayBuffer(base64) { + const binary = atob(base64); + const bytes = new Uint8Array(binary.length); + for (let i = 0; i < binary.length; i++) { + bytes[i] = binary.charCodeAt(i); + } + return bytes.buffer; +} + +/** + * Response returned from {@link fetch}. The host always buffers and + * base64-encodes the raw response bytes for transport (so binary bodies + * survive intact), and this class decodes that on demand depending on + * which accessor is called. `status`, `ok`, and `headers` (a `Map`) mirror + * the underlying HTTP response. */ export class PluginResponse { - #body; + /** @type {string} base64-encoded raw response bytes */ + #bodyBase64; /** * @internal * @param {SerializedFetchResponse} response @@ -570,21 +602,28 @@ export class PluginResponse { this.ok = ok; /** @type {Map} */ this.headers = new Map(Object.entries(headers ?? {})); - this.#body = body; + this.#bodyBase64 = body; + } + /** + * Resolves with the raw response bytes. + * @returns {Promise} + */ + async arrayBuffer() { + return base64ToArrayBuffer(this.#bodyBase64); } /** - * Resolves with the response body as a string. + * Resolves with the response body decoded as UTF-8 text. * @returns {Promise} */ async text() { - return this.#body; + return new TextDecoder().decode(await this.arrayBuffer()); } /** * Resolves with the response body parsed as JSON. * @returns {Promise} */ async json() { - return JSON.parse(this.#body); + return JSON.parse(await this.text()); } } @@ -2018,7 +2057,8 @@ export class VirtualEl { * @typedef {HostCallMessage | HostResultMessage | HostEventMessage} HostMessage * {@internal} Anything the host may post to this worker. * @typedef {{ status: number, ok: boolean, headers: Record, body: string }} SerializedFetchResponse - * {@internal} The host's reply to a proxied {@link fetch}. + * {@internal} The host's reply to a proxied {@link fetch}. `body` is + * always the raw response bytes, base64-encoded (see {@link PluginResponse}). * @typedef {{ method?: string, headers?: Record, body?: string }} SerializedFetchInit * {@internal} A {@link PluginFetchInit} with its headers flattened for transfer. * @typedef {Record} SerializedRichTextToken diff --git a/src/js/plugins/pluginRequests.js b/src/js/plugins/pluginRequests.js index 8d104929..c7873fff 100644 --- a/src/js/plugins/pluginRequests.js +++ b/src/js/plugins/pluginRequests.js @@ -4,6 +4,32 @@ const ALLOWED_METHODS = ["GET", "POST", "PUT", "PATCH", "DELETE", "HEAD"]; const FORBIDDEN_HEADERS = ["authorization", "cookie"]; const MAX_BODY_CHARS = 1_000_000; +// Response bytes are buffered into memory whole (no streaming to the +// worker), and base64-encoded for transport - generous enough for a +// legitimately large response, but still bounded so a plugin can't be +// handed an unbounded download. +export const MAX_RESPONSE_BYTES = 100_000_000; + +// Encodes in fixed-size chunks rather than String.fromCharCode(...bytes) in +// one call, which risks "too many arguments"/stack errors once bytes gets +// into the tens of millions of entries this is sized for. +export function bytesToBase64(bytes) { + const CHUNK_SIZE = 0x8000; + let binary = ""; + for (let i = 0; i < bytes.length; i += CHUNK_SIZE) { + binary += String.fromCharCode(...bytes.subarray(i, i + CHUNK_SIZE)); + } + return btoa(binary); +} + +export function base64ToArrayBuffer(base64) { + const binary = atob(base64); + const bytes = new Uint8Array(binary.length); + for (let i = 0; i < binary.length; i++) { + bytes[i] = binary.charCodeAt(i); + } + return bytes.buffer; +} export async function pluginFetch( permissions, @@ -21,12 +47,24 @@ export async function pluginFetch( mode: "cors", referrerPolicy: "no-referrer", }); - const bodyText = await response.text(); + const bodyBuffer = await response.arrayBuffer(); + if (bodyBuffer.byteLength > MAX_RESPONSE_BYTES) { + throw new Error( + `fetch response too large (${bodyBuffer.byteLength} bytes, max ${MAX_RESPONSE_BYTES})`, + ); + } + // Always base64, regardless of content type: this is the one encoding + // that survives the postMessage/structured-clone hop to the plugin + // worker byte-for-byte, whether the response is JSON, HTML, or a binary + // asset. PluginResponse (impro-plugin/main.js) decodes it back into + // text/JSON/raw bytes on the plugin side depending on which accessor is + // called. + const bodyBase64 = bytesToBase64(new Uint8Array(bodyBuffer)); return { status: response.status, ok: response.ok, headers: filterResponseHeaders(response.headers, ["content-type"]), - body: bodyText, + body: bodyBase64, }; } diff --git a/tests/unit/specs/plugins/pluginRequests.test.js b/tests/unit/specs/plugins/pluginRequests.test.js index 42170dc2..4663987b 100644 --- a/tests/unit/specs/plugins/pluginRequests.test.js +++ b/tests/unit/specs/plugins/pluginRequests.test.js @@ -1,6 +1,6 @@ import { describe, it } from "node:test"; import assert from "node:assert/strict"; -import { pluginFetch } from "/js/plugins/pluginRequests.js"; +import { pluginFetch, MAX_RESPONSE_BYTES } from "/js/plugins/pluginRequests.js"; function makePermissions(patterns) { return { fetch: patterns }; @@ -16,12 +16,20 @@ function makeFakeFetch({ status = 200, body = "", headers = {} } = {}) { headers: { get: (name) => headers[name.toLowerCase()] ?? null, }, - text: async () => body, + arrayBuffer: async () => new TextEncoder().encode(body).buffer, }; }; return { fakeFetch, calls }; } +// pluginFetch now always base64-encodes the response body for transport +// (see pluginRequests.js) so it can carry binary payloads, not just text - +// tests that care about the actual body content decode it back rather than +// comparing against raw text. +function decodeBody(base64Body) { + return Buffer.from(base64Body, "base64").toString("utf8"); +} + async function expectRejection(fn, includes) { let threw = false; try { @@ -323,6 +331,30 @@ describe("response shape", () => { ); assert.deepEqual(result.status, 404); assert.deepEqual(result.ok, false); - assert.deepEqual(result.body, "nope"); + assert.deepEqual(decodeBody(result.body), "nope"); + }); +}); + +describe("response size", () => { + it("rejects a response over the byte cap", async () => { + // The cap check only reads .byteLength before any bytes are touched, so + // this fakes an over-limit length without actually allocating that much + // memory in the test. + const fakeFetch = async () => ({ + status: 200, + ok: true, + headers: { get: () => null }, + arrayBuffer: async () => ({ byteLength: MAX_RESPONSE_BYTES + 1 }), + }); + await expectRejection( + () => + pluginFetch( + makePermissions(["https://api.example.com/*"]), + "https://api.example.com/x", + {}, + fakeFetch, + ), + "too large", + ); }); }); -- 2.51.2