Address feedback

-dispatch error on abnormal close
-reciprocal close message
-more url validation
-cleanup dead code
This commit is contained in:
Karl Seguin
2026-04-04 07:39:55 +08:00
parent 6bf35e1ed4
commit c4250418af
4 changed files with 64 additions and 60 deletions

View File

@@ -270,7 +270,11 @@ fn _abort(self: *Client, comptime abort_all: bool, frame_id: u32) void {
var leftover: usize = 0; var leftover: usize = 0;
while (it) |node| : (it = node.next) { while (it) |node| : (it = node.next) {
const conn: *http.Connection = @fieldParentPtr("node", node); const conn: *http.Connection = @fieldParentPtr("node", node);
std.debug.assert((Transfer.fromConnection(conn) catch unreachable).aborted); switch (conn.transport) {
.http => |transfer| std.debug.assert(transfer.aborted),
.websocket => {},
.none => {},
}
leftover += 1; leftover += 1;
} }
std.debug.assert(self.active == leftover); std.debug.assert(self.active == leftover);
@@ -1015,7 +1019,7 @@ fn processMessages(self: *Client) !bool {
ws.disconnected(null); ws.disconnected(null);
} }
return true; processed = true;
}, },
.none => unreachable, .none => unreachable,
} }

View File

@@ -304,35 +304,6 @@
} }
</script> </script>
<script id=binary_int32array22 type=module>
{
const state = await testing.async();
let received = [];
let ws = new WebSocket('ws://127.0.0.1:9584/');
ws.binaryType = 'arraybuffer';
ws.addEventListener('open', () => {
const arr = new Int32Array([0x01020304, 0x05060708]);
ws.send(arr);
});
ws.addEventListener('message', (e) => {
received.push(e.data.byteLength);
ws.close();
});
ws.addEventListener('close', () => {
state.resolve();
});
await state.done(() => {
// 1 marker byte + 8 bytes (2 x 4-byte int32)
testing.expectEqual([9], received);
});
}
</script>
<script id=binary_blob type=module> <script id=binary_blob type=module>
{ {
const state = await testing.async(); const state = await testing.async();
@@ -412,8 +383,8 @@
}); });
await state.done(() => { await state.done(() => {
// Connection was not cleanly closed // Connection was not cleanly closed - error fires before close
testing.expectEqual(['closed', false], received); testing.expectEqual(['error', 'closed', false], received);
}); });
} }
</script> </script>
@@ -582,3 +553,11 @@
}); });
} }
</script> </script>
<script id=url_with_fragment_rejected>
{
testing.expectError('SyntaxError', () => {
new WebSocket('ws://127.0.0.1:9584/#fragment');
});
}
</script>

View File

