From b666a2e7f29f407851ff79e3b7d8b08a12b4d3ff Mon Sep 17 00:00:00 2001 From: Yinon Burgansky Date: Tue, 24 Feb 2026 13:59:17 +0200 Subject: [PATCH] fix: prevent infinite loop from unexpected server response (#1898) - Fix a regression introduced in SolidStart v1.3.0 that could cause an infinite loop when a server function returns unexpected response (for example, S3/XML error responses). - Prevents the browser from freezing or crashing when encountering such responses. - Add unit tests to prevent this regression from returning. - Tests exposed that serialization drains remaining chunks in the background and that async errors from that drain were not propagated to an end user error boundary/handler. Add a temporary catch-all in the test suite to suppress unhandled rejections until a proper error-propagation solution is implemented. --- .changeset/thick-dingos-joke.md | 5 ++ .../start/src/runtime/serialization.test.ts | 88 +++++++++++++++++++ packages/start/src/runtime/serialization.ts | 4 + 3 files changed, 97 insertions(+) create mode 100644 .changeset/thick-dingos-joke.md create mode 100644 packages/start/src/runtime/serialization.test.ts diff --git a/.changeset/thick-dingos-joke.md b/.changeset/thick-dingos-joke.md new file mode 100644 index 000000000..9f6153628 --- /dev/null +++ b/.changeset/thick-dingos-joke.md @@ -0,0 +1,5 @@ +--- +"@solidjs/start": patch +--- + +Fix a regression introduced in SolidStart v1.3.0 that could cause an infinite loop when a server function returns unexpected response (for example, S3/XML error responses). diff --git a/packages/start/src/runtime/serialization.test.ts b/packages/start/src/runtime/serialization.test.ts new file mode 100644 index 000000000..364dab538 --- /dev/null +++ b/packages/start/src/runtime/serialization.test.ts @@ -0,0 +1,88 @@ +import { describe, expect, it, beforeEach, afterEach } from "vitest"; +import { deserializeJSONStream, deserializeJSStream } from "./serialization"; + +const encoder = new TextEncoder(); + +function makeChunk(dataStr: string, declaredBytes?: number): Uint8Array { + const data = encoder.encode(dataStr); + const bytes = declaredBytes ?? data.length; + const baseHex = bytes.toString(16).padStart(8, "0"); + const head = encoder.encode(`;0x${baseHex};`); + const chunk = new Uint8Array(head.length + data.length); + chunk.set(head); + chunk.set(data, head.length); + return chunk; +} + +function streamFromChunks(chunks: Uint8Array[]) { + return new ReadableStream({ + start(controller) { + for (const c of chunks) controller.enqueue(c); + controller.close(); + }, + }); +} + +function responseWithChunks(chunks: Uint8Array[] | null) { + if (chunks === null) return new Response(null); + return new Response(streamFromChunks(chunks)); +} + +const cases = [ + { name: "deserializeJSONStream", call: (r: Response) => deserializeJSONStream(r) }, + { name: "deserializeJSStream", call: (r: Response) => deserializeJSStream("server-fn:0", r) }, +]; + +describe("Serialization negative testing (unhappy paths)", () => { + // TODO: Serialization drains remaining chunks in the background for performance and + // its async errors aren't propagated to a designated error boundary. + // This is a temporary catch-all to avoid unhandled rejections in this test suite until + // we have a better solution for handling async errors in serialization. + const _unhandledRejectionHandler = (reason: any, promise?: Promise) => { + // eslint-disable-next-line no-console + console.error("Unhandled rejection (ignored) in serialization.test:", reason, promise); + }; + + // Install immediately and retain for the duration of this test file. + beforeEach(() => { + process.on("unhandledRejection", _unhandledRejectionHandler); + }); + + afterEach(async () => { + // Wait for any pending microtasks to allow background processes to complete + await new Promise(resolve => setTimeout(resolve, 0)); + process.off("unhandledRejection", _unhandledRejectionHandler); + }); + for (const fn of cases) { + it(`${fn.name} throws on missing body`, async () => { + await expect(fn.call(responseWithChunks(null))).rejects.toThrow("missing body"); + }); + + it(`${fn.name} throws on plain XML response`, async () => { + const xml = 'AccessDeniedAccess Denied'; + const chunk = encoder.encode(xml); + const resp = new Response(new ReadableStream({ + start(controller) { + controller.enqueue(chunk); + controller.close(); + }, + })); + await expect(fn.call(resp)).rejects.toThrow(); + }); + + it(`${fn.name} throws Malformed server function stream when header larger than provided bytes`, async () => { + const chunk = makeChunk("bad", 16); // declare more than actual + await expect(fn.call(responseWithChunks([chunk]))).rejects.toThrow("Malformed server function stream."); + }); + + it(`${fn.name} throws Malformed server function stream when header smaller than provided bytes`, async () => { + const chunk = makeChunk("bad", 2); // declare less than actual + await expect(fn.call(responseWithChunks([chunk]))).rejects.toThrow(); + }); + + it(`${fn.name} throws on valid header but invalid JSON body`, async () => { + const chunk = makeChunk("not-a-json"); + await expect(fn.call(responseWithChunks([chunk]))).rejects.toThrow(); + }); + } +}); diff --git a/packages/start/src/runtime/serialization.ts b/packages/start/src/runtime/serialization.ts index b206a6830..824f866c7 100644 --- a/packages/start/src/runtime/serialization.ts +++ b/packages/start/src/runtime/serialization.ts @@ -159,6 +159,10 @@ class SerovalChunkReader { // deserialize the data const head = new TextDecoder().decode(this.buffer.subarray(1, 11)); const bytes = Number.parseInt(head, 16); // ;0x00000000; + if (Number.isNaN(bytes)) { + throw new Error("Malformed server function stream header."); + } + // Check if the buffer has enough bytes to be parsed while (bytes > this.buffer.length - 12) { // If it's not enough, and the reader is done