Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
19 changes: 18 additions & 1 deletion src/js/thirdparty/ws.js
Original file line number Diff line number Diff line change
Expand Up @@ -91,6 +91,14 @@ function emitWarning(type, message) {
console.warn("[bun] Warning:", message);
}

// RFC 6455 7.4 + IANA: the status codes an endpoint may send in a Close
// frame. Matches npm `ws` lib/validation.js isValidStatusCode.
function isValidStatusCode(code) {
return (
(code >= 1000 && code <= 1014 && code !== 1004 && code !== 1005 && code !== 1006) || (code >= 3000 && code <= 4999)
);
}

// TODO: add private method on WebSocket to avoid these allocations
function normalizeData(data, opts) {
const isBinary = opts?.binary;
Expand Down Expand Up @@ -376,9 +384,15 @@ class BunWebSocket extends EventEmitter {
}

close(code, reason) {
// Match npm `ws`: TypeError for a code outside the RFC 6455 endpoint set.
// The native close() is WHATWG-strict (only 1000, 3000-4999); the third
// argument opts into the wider endpoint set so 1001-1014 pass.
if (code !== undefined && (typeof code !== "number" || !isValidStatusCode(code))) {
throw new TypeError("First argument must be a valid error code number");
}
const ws = this.#ws;
if (ws) {
ws.close(code, reason);
ws.close(code, reason, true);
}
}

Expand Down Expand Up @@ -986,6 +1000,9 @@ class BunWebSocketMocked extends EventEmitter {
}

close(code, reason) {
if (code !== undefined && (typeof code !== "number" || !isValidStatusCode(code))) {
throw new TypeError("First argument must be a valid error code number");
}
if (this.#state === ReadyState_OPEN) {
this.#state = ReadyState_CLOSING;
this.#ws.close(code, reason);
Expand Down
6 changes: 5 additions & 1 deletion src/jsc/bindings/webcore/JSWebSocket.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -761,7 +761,11 @@ static inline JSC::EncodedJSValue jsWebSocketPrototypeFunction_closeBody(JSC::JS
EnsureStillAliveScope argument1 = callFrame->argument(1);
auto reason = argument1.value().isUndefined() ? String() : convert<IDLUSVString>(*lexicalGlobalObject, argument1.value());
RETURN_IF_EXCEPTION(throwScope, {});
RELEASE_AND_RETURN(throwScope, JSValue::encode(toJS<IDLUndefined>(*lexicalGlobalObject, throwScope, [&]() -> decltype(auto) { return impl.close(WTF::move(code), WTF::move(reason)); })));
// Non-standard third argument: when truthy, validate the code against the
// RFC 6455 endpoint set instead of the WHATWG API set. Only the `ws` shim
// passes it, so npm `ws` clients can still close with 1001-1014.
bool allowEndpointCodes = callFrame->argument(2).toBoolean(lexicalGlobalObject);
RELEASE_AND_RETURN(throwScope, JSValue::encode(toJS<IDLUndefined>(*lexicalGlobalObject, throwScope, [&]() -> decltype(auto) { return impl.close(WTF::move(code), WTF::move(reason), allowEndpointCodes); })));
Comment thread
robobun marked this conversation as resolved.
Resolved
}

JSC_DEFINE_HOST_FUNCTION(jsWebSocketPrototypeFunction_close, (JSGlobalObject * lexicalGlobalObject, CallFrame* callFrame))
Expand Down
27 changes: 20 additions & 7 deletions src/jsc/bindings/webcore/WebSocket.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -118,10 +118,19 @@ static size_t getFramingOverhead(size_t payloadSize)

const size_t maxReasonSizeInBytes = 123;

// Close codes an endpoint may put on the wire (RFC 6455 7.4 + the IANA registry).
// Deliberately the RFC endpoint set, not the browser's "1000 or 3000-4999":
// non-browser clients legitimately send 1001 or 1011, and `ws` accepts the same set.
static inline bool isValidCloseCodeForSending(uint16_t code)
// https://websockets.spec.whatwg.org/#dom-websocket-close step 1: the API
// close code set is exactly 1000 and 3000-4999. 1001-1014 are legal RFC 6455
// endpoint codes on the wire but the WHATWG interface forbids passing them.
static inline bool isValidWHATWGCloseCode(uint16_t code)
{
return code == 1000 || (code >= 3000 && code <= 4999);
}

// RFC 6455 7.4 + the IANA registry: every code an endpoint may put on the
// wire. The `ws` npm package validates against this set, and Bun's `ws` shim
// forwards to this class, so close() accepts this wider set when the shim
// asks for it (undocumented third argument).
static inline bool isValidEndpointCloseCode(uint16_t code)
{
return (code >= 1000 && code <= 1014 && code != 1004 && code != 1005 && code != 1006)
|| (code >= 3000 && code <= 4999);
Expand Down Expand Up @@ -976,13 +985,17 @@ void WebSocket::failConnectingWebSocket()
updateHasPendingActivity();
}

ExceptionOr<void> WebSocket::close(std::optional<unsigned short> optionalCode, const String& reason)
ExceptionOr<void> WebSocket::close(std::optional<unsigned short> optionalCode, const String& reason, bool allowEndpointCodes)
{
// https://websockets.spec.whatwg.org/#dom-websocket-close
// Validation happens before the readyState checks: an invalid code or an
// over-long reason throws even on a CLOSING/CLOSED socket.
if (optionalCode && !isValidCloseCodeForSending(optionalCode.value()))
return Exception { InvalidAccessError, makeString("The close code must be a valid WebSocket close code (1000-1014, excluding the reserved codes 1004-1006, or in the range of 3000 to 4999). Received "_s, optionalCode.value(), '.') };
if (optionalCode) {
unsigned short code = optionalCode.value();
bool valid = allowEndpointCodes ? isValidEndpointCloseCode(code) : isValidWHATWGCloseCode(code);
if (!valid)
return Exception { InvalidAccessError, makeString("The close code must be either 1000 or in the range of 3000 to 4999. Received "_s, code, '.') };
}
Comment thread
robobun marked this conversation as resolved.
Resolved
if (!reason.isEmpty()) {
// The limit is on the UTF-8 encoding of the reason, not its UTF-16 length.
auto utf8Reason = reason.utf8(StrictConversionReplacingUnpairedSurrogatesWithFFFD);
Expand Down
2 changes: 1 addition & 1 deletion src/jsc/bindings/webcore/WebSocket.h
Original file line number Diff line number Diff line change
Expand Up @@ -177,7 +177,7 @@ class WebSocket final : public RefCounted<WebSocket>, public EventTargetWithInli
ExceptionOr<void> pong(JSC::ArrayBufferView&);
ExceptionOr<void> pong(JSBlob*);

ExceptionOr<void> close(std::optional<unsigned short> code, const String& reason);
ExceptionOr<void> close(std::optional<unsigned short> code, const String& reason, bool allowEndpointCodes = false);
ExceptionOr<void> terminate();

void setProtocol(const String& protocol);
Expand Down
2 changes: 1 addition & 1 deletion test/integration/bun-types/fixture/websocket.ts
Original file line number Diff line number Diff line change
Expand Up @@ -231,7 +231,7 @@ import { expectType } from "./utilities";
ws.close(1000);

// Close with code and reason
ws.close(1001, "Going away");
ws.close(3001, "Going away");
}

// Bun-specific WebSocket extensions test
Expand Down
4 changes: 2 additions & 2 deletions test/js/web/websocket/websocket-client.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -200,10 +200,10 @@ describe("WebSocket", () => {
});
test("(no reason)", (ws, done) => {
ws.addEventListener("open", () => {
ws.close(1001);
ws.close(3001);
});
ws.addEventListener("close", ({ code, reason, wasClean }) => {
expect(code).toBe(1001);
expect(code).toBe(3001);
expect(reason).toBeString();
expect(wasClean).toBeTrue();
done();
Expand Down
60 changes: 53 additions & 7 deletions test/js/web/websocket/websocket-close-code.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,15 +6,19 @@
import { describe, expect, it } from "bun:test";
import crypto from "node:crypto";
import { createServer, type Socket } from "node:net";
import { WebSocket as WsWebSocket } from "ws";

describe.concurrent("WebSocket close() argument validation", () => {
// Close codes an RFC 6455 endpoint must never put on the wire. close() has to
// reject them: the peer treats them as a protocol error.
const INVALID_CODES = [0, 999, 1004, 1005, 1006, 1015, 1016, 2999, 5000, 65535];
// Every code an endpoint may send (RFC 6455 7.4 + IANA): 1000-1014 minus the
// reserved 1004-1006, plus the 3000-4999 boundaries. Unlike browsers, the
// 1001-1014 band stays permitted: `ws` clients and Bun's inspector use 1001/1011.
const VALID_CODES = [1000, 1001, 1002, 1003, 1007, 1008, 1009, 1010, 1011, 1012, 1013, 1014, 3000, 4999];
// https://websockets.spec.whatwg.org/#dom-websocket-close step 1: the WHATWG
// API accepts only 1000 and 3000-4999. 1001-1014 are legal on the wire for an
// RFC 6455 endpoint but the browser interface forbids passing them; undici
// and every browser throw InvalidAccessError for them. The `ws` npm package
// accepts the wider endpoint set, covered in the "ws" describe below.
const INVALID_CODES = [
0, 999, 1001, 1002, 1003, 1004, 1005, 1006, 1007, 1008, 1009, 1010, 1011, 1012, 1013, 1014, 1015, 1016, 2999, 5000,
65535,
];
const VALID_CODES = [1000, 3000, 4000, 4999];
const LONG_REASON = Buffer.alloc(124, "R").toString();

// `constructor` is DOMException for InvalidAccessError, but the native
Expand Down Expand Up @@ -139,6 +143,48 @@ describe.concurrent("WebSocket close() argument validation", () => {
expect(await clientClosed).toBe(code);
});
});

describe("ws package close() argument validation", () => {
// npm `ws` validates against the RFC 6455 endpoint set and throws a
// TypeError, not a DOMException. Bun's shim forwards to the native
// WebSocket, so it has to opt out of the WHATWG-strict check above to
// keep 1001-1014 working.
const WS_VALID = [1000, 1001, 1002, 1003, 1007, 1008, 1009, 1010, 1011, 1012, 1013, 1014, 3000, 4999];
const WS_INVALID = [0, 999, 1004, 1005, 1006, 1015, 2999, 5000, 65535];

async function openWs(server: Bun.Server) {
const ws = new WsWebSocket(`ws://127.0.0.1:${server.port}/`);
const { promise, resolve, reject } = Promise.withResolvers<void>();
ws.on("open", resolve);
ws.on("error", reject);
await promise;
return ws;
}

it("throws TypeError for codes outside the RFC 6455 endpoint set", async () => {
using server = upgradeServer();
const ws = await openWs(server);
try {
for (const code of WS_INVALID) {
expectThrows(() => ws.close(code), TypeError, "TypeError", "valid error code number");
expect(ws.readyState).toBe(WsWebSocket.OPEN);
}
expectThrows(() => ws.close("1000" as any), TypeError, "TypeError", "valid error code number");
} finally {
ws.close();
}
});

describe.each(WS_VALID)("close(%i) is allowed", code => {
it("and the code reaches the server", async () => {
const serverGotClose = Promise.withResolvers<{ code: number; reason: string }>();
using server = upgradeServer(serverGotClose.resolve);
const ws = await openWs(server);
ws.close(code, "bye");
expect(await serverGotClose.promise).toEqual({ code, reason: "bye" });
});
});
});
});

describe.concurrent("WebSocket client and server-sent close frames", () => {
Expand Down
Loading