Harden WebSocket client decompression #25724
Conversation
|
Updated 8:36 PM PT - Dec 27th, 2025
❌ Your commit
🧪 To try this PR locally: bunx bun-pr 25724That installs a local version of the PR into your bun-25724 --bun |
WalkthroughAdds a 128 MB decompressed-size limit to permessage-deflate, introduces a TooLarge decompression error when exceeded, maps TooLarge to ErrorCode.message_too_big in the client, and adds a test that sends a 150 MB compressed frame to verify the client closes with code 1009 and reason "Message too big". Changes
Suggested reviewers
Pre-merge checks❌ Failed checks (1 inconclusive)
✅ Passed checks (1 passed)
📜 Recent review detailsConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Disabled knowledge base sources:
📒 Files selected for processing (3)
🧰 Additional context used📓 Path-based instructions (5)src/**/*.zig📄 CodeRabbit inference engine (src/CLAUDE.md)
Files:
**/*.zig📄 CodeRabbit inference engine (CLAUDE.md)
Files:
test/**/*.test.{ts,js,jsx,tsx,mjs,cjs}📄 CodeRabbit inference engine (test/CLAUDE.md)
Files:
**/*.test.ts?(x)📄 CodeRabbit inference engine (CLAUDE.md)
Files:
test/**/*.test.ts?(x)📄 CodeRabbit inference engine (CLAUDE.md)
Files:
🧠 Learnings (22)📚 Learning: 2025-11-10T00:57:09.173ZApplied to files:
📚 Learning: 2025-11-24T18:37:30.259ZApplied to files:
📚 Learning: 2025-10-18T05:23:24.403ZApplied to files:
📚 Learning: 2025-09-03T01:30:58.001ZApplied to files:
📚 Learning: 2025-11-24T18:36:59.706ZApplied to files:
📚 Learning: 2025-10-19T04:55:33.099ZApplied to files:
📚 Learning: 2025-09-20T00:58:38.042ZApplied to files:
📚 Learning: 2025-11-24T18:37:30.259ZApplied to files:
📚 Learning: 2025-12-16T00:21:32.179ZApplied to files:
📚 Learning: 2025-11-24T18:37:30.259ZApplied to files:
📚 Learning: 2025-11-24T18:37:30.259ZApplied to files:
📚 Learning: 2025-11-24T18:37:30.259ZApplied to files:
📚 Learning: 2025-12-05T19:28:39.534ZApplied to files:
📚 Learning: 2025-12-16T00:21:32.179ZApplied to files:
📚 Learning: 2025-11-24T18:37:30.259ZApplied to files:
📚 Learning: 2025-12-16T00:21:32.179ZApplied to files:
📚 Learning: 2025-11-24T18:37:30.259ZApplied to files:
📚 Learning: 2025-11-24T18:37:30.259ZApplied to files:
📚 Learning: 2025-10-08T13:48:02.430ZApplied to files:
📚 Learning: 2025-10-19T02:44:46.354ZApplied to files:
📚 Learning: 2025-11-08T04:06:33.198ZApplied to files:
📚 Learning: 2025-12-16T00:21:32.179ZApplied to files:
🔇 Additional comments (4)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Disabled knowledge base sources:
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (3)
src/http/websocket_client.zigsrc/http/websocket_client/WebSocketDeflate.zigtest/js/web/websocket/websocket-permessage-deflate-edge-cases.test.ts
🧰 Additional context used
📓 Path-based instructions (5)
test/**/*.test.{ts,js,jsx,tsx,mjs,cjs}
📄 CodeRabbit inference engine (test/CLAUDE.md)
test/**/*.test.{ts,js,jsx,tsx,mjs,cjs}: Usebun:testwith files that end in*.test.{ts,js,jsx,tsx,mjs,cjs}
Do not write flaky tests. Never wait for time to pass in tests; always wait for the condition to be met instead of using an arbitrary amount of time
Never use hardcoded port numbers in tests. Always useport: 0to get a random port
Prefer concurrent tests over sequential tests usingtest.concurrentordescribe.concurrentwhen multiple tests spawn processes or write files, unless it's very difficult to make them concurrent
When spawning Bun processes in tests, usebunExeandbunEnvfromharnessto ensure the same build of Bun is used and debug logging is silenced
Use-eflag for single-file tests when spawning Bun processes
UsetempDir()from harness to create temporary directories with files for multi-file tests instead of creating files manually
Prefer async/await over callbacks in tests
When callbacks must be used and it's just a single callback, usePromise.withResolversto create a promise that can be resolved or rejected from a callback
Do not set a timeout on tests. Bun already has timeouts
UseBuffer.alloc(count, fill).toString()instead of'A'.repeat(count)to create repetitive strings in tests, as ''.repeat is very slow in debug JavaScriptCore builds
Usedescribeblocks for grouping related tests
Always useawait usingorusingto ensure proper resource cleanup in tests for APIs like Bun.listen, Bun.connect, Bun.spawn, Bun.serve, etc
Always check exit codes and test error scenarios in error tests
Usedescribe.each()for parameterized tests
UsetoMatchSnapshot()for snapshot testing
UsebeforeAll(),afterEach(),beforeEach()for setup/teardown in tests
Track resources (servers, clients) in arrays for cleanup inafterEach()
Files:
test/js/web/websocket/websocket-permessage-deflate-edge-cases.test.ts
**/*.test.ts?(x)
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.test.ts?(x): Never usebun testdirectly - always usebun bd testto run tests with debug build changes
For single-file tests, prefer-eflag overtempDir
For multi-file tests, prefertempDirandBun.spawnover single-file tests
UsenormalizeBunSnapshotto normalize snapshot output of tests
Never write tests that check for 'panic', 'uncaught exception', or similar strings in test output
UsetempDirfromharnessto create temporary directories - do not usetmpdirSyncorfs.mkdtempSync
When spawning processes in tests, expect stdout before expecting exit code for more useful error messages on test failure
Do not write flaky tests - do not usesetTimeoutin tests; instead await the condition to be met
Verify tests fail withUSE_SYSTEM_BUN=1 bun test <file>and pass withbun bd test <file>- tests are invalid if they pass with USE_SYSTEM_BUN=1
Test files must end with.test.tsor.test.tsx
Avoid shell commands likefindorgrepin tests - use Bun's Glob and built-in tools instead
Files:
test/js/web/websocket/websocket-permessage-deflate-edge-cases.test.ts
test/**/*.test.ts?(x)
📄 CodeRabbit inference engine (CLAUDE.md)
Always use
port: 0in tests - do not hardcode ports or use custom random port number functions
Files:
test/js/web/websocket/websocket-permessage-deflate-edge-cases.test.ts
src/**/*.zig
📄 CodeRabbit inference engine (src/CLAUDE.md)
src/**/*.zig: Private fields in Zig are fully supported using the#prefix:struct { #foo: u32 };
Use decl literals in Zig for declaration initialization:const decl: Decl = .{ .binding = 0, .value = 0 };
Prefer@importat the bottom of the file (auto formatter will move them automatically)
Files:
src/http/websocket_client/WebSocketDeflate.zigsrc/http/websocket_client.zig
**/*.zig
📄 CodeRabbit inference engine (CLAUDE.md)
In Zig code, be careful with allocators and use defer for cleanup
Files:
src/http/websocket_client/WebSocketDeflate.zigsrc/http/websocket_client.zig
🧠 Learnings (2)
📚 Learning: 2025-10-18T05:23:24.403Z
Learnt from: theshadow27
Repo: oven-sh/bun PR: 23798
File: test/js/bun/telemetry-server.test.ts:91-100
Timestamp: 2025-10-18T05:23:24.403Z
Learning: In the Bun codebase, telemetry tests (test/js/bun/telemetry-*.test.ts) should focus on telemetry API behavior: configure/disable/isEnabled, callback signatures and invocation, request ID correlation, and error handling. HTTP protocol behaviors like status code normalization (e.g., 200 with empty body → 204) should be tested in HTTP server tests (test/js/bun/http/), not in telemetry tests. Keep separation of concerns: telemetry tests verify the telemetry API contract; HTTP tests verify HTTP semantics.
Applied to files:
test/js/web/websocket/websocket-permessage-deflate-edge-cases.test.ts
📚 Learning: 2025-11-10T00:57:09.173Z
Learnt from: franciscop
Repo: oven-sh/bun PR: 24514
File: src/bun.js/api/crypto/PasswordObject.zig:86-101
Timestamp: 2025-11-10T00:57:09.173Z
Learning: In Bun's Zig codebase (PasswordObject.zig), when validating the parallelism parameter for Argon2, the upper limit is set to 65535 (2^16 - 1) rather than using `std.math.maxInt(u24)` because the latter triggers Zig's truncation limit checks. The value 65535 is a practical upper bound that avoids compiler issues while being sufficient for thread parallelism use cases.
Applied to files:
src/http/websocket_client/WebSocketDeflate.zig
🧬 Code graph analysis (1)
test/js/web/websocket/websocket-permessage-deflate-edge-cases.test.ts (1)
src/bun.js/bindings/webcore/WebSocket.cpp (2)
WebSocket(179-187)WebSocket(189-221)
🔇 Additional comments (3)
src/http/websocket_client.zig (1)
234-242: LGTM: Error mapping for decompression size limit is correct.The new mapping from
error.TooLargetoErrorCode.message_too_bigproperly integrates with the existing error handling flow and aligns with the decompression size limit enforcement added in WebSocketDeflate.zig.src/http/websocket_client/WebSocketDeflate.zig (2)
79-80: LGTM: Maximum decompressed size constant is well-defined.The 16 MB limit is reasonable for preventing decompression bomb attacks while allowing legitimate large messages. The comment clearly indicates alignment with server defaults.
142-176: LGTM: Size limit enforcement is correctly implemented.The implementation properly:
- Extends the error set to include
TooLarge- Tracks the initial length to measure decompressed growth
- Checks the limit after libdeflate decompression (line 150-152)
- Incrementally checks during zlib inflate (line 173-176), enabling early detection
The use of
>rather than>=at the boundary allows exactly 16 MB, which appears intentional.
test/js/web/websocket/websocket-permessage-deflate-edge-cases.test.ts
Outdated
Show resolved
Hide resolved
robobun
force-pushed
the
claude/websocket-deflate-hardening
branch
from
0594650 to
5f8d2e4
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
test/js/web/websocket/websocket-permessage-deflate-edge-cases.test.ts (1)
204-319: Ensure TCP server and client cleanup on test failure.The TCP server cleanup at line 315 won't execute if the test fails or times out. Use
try/finallyto guarantee cleanup. Additionally, consider closing the WebSocket client explicitly in cleanup.🔎 Proposed fix for guaranteed cleanup
const port = await serverReady; + let client: WebSocket | null = null; + try { tcpServer.on("connection", socket => { let buffer = Buffer.alloc(0); // ... connection handler code unchanged ... }); // Connect with Bun's WebSocket client - const client = new WebSocket(`ws://localhost:${port}`); + client = new WebSocket(`ws://localhost:${port}`); const result = await new Promise<{ code: number; reason: string }>(resolve => { // ... handlers unchanged ... }); - tcpServer.close(); - // The connection should be closed with code 1009 (Message Too Big) expect(result.code).toBe(1009); expect(result.reason).toBe("Message too big"); + } finally { + if (client && client.readyState !== WebSocket.CLOSED) { + client.close(); + } + tcpServer.close(); + }
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Disabled knowledge base sources:
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (3)
src/http/websocket_client.zigsrc/http/websocket_client/WebSocketDeflate.zigtest/js/web/websocket/websocket-permessage-deflate-edge-cases.test.ts
🧰 Additional context used
📓 Path-based instructions (5)
src/**/*.zig
📄 CodeRabbit inference engine (src/CLAUDE.md)
src/**/*.zig: Private fields in Zig are fully supported using the#prefix:struct { #foo: u32 };
Use decl literals in Zig for declaration initialization:const decl: Decl = .{ .binding = 0, .value = 0 };
Prefer@importat the bottom of the file (auto formatter will move them automatically)
Files:
src/http/websocket_client.zigsrc/http/websocket_client/WebSocketDeflate.zig
**/*.zig
📄 CodeRabbit inference engine (CLAUDE.md)
In Zig code, be careful with allocators and use defer for cleanup
Files:
src/http/websocket_client.zigsrc/http/websocket_client/WebSocketDeflate.zig
test/**/*.test.{ts,js,jsx,tsx,mjs,cjs}
📄 CodeRabbit inference engine (test/CLAUDE.md)
test/**/*.test.{ts,js,jsx,tsx,mjs,cjs}: Usebun:testwith files that end in*.test.{ts,js,jsx,tsx,mjs,cjs}
Do not write flaky tests. Never wait for time to pass in tests; always wait for the condition to be met instead of using an arbitrary amount of time
Never use hardcoded port numbers in tests. Always useport: 0to get a random port
Prefer concurrent tests over sequential tests usingtest.concurrentordescribe.concurrentwhen multiple tests spawn processes or write files, unless it's very difficult to make them concurrent
When spawning Bun processes in tests, usebunExeandbunEnvfromharnessto ensure the same build of Bun is used and debug logging is silenced
Use-eflag for single-file tests when spawning Bun processes
UsetempDir()from harness to create temporary directories with files for multi-file tests instead of creating files manually
Prefer async/await over callbacks in tests
When callbacks must be used and it's just a single callback, usePromise.withResolversto create a promise that can be resolved or rejected from a callback
Do not set a timeout on tests. Bun already has timeouts
UseBuffer.alloc(count, fill).toString()instead of'A'.repeat(count)to create repetitive strings in tests, as ''.repeat is very slow in debug JavaScriptCore builds
Usedescribeblocks for grouping related tests
Always useawait usingorusingto ensure proper resource cleanup in tests for APIs like Bun.listen, Bun.connect, Bun.spawn, Bun.serve, etc
Always check exit codes and test error scenarios in error tests
Usedescribe.each()for parameterized tests
UsetoMatchSnapshot()for snapshot testing
UsebeforeAll(),afterEach(),beforeEach()for setup/teardown in tests
Track resources (servers, clients) in arrays for cleanup inafterEach()
Files:
test/js/web/websocket/websocket-permessage-deflate-edge-cases.test.ts
**/*.test.ts?(x)
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.test.ts?(x): Never usebun testdirectly - always usebun bd testto run tests with debug build changes
For single-file tests, prefer-eflag overtempDir
For multi-file tests, prefertempDirandBun.spawnover single-file tests
UsenormalizeBunSnapshotto normalize snapshot output of tests
Never write tests that check for 'panic', 'uncaught exception', or similar strings in test output
UsetempDirfromharnessto create temporary directories - do not usetmpdirSyncorfs.mkdtempSync
When spawning processes in tests, expect stdout before expecting exit code for more useful error messages on test failure
Do not write flaky tests - do not usesetTimeoutin tests; instead await the condition to be met
Verify tests fail withUSE_SYSTEM_BUN=1 bun test <file>and pass withbun bd test <file>- tests are invalid if they pass with USE_SYSTEM_BUN=1
Test files must end with.test.tsor.test.tsx
Avoid shell commands likefindorgrepin tests - use Bun's Glob and built-in tools instead
Files:
test/js/web/websocket/websocket-permessage-deflate-edge-cases.test.ts
test/**/*.test.ts?(x)
📄 CodeRabbit inference engine (CLAUDE.md)
Always use
port: 0in tests - do not hardcode ports or use custom random port number functions
Files:
test/js/web/websocket/websocket-permessage-deflate-edge-cases.test.ts
🧠 Learnings (19)
📚 Learning: 2025-11-10T00:57:09.173Z
Learnt from: franciscop
Repo: oven-sh/bun PR: 24514
File: src/bun.js/api/crypto/PasswordObject.zig:86-101
Timestamp: 2025-11-10T00:57:09.173Z
Learning: In Bun's Zig codebase (PasswordObject.zig), when validating the parallelism parameter for Argon2, the upper limit is set to 65535 (2^16 - 1) rather than using `std.math.maxInt(u24)` because the latter triggers Zig's truncation limit checks. The value 65535 is a practical upper bound that avoids compiler issues while being sufficient for thread parallelism use cases.
Applied to files:
src/http/websocket_client/WebSocketDeflate.zig
📚 Learning: 2025-09-03T01:30:58.001Z
Learnt from: markovejnovic
Repo: oven-sh/bun PR: 21728
File: test/js/valkey/valkey.test.ts:264-271
Timestamp: 2025-09-03T01:30:58.001Z
Learning: For test/js/valkey/valkey.test.ts PUB/SUB tests, avoid arbitrary sleeps and async-forEach. Instead, resolve a Promise from the subscriber callback when the expected number of messages is observed and await it with a bounded timeout (e.g., withTimeout + Promise.withResolvers) to account for Redis server→subscriber propagation.
Applied to files:
test/js/web/websocket/websocket-permessage-deflate-edge-cases.test.ts
📚 Learning: 2025-10-18T05:23:24.403Z
Learnt from: theshadow27
Repo: oven-sh/bun PR: 23798
File: test/js/bun/telemetry-server.test.ts:91-100
Timestamp: 2025-10-18T05:23:24.403Z
Learning: In the Bun codebase, telemetry tests (test/js/bun/telemetry-*.test.ts) should focus on telemetry API behavior: configure/disable/isEnabled, callback signatures and invocation, request ID correlation, and error handling. HTTP protocol behaviors like status code normalization (e.g., 200 with empty body → 204) should be tested in HTTP server tests (test/js/bun/http/), not in telemetry tests. Keep separation of concerns: telemetry tests verify the telemetry API contract; HTTP tests verify HTTP semantics.
Applied to files:
test/js/web/websocket/websocket-permessage-deflate-edge-cases.test.ts
📚 Learning: 2025-10-19T04:55:33.099Z
Learnt from: theshadow27
Repo: oven-sh/bun PR: 23798
File: test/js/bun/http/node-telemetry.test.ts:27-203
Timestamp: 2025-10-19T04:55:33.099Z
Learning: In test/js/bun/http/node-telemetry.test.ts and the Bun.telemetry._node_binding API, after the architecture refactor, the _node_binding interface only contains two methods: handleIncomingRequest(req, res) and handleWriteHead(res, statusCode). The handleRequestFinish hook and other lifecycle hooks were removed during simplification. Both current methods are fully tested.
Applied to files:
test/js/web/websocket/websocket-permessage-deflate-edge-cases.test.ts
📚 Learning: 2025-11-24T18:37:30.259Z
Learnt from: CR
Repo: oven-sh/bun PR: 0
File: test/CLAUDE.md:0-0
Timestamp: 2025-11-24T18:37:30.259Z
Learning: Applies to test/**/*.test.{ts,js,jsx,tsx,mjs,cjs} : Always use `await using` or `using` to ensure proper resource cleanup in tests for APIs like Bun.listen, Bun.connect, Bun.spawn, Bun.serve, etc
Applied to files:
test/js/web/websocket/websocket-permessage-deflate-edge-cases.test.ts
📚 Learning: 2025-12-16T00:21:32.179Z
Learnt from: CR
Repo: oven-sh/bun PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-16T00:21:32.179Z
Learning: Applies to test/**/*.test.ts?(x) : Always use `port: 0` in tests - do not hardcode ports or use custom random port number functions
Applied to files:
test/js/web/websocket/websocket-permessage-deflate-edge-cases.test.ts
📚 Learning: 2025-11-24T18:37:30.259Z
Learnt from: CR
Repo: oven-sh/bun PR: 0
File: test/CLAUDE.md:0-0
Timestamp: 2025-11-24T18:37:30.259Z
Learning: Applies to test/**/*.test.{ts,js,jsx,tsx,mjs,cjs} : Never use hardcoded port numbers in tests. Always use `port: 0` to get a random port
Applied to files:
test/js/web/websocket/websocket-permessage-deflate-edge-cases.test.ts
📚 Learning: 2025-11-24T18:37:30.259Z
Learnt from: CR
Repo: oven-sh/bun PR: 0
File: test/CLAUDE.md:0-0
Timestamp: 2025-11-24T18:37:30.259Z
Learning: Applies to test/**/*.test.{ts,js,jsx,tsx,mjs,cjs} : Track resources (servers, clients) in arrays for cleanup in `afterEach()`
Applied to files:
test/js/web/websocket/websocket-permessage-deflate-edge-cases.test.ts
📚 Learning: 2025-09-20T00:58:38.042Z
Learnt from: markovejnovic
Repo: oven-sh/bun PR: 22568
File: test/js/valkey/valkey.test.ts:561-564
Timestamp: 2025-09-20T00:58:38.042Z
Learning: For test/js/valkey/valkey.test.ts, do not comment on synchronous throw assertions for async Redis methods (like ctx.redis.set(), ctx.redis.unsubscribe(), etc.) - Bun's Redis client implementation differs from Node.js and can throw synchronously even for async methods. The maintainer has explicitly requested to stop looking at this error pattern.
Applied to files:
test/js/web/websocket/websocket-permessage-deflate-edge-cases.test.ts
📚 Learning: 2025-11-24T18:37:30.259Z
Learnt from: CR
Repo: oven-sh/bun PR: 0
File: test/CLAUDE.md:0-0
Timestamp: 2025-11-24T18:37:30.259Z
Learning: Applies to test/**/*.test.{ts,js,jsx,tsx,mjs,cjs} : Always check exit codes and test error scenarios in error tests
Applied to files:
test/js/web/websocket/websocket-permessage-deflate-edge-cases.test.ts
📚 Learning: 2025-12-05T19:28:39.534Z
Learnt from: alii
Repo: oven-sh/bun PR: 25360
File: src/bake/hmr-runtime-client.ts:240-249
Timestamp: 2025-12-05T19:28:39.534Z
Learning: In Bun's HMR runtime client (src/bake/hmr-runtime-client.ts), when handling error events where `event.error` is falsy, do not wrap `event.message` in a new Error object. Doing so would create misleading stack frames that show the error originating from the HMR client code rather than the actual error location, which hinders debugging.
Applied to files:
test/js/web/websocket/websocket-permessage-deflate-edge-cases.test.ts
📚 Learning: 2025-09-20T00:57:56.685Z
Learnt from: markovejnovic
Repo: oven-sh/bun PR: 22568
File: test/js/valkey/valkey.test.ts:268-276
Timestamp: 2025-09-20T00:57:56.685Z
Learning: For test/js/valkey/valkey.test.ts, do not comment on synchronous throw assertions for async Redis methods like ctx.redis.set() - the maintainer has explicitly requested to stop looking at this error pattern.
Applied to files:
test/js/web/websocket/websocket-permessage-deflate-edge-cases.test.ts
📚 Learning: 2025-10-08T13:48:02.430Z
Learnt from: Jarred-Sumner
Repo: oven-sh/bun PR: 23373
File: test/js/bun/tarball/extract.test.ts:107-111
Timestamp: 2025-10-08T13:48:02.430Z
Learning: In Bun's test runner, use `expect(async () => { await ... }).toThrow()` to assert async rejections. Unlike Jest/Vitest, Bun does not require `await expect(...).rejects.toThrow()` - the async function wrapper with `.toThrow()` is the correct pattern for async error assertions in Bun tests.
Applied to files:
test/js/web/websocket/websocket-permessage-deflate-edge-cases.test.ts
📚 Learning: 2025-11-24T18:37:30.259Z
Learnt from: CR
Repo: oven-sh/bun PR: 0
File: test/CLAUDE.md:0-0
Timestamp: 2025-11-24T18:37:30.259Z
Learning: Applies to test/**/*.test.{ts,js,jsx,tsx,mjs,cjs} : Do not set a timeout on tests. Bun already has timeouts
Applied to files:
test/js/web/websocket/websocket-permessage-deflate-edge-cases.test.ts
📚 Learning: 2025-11-24T18:37:30.259Z
Learnt from: CR
Repo: oven-sh/bun PR: 0
File: test/CLAUDE.md:0-0
Timestamp: 2025-11-24T18:37:30.259Z
Learning: Applies to test/**/*.test.{ts,js,jsx,tsx,mjs,cjs} : When spawning Bun processes in tests, use `bunExe` and `bunEnv` from `harness` to ensure the same build of Bun is used and debug logging is silenced
Applied to files:
test/js/web/websocket/websocket-permessage-deflate-edge-cases.test.ts
📚 Learning: 2025-10-19T02:44:46.354Z
Learnt from: theshadow27
Repo: oven-sh/bun PR: 23798
File: packages/bun-otel/context-propagation.test.ts:1-1
Timestamp: 2025-10-19T02:44:46.354Z
Learning: In the Bun repository, standalone packages under packages/ (e.g., bun-vscode, bun-inspector-protocol, bun-plugin-yaml, bun-plugin-svelte, bun-debug-adapter-protocol, bun-otel) co-locate their tests with package source code using *.test.ts files. This follows standard npm/monorepo patterns. The test/ directory hierarchy (test/js/bun/, test/cli/, test/js/node/) is reserved for testing Bun's core runtime APIs and built-in functionality, not standalone packages.
Applied to files:
test/js/web/websocket/websocket-permessage-deflate-edge-cases.test.ts
📚 Learning: 2025-11-24T18:36:59.706Z
Learnt from: CR
Repo: oven-sh/bun PR: 0
File: src/bun.js/bindings/v8/CLAUDE.md:0-0
Timestamp: 2025-11-24T18:36:59.706Z
Learning: Applies to src/bun.js/bindings/v8/test/v8/v8.test.ts : Add corresponding test cases to test/v8/v8.test.ts using checkSameOutput() function to compare Node.js and Bun output
Applied to files:
test/js/web/websocket/websocket-permessage-deflate-edge-cases.test.ts
📚 Learning: 2025-09-02T06:10:17.252Z
Learnt from: Jarred-Sumner
Repo: oven-sh/bun PR: 22323
File: test/js/web/websocket/websocket-subprotocol-strict.test.ts:80-91
Timestamp: 2025-09-02T06:10:17.252Z
Learning: Bun's WebSocket implementation includes a terminate() method, which is available even though it's not part of the standard WebSocket API. This method can be used in Bun test files and applications for immediate WebSocket closure.
Applied to files:
test/js/web/websocket/websocket-permessage-deflate-edge-cases.test.ts
📚 Learning: 2025-11-08T04:06:33.198Z
Learnt from: Jarred-Sumner
Repo: oven-sh/bun PR: 24491
File: test/js/bun/transpiler/declare-global.test.ts:17-17
Timestamp: 2025-11-08T04:06:33.198Z
Learning: In Bun test files, `await using` with Bun.spawn() is the preferred pattern for spawned processes regardless of whether they are short-lived or long-running. Do not suggest replacing `await using proc = Bun.spawn(...)` with `const proc = Bun.spawn(...); await proc.exited;`.
Applied to files:
test/js/web/websocket/websocket-permessage-deflate-edge-cases.test.ts
🧬 Code graph analysis (1)
test/js/web/websocket/websocket-permessage-deflate-edge-cases.test.ts (1)
test/js/web/websocket/websocket.test.js (1)
socket(805-805)
🔇 Additional comments (4)
src/http/websocket_client.zig (1)
234-242: LGTM! Error mapping correctly handles the new TooLarge case.The
error.TooLarge => ErrorCode.message_too_bigmapping appropriately translates the decompression size limit violation to the WebSocket close code 1009 (Message Too Big), which aligns with RFC 6455 semantics. The error handling remains exhaustive for all possibledecompresserror cases.src/http/websocket_client/WebSocketDeflate.zig (3)
79-80: LGTM! Reasonable decompression limit.The 128 MB limit provides effective protection against decompression bombs while being generous enough for legitimate large messages.
142-154: LGTM! Size limit correctly enforced in libdeflate path.The
initial_lentracking ensures only the newly decompressed bytes are counted against the limit, and the check is performed after a successful decompression.
173-176: LGTM! Incremental size check provides early bomb detection.Checking the size within the inflate loop allows early termination during decompression of malicious payloads, avoiding unnecessary memory allocation and CPU consumption.
| import { expect, setDefaultTimeout, test } from "bun:test"; | ||
|
|
||
| setDefaultTimeout(30_000); |
There was a problem hiding this comment.
Remove the explicit test timeout.
Per coding guidelines, tests should not set a timeout as Bun already has built-in timeouts. Remove setDefaultTimeout(30_000).
🔎 Proposed fix
-import { expect, setDefaultTimeout, test } from "bun:test";
-
-setDefaultTimeout(30_000);
+import { expect, test } from "bun:test";📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| import { expect, setDefaultTimeout, test } from "bun:test"; | |
| setDefaultTimeout(30_000); | |
| import { expect, test } from "bun:test"; |
🤖 Prompt for AI Agents
In test/js/web/websocket/websocket-permessage-deflate-edge-cases.test.ts around
lines 2 to 4, remove the explicit test timeout call setDefaultTimeout(30_000) so
the file relies on Bun's built-in timeouts; simply delete that line and keep the
existing imports and any other setup intact.
5f8d2e4 to
602bcfb
Compare
Add maximum decompressed message size limit to prevent resource exhaustion from highly compressible payloads. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
robobun
force-pushed
the
claude/websocket-deflate-hardening
branch
from
602bcfb to
c6dba46
Compare
Summary
Test plan
bun test test/js/web/websocket/websocket-permessage-deflate-edge-cases.test.ts🤖 Generated with Claude Code