websocket, wpt: Improve websocket WPT results + flakiness

1 - Blocks connection to "bad" ports
2 - new WebSocket('...'); mostly returns the instance, and errors are emitted
    in a close event (1)
3 - More validation (no # allowed, ...);
4 - http -> ws, https -> wss  (yes, per spec)

There's also some groundwork for better float16 support, but it turns out this
will require a new v8 build to complete. So I left the harmless float16 mapping
in (but, without the matching Float16Array, it doesn't help much).

(1) Some things cause an exception to be raised, but most don't
This commit is contained in:
Karl Seguin
2026-07-22 16:16:10 +08:00
parent 12fa469214
commit c451cd763a
5 changed files with 242 additions and 30 deletions

View File

@@ -623,7 +623,8 @@ pub fn jsValueToZig(self: *const Local, comptime T: type, js_val: js.Value) !T {
return try self.jsValueToZig(o.child, js_val);
},
.float => |f| switch (f.bits) {
0...32 => return js_val.toF32(),
0...16 => return js_val.toF16(),
17...32 => return js_val.toF32(),
33...64 => return js_val.toF64(),
else => {},
},

View File

@@ -234,6 +234,10 @@ pub fn typeOf(self: Value) js.String {
return js.String{ .local = self.local, .handle = str_handle };
}
pub fn toF16(self: Value) !f16 {
return @floatCast(try self.toF64());
}
pub fn toF32(self: Value) !f32 {
return @floatCast(try self.toF64());
}

View File

@@ -562,6 +562,60 @@
}
</script>
<script id=url_scheme_normalization>
{
// http(s) is an accepted spelling of ws(s), and the url is resolved against
// the document base before its scheme is looked at — so a relative url is
// as valid as an absolute one.
const dir = new URL('.', location);
const cases = [
['http://127.0.0.1:9584/chat', 'ws://127.0.0.1:9584/chat'],
['https://127.0.0.1:9584/chat', 'wss://127.0.0.1:9584/chat'],
['ws://127.0.0.1:9584/chat', 'ws://127.0.0.1:9584/chat'],
['chat', 'ws://' + dir.host + dir.pathname + 'chat'],
['?x=1', 'ws://' + dir.host + location.pathname + '?x=1'],
];
for (const [input, expected] of cases) {
const ws = new WebSocket(input);
testing.expectEqual(expected, ws.url);
ws.close();
}
// the document's charset never applies: a websocket url percent-encodes
// as UTF-8 no matter what the page is in
const euro = new WebSocket('ws://127.0.0.1:9584/?€');
testing.expectEqual('ws://127.0.0.1:9584/?%E2%82%AC', euro.url);
euro.close();
// every other scheme is a SyntaxError, as is a url the parser rejects
for (const input of ['ftp://127.0.0.1:9584/', 'about:blank', 'mailto:a@b.org', 'ws://foo bar.com/']) {
testing.expectError('SyntaxError', () => new WebSocket(input));
}
}
</script>
<script id=blocked_port type=module>
{
const state = await testing.async();
let received = [];
// 22 (ssh) is a bad port; the constructor still returns an object in
// CONNECTING and the connection is failed asynchronously.
let ws = new WebSocket('ws://127.0.0.1:22/');
received.push(ws.readyState);
ws.addEventListener('open', () => { received.push('open'); });
ws.addEventListener('error', () => { received.push('error'); });
ws.addEventListener('close', (e) => {
received.push(['close', e.code, e.wasClean]);
state.resolve();
});
await state.done(() => {
testing.expectEqual([0, 'error', ['close', 1006, false]], received);
});
}
</script>
<script id=close_while_connecting type=module>
{
const state = await testing.async();

View File

@@ -139,15 +139,8 @@ pub const BinaryType = enum {
pub fn init(url: []const u8, protocols: [][]const u8, exec: *const Execution) !*WebSocket {
{
if (url.len < 6) {
return error.SyntaxError;
}
const normalized_start = std.ascii.lowerString(exec.buf, url[0..6]);
if (!std.mem.startsWith(u8, normalized_start, "ws://") and !std.mem.startsWith(u8, normalized_start, "wss://")) {
return error.SyntaxError;
}
// Fragments are not allowed in WebSocket URLs
if (std.mem.indexOfScalar(u8, url, '#') != null) {
// Fragments are not allowed in WebSocket URLs.
return error.SyntaxError;
}
for (protocols) |protocol| {
@@ -160,9 +153,78 @@ pub fn init(url: []const u8, protocols: [][]const u8, exec: *const Execution) !*
const arena = try exec.getArena(.medium, "WebSocket");
errdefer exec.releaseArena(arena);
const resolved_url = try URL.resolve(arena, exec.base(), url, .{ .encoding = exec.charset.* });
const resolved_url = blk: {
// Always UTF-8, never the document's charse
const resolved = URL.resolve(arena, exec.base(), url, .{ .encoding = "UTF-8" }) catch |err| switch (err) {
error.TypeError => return error.SyntaxError,
else => return err,
};
const scheme = URL.getProtocol(resolved);
if (std.mem.eql(u8, scheme, "ws:") or std.mem.eql(u8, scheme, "wss:")) {
// normal case
break :blk resolved;
}
// yup, this is what we're supposed to do.
if (std.mem.eql(u8, scheme, "http:")) {
break :blk try std.fmt.allocPrintSentinel(arena, "ws{s}", .{resolved["http".len..]}, 0);
}
if (std.mem.eql(u8, scheme, "https:")) {
break :blk try std.fmt.allocPrintSentinel(arena, "wss{s}", .{resolved["https".len..]}, 0);
}
return error.SyntaxError;
};
const http_client = &exec.session.browser.http_client;
const self = try exec._factory.eventTargetWithAllocator(arena, WebSocket{
._exec = exec,
._conn = null,
._arena = arena,
._proto = undefined,
._url = resolved_url,
._req_headers = .{ .headers = null },
._http_client = http_client,
});
// This ensures that if we fail to connect, we have at least 1 event slot
// to register the close+error
try self._events.ensureTotalCapacity(arena, 1);
exec.httpOwner().addWS(self);
// Unlike an XHR object where we only selectively reference the instance
// while the request is actually inflight, WS connection is "inflight" from
// the moment it's created. deactivate() releases this reference.
self.acquireRef();
if (comptime IS_DEBUG) {
log.info(.websocket, "connecting", .{ .url = url });
}
// "Establish a WebSocket connection" only ever fails the connection, it
// never throws: the object is returned in CONNECTING and the failure
// surfaces as an error event followed by close. That covers a blocked
// port as much as it covers running out of connections.
self.connect(protocols) catch |err| {
self.transportClosed(err);
};
return self;
}
fn connect(self: *WebSocket, protocols: [][]const u8) !void {
const exec = self._exec;
const arena = self._arena;
const resolved_url = self._url;
const http_client = self._http_client;
if (isBlockedPort(resolved_url)) {
return error.BlockedPort;
}
const conn = http_client.network.newConnection() orelse {
return error.NoFreeConnection;
};
@@ -211,29 +273,20 @@ pub fn init(url: []const u8, protocols: [][]const u8, exec: *const Execution) !*
try conn.setHeaders(&headers);
const self = try exec._factory.eventTargetWithAllocator(arena, WebSocket{
._exec = exec,
._conn = conn,
._arena = arena,
._proto = undefined,
._url = resolved_url,
._req_headers = headers,
._http_client = http_client,
});
conn.transport = .{ .websocket = self };
try http_client.trackConn(conn);
exec.httpOwner().addWS(self);
if (comptime IS_DEBUG) {
log.info(.websocket, "connecting", .{ .url = url });
self._conn = conn;
self._req_headers = headers;
}
fn isBlockedPort(url: [:0]const u8) bool {
const port = URL.getPort(url);
if (port.len == 0) {
// the default port for ws/wss is never blocked
return false;
}
// Unlike an XHR object where we only selectively reference the instance
// while the request is actually inflight, WS connection is "inflight" from
// the moment it's created. deactivate() releases this reference.
self.acquireRef();
return self;
return http.isBadPort(std.fmt.parseInt(u16, port, 10) catch return false);
}
pub fn deinit(self: *WebSocket, page: *Page) void {
@@ -509,6 +562,7 @@ const BinaryData = union(enum) {
uint32: []u32,
int64: []i64,
uint64: []u64,
float16: []f16,
float32: []f32,
float64: []f64,
@@ -516,7 +570,7 @@ const BinaryData = union(enum) {
return switch (self) {
.int8 => |b| @as([*]u8, @ptrCast(b.ptr))[0..b.len],
.uint8 => |b| b,
inline .int16, .uint16 => |b| @as([*]u8, @ptrCast(b.ptr))[0 .. b.len * 2],
inline .int16, .uint16, .float16 => |b| @as([*]u8, @ptrCast(b.ptr))[0 .. b.len * 2],
inline .int32, .uint32, .float32 => |b| @as([*]u8, @ptrCast(b.ptr))[0 .. b.len * 4],
inline .int64, .uint64, .float64 => |b| @as([*]u8, @ptrCast(b.ptr))[0 .. b.len * 8],
};

View File

@@ -778,6 +778,96 @@ pub fn statusCategory(status: u16) StatusCategory {
};
}
// https://fetch.spec.whatwg.org/#bad-port
pub fn isBadPort(port: u16) bool {
return switch (port) {
1, // tcpmux
7, // echo
9, // discard
11, // systat
13, // daytime
15, // netstat
17, // qotd
19, // chargen
20, // ftp-data
21, // ftp
22, // ssh
23, // telnet
25, // smtp
37, // time
42, // name
43, // nicname
53, // domain
69, // tftp
77, // priv-rjs
79, // finger
87, // ttylink
95, // supdup
101, // hostriame
102, // iso-tsap
103, // gppitnp
104, // acr-nema
109, // pop2
110, // pop3
111, // sunrpc
113, // auth
115, // sftp
117, // uucp-path
119, // nntp
123, // ntp
135, // loc-srv / epmap
137, // netbios-ns
139, // netbios-ssn
143, // imap2
161, // snmp
179, // bgp
389, // ldap
427, // afp (alternate)
465, // smtp (alternate)
512, // print / exec
513, // login
514, // shell
515, // printer
526, // tempo
530, // courier
531, // chat
532, // netnews
540, // uucp
548, // afp
554, // rtsp
556, // remotefs
563, // nntp+ssl
587, // smtp (outgoing)
601, // syslog-conn
636, // ldap+ssl
989, // ftps-data
990, // ftps
993, // imap+ssl
995, // pop3+ssl
1719, // h323gatestat
1720, // h323hostcall
1723, // pptp
2049, // nfs
3659, // apple-sasl
4045, // lockd
4190, // sieve
5060, // sip
5061, // sips
6000, // x11
6566, // sane-port
6665, // irc (alternate)
6666, // irc (alternate)
6667, // irc (default)
6668, // irc (alternate)
6669, // irc (alternate)
6679, // osaut
6697, // irc+tls
10080, // amanda
=> true,
else => false,
};
}
// Coarse failure classes for the http_error metric. Anything we don't
// explicitly bucket lands in `other`.
pub const ErrorReason = enum {
@@ -850,6 +940,15 @@ fn makeSockAddrV4(ip: [4]u8) libcurl.CurlSockAddr {
const testing = @import("../testing.zig");
test "isBadPort" {
for ([_]u16{ 1, 22, 25, 143, 6697, 10080 }) |port| {
try testing.expect(isBadPort(port));
}
for ([_]u16{ 0, 80, 443, 8000, 8080, 9584, 65535 }) |port| {
try testing.expect(!isBadPort(port));
}
}
test "Header.firstValue" {
try testing.expectEqualSlices(u8, "attachment", (Header{ .name = "Content-Disposition", .value = "attachment" }).firstValue());
// firstValue trims but preserves case (callers compare case-insensitively).