@@ -99,6 +99,10 @@ pub fn init(url: []const u8, protocols_: ?[]const u8, page: *Page) !*WebSocket {
if (!std.mem.startsWith(u8, normalized_start, "ws://") and !std.mem.startsWith(u8, normalized_start, "wss://")) { if (!std.mem.startsWith(u8, normalized_start, "ws://") and !std.mem.startsWith(u8, normalized_start, "wss://")) {
return error.SyntaxError; return error.SyntaxError;
} }
// Fragments are not allowed in WebSocket URLs
if (std.mem.indexOfScalar(u8, url, '#') != null) {
return error.SyntaxError;
}
} }
const arena = try page.getArena(.{ .debug = "WebSocket" }); const arena = try page.getArena(.{ .debug = "WebSocket" });
@@ -118,7 +122,7 @@ pub fn init(url: []const u8, protocols_: ?[]const u8, page: *Page) !*WebSocket {
try conn.setReadCallback(sendDataCallback, true); try conn.setReadCallback(sendDataCallback, true);
try conn.setWriteCallback(receivedDataCallback); try conn.setWriteCallback(receivedDataCallback);
try conn.setHeaderCallback(receivedHeaderCalllback); try conn.setHeaderCallback(receivedHeaderCallback);
const self = try page._factory.eventTargetWithAllocator(arena, WebSocket{ const self = try page._factory.eventTargetWithAllocator(arena, WebSocket{
._page = page, ._page = page,
@@ -187,6 +191,13 @@ pub fn disconnected(self: *WebSocket, err_: ?anyerror) void {
const code = if (was_clean) self._close_code else 1006; const code = if (was_clean) self._close_code else 1006;
const reason = if (was_clean) self._close_reason else ""; const reason = if (was_clean) self._close_reason else "";
// Spec requires error event before close on abnormal closure
if (!was_clean) {
self.dispatchErrorEvent() catch |err| {
log.err(.websocket, "error event dispatch failed", .{ .err = err });
};
}
self.dispatchCloseEvent(code, reason, was_clean) catch |err| { self.dispatchCloseEvent(code, reason, was_clean) catch |err| {
log.err(.websocket, "close event dispatch failed", .{ .err = err }); log.err(.websocket, "close event dispatch failed", .{ .err = err });
}; };
@@ -197,6 +208,7 @@ fn cleanup(self: *WebSocket) void {
self._http_client.removeConn(conn); self._http_client.removeConn(conn);
self._conn = null; self._conn = null;
self.releaseRef(self._page._session); self.releaseRef(self._page._session);
self._send_queue.clearRetainingCapacity();
} }
} }
@@ -293,6 +305,13 @@ pub fn close(self: *WebSocket, code_: ?u16, reason_: ?[]const u8) !void {
return; return;
} }
// Validate close code per spec: must be 1000 or in range 3000-4999
if (code_) |code| {
if (code != 1000 and (code < 3000 or code > 4999)) {
return error.InvalidAccessError;
}
}
const code = code_ orelse 1000; const code = code_ orelse 1000;
const reason = reason_ orelse ""; const reason = reason_ orelse "";
@@ -329,14 +348,6 @@ pub fn getBufferedAmount(self: *const WebSocket) u32 {
return buffered; return buffered;
} }
pub fn getProtocol(self: *const WebSocket) []const u8 {
return self._protocol;
}
pub fn getExtensions(self: *const WebSocket) []const u8 {
return self._extensions;
}
pub fn getBinaryType(self: *const WebSocket) []const u8 { pub fn getBinaryType(self: *const WebSocket) []const u8 {
return @tagName(self._binary_type); return @tagName(self._binary_type);
} }
@@ -404,7 +415,7 @@ fn dispatchOpenEvent(self: *WebSocket) !void {
const target = self.asEventTarget(); const target = self.asEventTarget();
if (page._event_manager.hasDirectListeners(target, "open", self._on_open)) { if (page._event_manager.hasDirectListeners(target, "open", self._on_open)) {
const event = try Event.initTrusted(.wrap("open"), .{}, page); const event = try Event.initTrusted(comptime .wrap("open"), .{}, page);
try page._event_manager.dispatchDirect(target, event, self._on_open, .{ .context = "WebSocket open" }); try page._event_manager.dispatchDirect(target, event, self._on_open, .{ .context = "WebSocket open" });
} }
} }
@@ -426,7 +437,7 @@ fn dispatchMessageEvent(self: *WebSocket, data: []const u8, frame_type: http.WsF
else else
.{ .string = data }; .{ .string = data };
const event = try MessageEvent.initTrusted(.wrap("message"), .{ const event = try MessageEvent.initTrusted(comptime .wrap("message"), .{
.data = msg_data, .data = msg_data,
.origin = "", .origin = "",
}, page); }, page);
@@ -434,12 +445,22 @@ fn dispatchMessageEvent(self: *WebSocket, data: []const u8, frame_type: http.WsF
} }
} }
fn dispatchErrorEvent(self: *WebSocket) !void {
const page = self._page;
const target = self.asEventTarget();
if (page._event_manager.hasDirectListeners(target, "error", self._on_error)) {
const event = try Event.initTrusted(comptime .wrap("error"), .{}, page);
try page._event_manager.dispatchDirect(target, event, self._on_error, .{ .context = "WebSocket error" });
}
}
fn dispatchCloseEvent(self: *WebSocket, code: u16, reason: []const u8, was_clean: bool) !void { fn dispatchCloseEvent(self: *WebSocket, code: u16, reason: []const u8, was_clean: bool) !void {
const page = self._page; const page = self._page;
const target = self.asEventTarget(); const target = self.asEventTarget();
if (page._event_manager.hasDirectListeners(target, "close", self._on_close)) { if (page._event_manager.hasDirectListeners(target, "close", self._on_close)) {
const event = try CloseEvent.initTrusted(.wrap("close"), .{ const event = try CloseEvent.initTrusted(comptime .wrap("close"), .{
.code = code, .code = code,
.reason = reason, .reason = reason,
.wasClean = was_clean, .wasClean = was_clean,
@@ -573,15 +594,24 @@ fn _receivedDataCallback(conn: *http.Connection, data: []const u8) !void {
.text, .binary => try self.dispatchMessageEvent(message, meta.frame_type), .text, .binary => try self.dispatchMessageEvent(message, meta.frame_type),
.close => { .close => {
// Parse close frame: 2-byte code (big-endian) + optional reason // Parse close frame: 2-byte code (big-endian) + optional reason
self._close_code = if (message.len >= 2) const received_code = if (message.len >= 2)
@as(u16, message[0]) << 8 | message[1] @as(u16, message[0]) << 8 | message[1]
else else
1005; // No status code received 1005; // No status code received
if (self._ready_state == .closing) {
// Client-initiated close: this is the server's response.
// Close handshake complete - disconnect.
self.disconnected(null);
} else {
// Server-initiated close: send reciprocal close frame per RFC 6455 §5.5.1
self._close_code = received_code;
if (message.len > 2) { if (message.len > 2) {
self._close_reason = try self._arena.dupe(u8, message[2..]); self._close_reason = try self._arena.dupe(u8, message[2..]);
} }
self._ready_state = .closing; self._ready_state = .closing;
self.disconnected(null); try self.queueMessage(.close);
}
}, },
.ping, .pong, .cont => {}, .ping, .pong, .cont => {},
} }
@@ -589,7 +619,7 @@ fn _receivedDataCallback(conn: *http.Connection, data: []const u8) !void {
// libcurl has no mechanism to signal that the connection is established. The // libcurl has no mechanism to signal that the connection is established. The
// best option I could come up with was looking for an upgrade header response. // best option I could come up with was looking for an upgrade header response.
fn receivedHeaderCalllback(buffer: [*]const u8, header_count: usize, buf_len: usize, data: *anyopaque) usize { fn receivedHeaderCallback(buffer: [*]const u8, header_count: usize, buf_len: usize, data: *anyopaque) usize {
if (comptime IS_DEBUG) { if (comptime IS_DEBUG) {
std.debug.assert(header_count == 1); std.debug.assert(header_count == 1);
} }

View File

@@ -349,15 +349,6 @@ pub const Connection = struct {
try libcurl.curl_easy_setopt(self._easy, .header_function, data_cb); try libcurl.curl_easy_setopt(self._easy, .header_function, data_cb);
} }
pub const PauseFlags = packed struct {
red: bool = false,
green: bool = false,
blue: bool = false,
alpha: bool = false,
// Optional padding to match a specific size, e.g., a u32
_padding: u28 = 0,
};
pub fn pause( pub fn pause(
self: *Connection, self: *Connection,
flags: libcurl.CurlPauseFlags, flags: libcurl.CurlPauseFlags,