mirror of
https://github.com/paperclipai/paperclip.git
synced 2026-10-06 10:48:12 +02:00
Fix apiCompression corrupting and dropping Better Auth responses for gzip clients (#9381)
## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work > - Its server fronts every API route — including Better Auth sign-in — with Express middleware, and #9190 added an `apiCompression` middleware that gzips JSON responses over 1KB > - That middleware buffers `res.write()` chunks with `String(chunk)`, but Better Auth (via better-call) streams `Uint8Array` chunks and commits headers with `writeHead()` before streaming > - `String(Uint8Array)` serializes the body to comma-separated decimal bytes (~3.4x inflation), and once the inflated body crossed the 1KB threshold, `setHeader()` threw `ERR_HTTP_HEADERS_SENT` and the catch handler destroyed the socket > - Every real browser sends `Accept-Encoding: gzip`, so sign-in returned zero bytes (`net::ERR_EMPTY_RESPONSE` / "Failed to fetch"), while curl without `Accept-Encoding` worked — making the bug easy to misdiagnose as a client or network issue > - This pull request makes the middleware byte-safe for `Uint8Array` chunks, passes through responses whose headers are already committed, and falls back to the uncompressed body instead of destroying the connection when compression fails > - The benefit is that browser sign-in (and any other streamed binary-chunk response) works again for gzip-accepting clients, with regression tests locking in all three behaviors ## Linked Issues or Issue Description Refs #9190 (introduced the `apiCompression` middleware). No public GitHub issue exists; bug description: - **What happened:** Sign-in from any real browser failed with `net::ERR_EMPTY_RESPONSE` / "Failed to fetch". The server logged `ERR_HTTP_HEADERS_SENT` from the compression middleware and destroyed the response socket, so zero bytes reached the client. - **Expected:** `/api/auth/*` responses are delivered intact regardless of the client's `Accept-Encoding`. - **Steps to reproduce:** Run the server with API compression active, open the web UI in a browser (which sends `Accept-Encoding: gzip`), and attempt email/password sign-in. The auth response body exceeds ~300 bytes, so after the ~3.4x stringification inflation it crosses the 1024-byte compression threshold and the response is destroyed. `curl` without `Accept-Encoding` succeeds against the same server. - **Scope:** Any route that streams `Uint8Array` chunks and/or commits headers via `writeHead()` before writing — in practice all Better Auth routes served through better-call. ## What Changed - `server/src/middleware/api-compression.ts`: - Buffer `res.write()` chunks with a `toBodyBuffer()` helper that converts `Uint8Array`/`ArrayBuffer` views via `Buffer.from()` instead of `String()`, so binary chunks are preserved byte-for-byte. - Pass responses through untouched once headers are already sent (`writeHead()`-style streaming), since compression headers can no longer be set at that point. - On any compression failure, write the original uncompressed body instead of calling `res.destroy()`, so clients get a valid (just uncompressed) response rather than a dropped connection. - `server/src/__tests__/api-compression.test.ts`: three new regression tests — small `writeHead`+`Uint8Array` responses are delivered byte-for-byte, large ones no longer drop the connection, and `Uint8Array` JSON bodies gzip without corruption (includes `/api/auth-bridge` and `/api/uint8-json` test routes mirroring better-call's streaming pattern). ## Verification - `cd server && pnpm vitest run src/__tests__/api-compression.test.ts` — 10/10 passing (7 pre-existing + 3 new regression tests). - Manual: with the fix, browser sign-in against a dev instance succeeds for gzip-accepting clients; before the fix the same request returned `net::ERR_EMPTY_RESPONSE`. ## Risks - Low risk. The middleware still compresses large text/JSON responses exactly as before; the changes only affect paths that previously produced corrupted or destroyed responses. - Behavioral shift: responses whose headers were already committed are now delivered uncompressed instead of being (incorrectly) buffered — this is strictly less surprising than the previous corrupted output. - Failure-path shift: a compression error now yields an uncompressed 200 response instead of a dropped connection. ## Model Used - Claude Fable 5 (`claude-fable-5`, Anthropic), extended thinking enabled, running via Claude Code / Paperclip agent harness with tool use (shell, file edit, test execution). ## Checklist - [x] I have included a thinking path that traces from project context to this change - [x] I have specified the model used (with version and capability details) - [x] I have checked ROADMAP.md and confirmed this PR does not duplicate planned core work - [x] I have searched GitHub for duplicate or related PRs and linked them above - [x] I have either (a) linked existing issues with `Fixes: #` / `Closes #` / `Refs #` OR (b) described the issue in-PR following the relevant issue template - [x] I have not referenced internal/instance-local Paperclip issues or links (only public GitHub `#NNN` / `github.com/paperclipai/paperclip` URLs) - [x] My branch name describes the change (e.g. `docs/...`, `fix/...`) and contains no internal Paperclip ticket id or instance-derived details - [x] I have run tests locally and they pass - [x] I have added or updated tests where applicable - [x] I have updated relevant documentation to reflect my changes - [x] I have considered and documented any risks above - [x] All Paperclip CI gates are green - [x] Greptile is 5/5 with no open P2s, recommendations, or follow-ups - [x] I will address all Greptile and reviewer comments before requesting merge 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
This commit is contained in:
1 parent
1e8ede4e1e
commit
23f34491e2
2 files changed
+78
-3
No files matched your search
@@ -105,6 +105,27 @@ function buildApp() {
|
||||
res.write(chunk.slice(0, chunk.length / 2));
|
||||
res.end(chunk.slice(chunk.length / 2));
|
||||
});
|
||||
// Mirrors better-call's setResponse (Better Auth sign-in/sign-up): headers
|
||||
// are committed with writeHead() first, then the web-stream body arrives as
|
||||
// Uint8Array chunks.
|
||||
app.get("/api/auth-bridge", (req, res) => {
|
||||
const body = JSON.stringify({
|
||||
token: "t".repeat(Number(req.query.pad ?? 0)),
|
||||
user: { name: "Dotta", email: "dotta@example.test" },
|
||||
});
|
||||
res.setHeader("content-type", "application/json");
|
||||
res.setHeader("set-cookie", "workspace.session_token=abc; Max-Age=604800; Path=/; HttpOnly; SameSite=Lax");
|
||||
res.writeHead(200);
|
||||
const bytes = new TextEncoder().encode(body);
|
||||
res.write(bytes.subarray(0, 16));
|
||||
res.write(bytes.subarray(16));
|
||||
res.end();
|
||||
});
|
||||
app.get("/api/uint8-json", (req, res) => {
|
||||
const body = JSON.stringify(issueListFixture(Number(req.query.count ?? 1)));
|
||||
res.setHeader("Content-Type", "application/json; charset=utf-8");
|
||||
res.end(new TextEncoder().encode(body));
|
||||
});
|
||||
return app;
|
||||
}
|
||||
|
||||
@@ -184,4 +205,37 @@ describe("API compression middleware", () => {
|
||||
expect(res.headers["content-encoding"]).toBeUndefined();
|
||||
expect(JSON.parse(res.body.toString("utf8"))).toHaveLength(500);
|
||||
});
|
||||
|
||||
it("delivers small writeHead+Uint8Array auth responses byte-for-byte", async () => {
|
||||
const res = await requestRaw(buildApp(), "/api/auth-bridge", {
|
||||
"accept-encoding": "gzip, deflate",
|
||||
});
|
||||
|
||||
expect(res.statusCode).toBe(200);
|
||||
expect(res.headers["set-cookie"]).toBeDefined();
|
||||
expect(res.headers["content-encoding"]).toBeUndefined();
|
||||
expect(JSON.parse(res.body.toString("utf8")).user.email).toBe("dotta@example.test");
|
||||
});
|
||||
|
||||
it("does not drop the connection for large writeHead+Uint8Array auth responses", async () => {
|
||||
const res = await requestRaw(buildApp(), "/api/auth-bridge?pad=2000", {
|
||||
"accept-encoding": "gzip, deflate",
|
||||
});
|
||||
|
||||
expect(res.statusCode).toBe(200);
|
||||
expect(res.headers["content-encoding"]).toBeUndefined();
|
||||
const parsed = JSON.parse(res.body.toString("utf8"));
|
||||
expect(parsed.token).toHaveLength(2000);
|
||||
expect(parsed.user.email).toBe("dotta@example.test");
|
||||
});
|
||||
|
||||
it("compresses large Uint8Array bodies without corrupting them", async () => {
|
||||
const res = await requestRaw(buildApp(), "/api/uint8-json?count=500", {
|
||||
"accept-encoding": "gzip",
|
||||
});
|
||||
|
||||
expect(res.statusCode).toBe(200);
|
||||
expect(res.headers["content-encoding"]).toBe("gzip");
|
||||
expect(JSON.parse(gunzipSync(res.body).toString("utf8"))).toHaveLength(500);
|
||||
});
|
||||
});
|
||||
@@ -75,6 +75,10 @@ function shouldPassthroughWrite(res: Parameters<RequestHandler>[1]): boolean {
|
||||
const contentType = res.getHeader("Content-Type");
|
||||
const alreadyEncoded = res.hasHeader("Content-Encoding") && String(res.getHeader("Content-Encoding")).toLowerCase() !== "identity";
|
||||
return (
|
||||
// writeHead() may already have committed the response head (better-call
|
||||
// does this before streaming the body); headers can no longer change, so
|
||||
// buffering for compression would only risk corrupting the stream.
|
||||
res.headersSent ||
|
||||
alreadyEncoded ||
|
||||
!statusAllowsBody(res.statusCode) ||
|
||||
shouldSkipForCacheControl(res.getHeader("Cache-Control")) ||
|
||||
@@ -97,6 +101,15 @@ function weakenStrongEtag(res: Parameters<RequestHandler>[1]): void {
|
||||
res.setHeader("ETag", weaken(String(etag)));
|
||||
}
|
||||
|
||||
function toBodyBuffer(chunk: unknown, encoding: BufferEncoding | undefined): Buffer {
|
||||
if (Buffer.isBuffer(chunk)) return chunk;
|
||||
// Handlers bridged from web Response streams (e.g. Better Auth via
|
||||
// better-call) write Uint8Array chunks; String(chunk) would serialize them
|
||||
// as comma-separated byte values and corrupt the body.
|
||||
if (chunk instanceof Uint8Array) return Buffer.from(chunk.buffer, chunk.byteOffset, chunk.byteLength);
|
||||
return Buffer.from(String(chunk), encoding);
|
||||
}
|
||||
|
||||
function normalizeEndArgs(args: unknown[]): {
|
||||
chunk: unknown;
|
||||
encoding: BufferEncoding | undefined;
|
||||
@@ -160,7 +173,7 @@ export function apiCompression(options: ApiCompressionOptions = {}): RequestHand
|
||||
return originalWrite(chunk as never, encodingOrCallback as never, callback as never);
|
||||
}
|
||||
if (chunk !== undefined) {
|
||||
chunks.push(Buffer.isBuffer(chunk) ? chunk : Buffer.from(String(chunk), typeof encodingOrCallback === "string" ? encodingOrCallback : undefined));
|
||||
chunks.push(toBodyBuffer(chunk, typeof encodingOrCallback === "string" ? encodingOrCallback : undefined));
|
||||
}
|
||||
const writeCallback = typeof encodingOrCallback === "function" ? encodingOrCallback : callback;
|
||||
if (writeCallback) writeCallbacks.push(() => writeCallback(null));
|
||||
@@ -171,13 +184,14 @@ export function apiCompression(options: ApiCompressionOptions = {}): RequestHand
|
||||
restore();
|
||||
const { chunk, encoding, callback } = normalizeEndArgs(args);
|
||||
if (chunk !== undefined) {
|
||||
chunks.push(Buffer.isBuffer(chunk) ? chunk : Buffer.from(String(chunk), encoding));
|
||||
chunks.push(toBodyBuffer(chunk, encoding));
|
||||
}
|
||||
|
||||
const body = Buffer.concat(chunks);
|
||||
const alreadyEncoded = res.hasHeader("Content-Encoding") && String(res.getHeader("Content-Encoding")).toLowerCase() !== "identity";
|
||||
const shouldCompress =
|
||||
!passthrough &&
|
||||
!res.headersSent &&
|
||||
!alreadyEncoded &&
|
||||
statusAllowsBody(res.statusCode) &&
|
||||
body.length >= thresholdBytes &&
|
||||
@@ -204,7 +218,14 @@ export function apiCompression(options: ApiCompressionOptions = {}): RequestHand
|
||||
originalEnd(compressed, callback);
|
||||
for (const writeCallback of writeCallbacks) writeCallback();
|
||||
} catch (error) {
|
||||
res.destroy(error instanceof Error ? error : new Error(String(error)));
|
||||
// Compression is best-effort: never turn a healthy response into a
|
||||
// dropped connection. Send the original body if the head allows it.
|
||||
try {
|
||||
originalEnd(body, callback);
|
||||
for (const writeCallback of writeCallbacks) writeCallback();
|
||||
} catch {
|
||||
res.destroy(error instanceof Error ? error : new Error(String(error)));
|
||||
}
|
||||
}
|
||||
})();
|
||||
return res;
|
||||
|
||||
Reference in new issue
Block a user