From ee4d54928d0e5c233f5b6fb2bf5dbfa29e9dcc68 Mon Sep 17 00:00:00 2001 From: Karl Seguin Date: Mon, 21 Sep 2026 17:52:41 +0800 Subject: [PATCH] webdriver: increase http default / max limit The http max default was 4K with a 16KB hard limit. The default limit is now 1MB with an initial default of 4K. This is to accommodate larger WebDriver payloads. --- src/Config.zig | 5 +- src/help.zon | 5 +- src/server/Server.zig | 54 ++++++++---- src/server/bidi/http_command.zig | 3 +- src/server/http.zig | 145 ++++++++++++++++++++++++++----- 5 files changed, 167 insertions(+), 45 deletions(-) diff --git a/src/Config.zig b/src/Config.zig index 7eb5bf809..708236f5b 100644 --- a/src/Config.zig +++ b/src/Config.zig @@ -405,8 +405,7 @@ const Commands = cli.Builder(.{ .{ .name = "cdp_max_connections", .type = u16, .default = 16 }, .{ .name = "cdp_max_pending_connections", .type = u16, .default = 128 }, .{ .name = "cdp_max_message_size", .type = u32, .default = 1024 * 1024 }, - // Don't widen this without growing the reader buffer in the HTTP path. - .{ .name = "cdp_max_http_message_size", .type = u14, .default = 4096 }, + .{ .name = "cdp_max_http_message_size", .type = u32, .default = 1024 * 1024 }, .{ .name = "http_session_timeout", .type = u32, .default = 60 }, .{ .name = "disable_metrics", .type = bool }, }, @@ -919,7 +918,7 @@ pub fn dumpMetricsOnExit(self: *const Config) bool { }; } -pub fn cdpMaxHTTPMessageSize(self: *const Config) u14 { +pub fn cdpMaxHTTPMessageSize(self: *const Config) u32 { return switch (self.mode) { .serve => |opts| opts.cdp_max_http_message_size, else => unreachable, diff --git a/src/help.zon b/src/help.zon index c6aba3f2a..d9e7e2219 100644 --- a/src/help.zon +++ b/src/help.zon @@ -28,8 +28,9 @@ \\ Maximum number of simultaneous CDP connections. \\ Defaults to 16. \\ --cdp-max-http-message-size - \\ Maximum allowed HTTP request size - \\ Defaults to 4096 (maximum allowed: 16383) + \\ Maximum allowed HTTP request size. Connections start with a small + \\ read buffer and only grow as needed. + \\ Defaults to 1048576 (1MB) \\ --cdp-max-message-size \\ Maximum allowed incoming websocket message size. \\ Defaults to 1048576 (1MB) diff --git a/src/server/Server.zig b/src/server/Server.zig index 44c8f47d8..9aa50b26c 100644 --- a/src/server/Server.zig +++ b/src/server/Server.zig @@ -844,8 +844,9 @@ fn fdBudget(config: *const Config) usize { }; break :blk limit.cur; }; - // put some limit incase of a unlimited or very large rlimit - const ceiling = (64 * 1024 * 1024) / @max(@as(u64, config.cdpMaxHTTPMessageSize()), 1); + // put some limit incase of a unlimited or very large rlimit. A connection + // only commits INITIAL_BUFFER_SIZE up front + const ceiling = (64 * 1024 * 1024) / http.INITIAL_BUFFER_SIZE; const budget = @min(soft, ceiling) -| reserve; return @intCast(@max(budget, 8)); } @@ -1441,13 +1442,30 @@ test "server: buildJSONVersionResponse" { try testing.expect(std.mem.indexOf(u8, res, "\"webSocketDebuggerUrl\": \"ws://127.0.0.1:9222/\"") != null); } -test "Client: http invalid request" { - testing.silenceLog(&.{.cdp}); - +test "Client: http header past the initial buffer" { var c = try createTestClient(); defer c.deinit(); + // A header this size doesn't fit the buffer a connection starts with; it + // grows to take it rather than rejecting the request. const res = try c.httpRequest("GET /over/9000 HTTP/1.1\r\n" ++ "Header: " ++ ("a" ** 4100) ++ "\r\n\r\n"); + try testing.expectEqual("HTTP/1.1 404 \r\n" ++ + "Connection: Close\r\n" ++ + "Content-Length: 9\r\n\r\n" ++ + "Not found", res); +} + +test "Client: http request past the limit" { + var c = try createTestClient(); + defer c.deinit(); + + // The body never arrives: Content-Length alone is enough to turn it down, + // so we never read (or make room for) any of it. + var buf: [128]u8 = undefined; + const request = try std.fmt.bufPrint(&buf, "POST /session HTTP/1.1\r\nContent-Length: {d}\r\n\r\n", .{ + @as(u64, testing.test_app.config.cdpMaxHTTPMessageSize()) + 1, + }); + const res = try c.httpRequest(request); try testing.expectEqual("HTTP/1.1 413 \r\n" ++ "Connection: Close\r\n" ++ "Content-Length: 17\r\n\r\n" ++ @@ -2891,19 +2909,14 @@ test "server: releasing past the pool's retain destroys the connection" { } test "server: the connection budget is bounded by buffer memory" { - const opts = &testing.test_config.mode.serve; - const original = opts.cdp_max_http_message_size; - defer opts.cdp_max_http_message_size = original; - // whatever NOFILE happens to be, we never sign up for more read buffers - // than fdBudget's ceiling pays for (kept in step with it by hand) + // than fdBudget's ceiling pays for (kept in step with it by hand). Only + // the initial size is committed; --cdp-max-http-message-size caps what a + // request in flight may grow one to, and doesn't enter into the budget. const ceiling = 64 * 1024 * 1024; - for ([_]u14{ 1024, 4096, 16383 }) |size| { - opts.cdp_max_http_message_size = size; - const budget = fdBudget(testing.test_app.config); - try testing.expect(budget * size <= ceiling); - try testing.expect(budget >= 8); - } + const budget = fdBudget(testing.test_app.config); + try testing.expect(budget * http.INITIAL_BUFFER_SIZE <= ceiling); + try testing.expect(budget >= 8); } test "server: accepted sockets get TCP keepalive" { @@ -2924,12 +2937,12 @@ test "server: accepted sockets get TCP keepalive" { http.disconnect(lt.server, conn); } -test "server: the http read buffer is sized by --cdp-max-http-message-size" { +test "server: --cdp-max-http-message-size is a limit, not an allocation" { // the pool is built in Server.init, so this has to move first const opts = &testing.test_config.mode.serve; const original = opts.cdp_max_http_message_size; defer opts.cdp_max_http_message_size = original; - opts.cdp_max_http_message_size = 8192; + opts.cdp_max_http_message_size = 512 * 1024; var lt = try LoopTest.init(); defer lt.deinit(); @@ -2937,7 +2950,10 @@ test "server: the http read buffer is sized by --cdp-max-http-message-size" { const client, const conn = try lt.accept(); defer sys_net.close(client); - try testing.expectEqual(8192, conn.buffer.buf.len); + // a connection costs the initial buffer whatever the limit is; only a + // request that needs the room grows it + try testing.expectEqual(http.INITIAL_BUFFER_SIZE, conn.buffer.buf.len); + try testing.expectEqual(512 * 1024, conn.buffer.max); http.disconnect(lt.server, conn); } diff --git a/src/server/bidi/http_command.zig b/src/server/bidi/http_command.zig index 3b391385a..2a7f4d8d3 100644 --- a/src/server/bidi/http_command.zig +++ b/src/server/bidi/http_command.zig @@ -109,7 +109,8 @@ fn parseBody(comptime T: type, arena: Allocator, body: []const u8) ParseError!T } return std.json.parseFromSliceLeaky(T, arena, body, .{ .ignore_unknown_fields = true, - // body is the connection's read buffer, reused once the request is parked + // body is the connection's read buffer, if we park the connection, that + // buffer will be re-used. Our result cannot point into it. .allocate = .alloc_always, }) catch |err| switch (err) { error.OutOfMemory => error.OutOfMemory, diff --git a/src/server/http.zig b/src/server/http.zig index 51d3447fa..655964532 100644 --- a/src/server/http.zig +++ b/src/server/http.zig @@ -131,9 +131,15 @@ pub const Connection = struct { header: void, // still parsing the header request: Request, - fn parseHeader(self: *State, arena: Allocator, data: []u8) !bool { + // What the connection still needs before the request can be served. + const Parsed = union(enum) { + complete, + need: usize, // bytes the buffer needs will hold, 0 while parsing the header + }; + + fn parseHeader(self: *State, arena: Allocator, data: []u8) !Parsed { const header_index = std.mem.indexOf(u8, data, "\r\n\r\n") orelse { - return false; + return .{ .need = 0 }; }; // include the last line's \r\n so every line, including the request @@ -143,10 +149,11 @@ pub const Connection = struct { _ = line_1_end; const body_start = header_index + 4; - const total = body_start + try contentLength(header); + // large content lenghts will saturate to max(usize) -> 413 + const total = body_start +| try contentLength(header); if (data.len < total) { // the body is still arriving - return false; + return .{ .need = total }; } // A WebSocket upgrade may be pipelined with its first frames, but every // client we care about waits for the 101 first. Anything past the @@ -164,11 +171,9 @@ pub const Connection = struct { .arena = arena, } }; - return true; + return .complete; } - // The HTTP WebDriver bootstrap (POST /session) is the only thing - // that sends a body; everything else is 0. fn contentLength(header: []const u8) !usize { const key = "\r\ncontent-length:"; const at = std.ascii.indexOfIgnoreCase(header, key) orelse return 0; @@ -207,16 +212,17 @@ pub const Connection = struct { const Buffer = struct { buf: []u8, - // position in buf up until where we have valid data len: usize, - + max: usize, allocator: Allocator, - fn init(allocator: Allocator, size: usize) !Buffer { + fn init(allocator: Allocator, max: usize) !Buffer { + const real_max = @max(max, INITIAL_BUFFER_SIZE); return .{ .len = 0, - .buf = try allocator.alloc(u8, size), + .max = real_max, + .buf = try allocator.alloc(u8, INITIAL_BUFFER_SIZE), .allocator = allocator, }; } @@ -225,10 +231,34 @@ pub const Connection = struct { self.allocator.free(self.buf); } + fn reset(self: *Buffer) void { + self.len = 0; + if (self.buf.len == INITIAL_BUFFER_SIZE) { + return; + } + // keeping the larger buffer is only wasteful, so failure is fine + self.buf = self.allocator.realloc(self.buf, INITIAL_BUFFER_SIZE) catch self.buf; + } + + fn ensureCapacity(self: *Buffer, needed: usize) !void { + if (needed <= self.buf.len) { + return; + } + if (needed > self.max) { + return error.RequestTooLarge; + } + self.buf = try self.allocator.realloc(self.buf, needed); + } + pub fn read(self: *Buffer, socket: posix.socket_t) ![]u8 { const len = self.len; if (len == self.buf.len) { - return error.RequestTooLarge; + if (self.buf.len == self.max) { + return error.RequestTooLarge; + } + // Only the header gets here: its length isn't declared, so we + // double until it fits. A body is sized from Content-Length. + try self.ensureCapacity(@min(self.buf.len * 2, self.max)); } const n = try posix.read(socket, self.buf[len..]); @@ -247,7 +277,7 @@ pub const Connection = struct { live: usize, // acquired and not yet released retain: usize, // min # to keep free_count: usize, // # of connections available in free - buffer_size: usize, // --cdp-max-http-message-size + max_buffer_size: usize, // --cdp-max-http-message-size pub fn init(app: *App) !Pool { const retain = app.config.maxConnections(); @@ -257,7 +287,7 @@ pub const Connection = struct { .free_count = 0, .retain = retain, .allocator = app.allocator, - .buffer_size = app.config.cdpMaxHTTPMessageSize(), + .max_buffer_size = app.config.cdpMaxHTTPMessageSize(), }; errdefer self.deinit(); @@ -302,7 +332,7 @@ pub const Connection = struct { conn.address = .{ .ip4 = .unspecified(0) }; conn.deadline = 0; conn.pending = null; - conn.buffer.len = 0; + conn.buffer.reset(); conn.state = .header; self.free.prepend(&conn.node); @@ -320,7 +350,7 @@ pub const Connection = struct { .deadline = 0, .pending = null, .state = .header, - .buffer = try .init(allocator, self.buffer_size), + .buffer = try .init(allocator, self.max_buffer_size), }; return conn; } @@ -335,6 +365,11 @@ pub const Connection = struct { // How long a connection may sit without completing a request before we close it. pub const IDLE_TIMEOUT_MS = 10_000; +// Default buffer size of a new connection. For CDP connections, this should be +// enough for the few HTTP requests that it makes. WebDriver can send larger +// bodies and the buffer will grow up to --cdp-max-http-message-size as needed +pub const INITIAL_BUFFER_SIZE = 4096; + const REQUEST_ARENA_RETAIN = 8192; pub fn processEvent(server: *Server, conn: *Connection, rw: Server.IOEvent.ReadWrite, now: u64) void { @@ -399,9 +434,12 @@ fn processHTTP(server: *Server, conn: *Connection, now: u64) !bool { switch (http.*) { .header => { const data = try conn.buffer.read(conn.socket); - if (try http.parseHeader(arena, data) == false) { - // don't have a complete header yet - return true; + switch (try http.parseHeader(arena, data)) { + .need => |needed| { + try conn.buffer.ensureCapacity(needed); + return true; + }, + .complete => {}, } if (comptime lp.IS_DEBUG) { // we do have a complete header, the state must have transitioned @@ -423,7 +461,9 @@ fn processHTTP(server: *Server, conn: *Connection, now: u64) !bool { // req lives in http.*; read what we need before resetting it const keepalive = req.keepalive; http.* = .header; - conn.buffer.len = 0; + // safe to free the buffer, a parked command wil have copied + // what it needed from it. + conn.buffer.reset(); if (served == .parked) { // off the loop until its worker answers (resumeParked) @@ -1119,3 +1159,68 @@ fn webSocketAccept(head: []const u8, out: *[28]u8) ![]const u8 { _ = std.base64.standard.Encoder.encode(out, &sha); return out; } + +const testing = @import("../testing.zig"); + +test "http: the read buffer grows with the request and gives the space back" { + var pair: [2]posix.socket_t = undefined; + if (std.c.socketpair(posix.AF.LOCAL, posix.SOCK.STREAM, 0, &pair) != 0) { + return error.SocketPairFailed; + } + defer sys_net.close(pair[0]); + defer sys_net.close(pair[1]); + + const max = INITIAL_BUFFER_SIZE * 2; + var buffer = try Connection.Buffer.init(testing.allocator, max); + defer buffer.deinit(); + + // a connection commits the initial size, never the limit + try testing.expectEqual(INITIAL_BUFFER_SIZE, buffer.buf.len); + + // a header declares no length, so the buffer doubles to take it + const filler = "a" ** max; + try sys_net.writeAll(pair[1], filler); + while (buffer.len < filler.len) { + _ = try buffer.read(pair[0]); + } + try testing.expectEqual(max, buffer.buf.len); + + // and stops doubling at the limit + try sys_net.writeAll(pair[1], "a"); + try testing.expectError(error.RequestTooLarge, buffer.read(pair[0])); + + // the next request on this connection starts small again + buffer.reset(); + try testing.expectEqual(0, buffer.len); + try testing.expectEqual(INITIAL_BUFFER_SIZE, buffer.buf.len); +} + +test "http: a declared body is sized upfront" { + var pair: [2]posix.socket_t = undefined; + if (std.c.socketpair(posix.AF.LOCAL, posix.SOCK.STREAM, 0, &pair) != 0) { + return error.SocketPairFailed; + } + defer sys_net.close(pair[0]); + defer sys_net.close(pair[1]); + + var buffer = try Connection.Buffer.init(testing.allocator, 1024 * 1024); + defer buffer.deinit(); + + var state: Connection.State = .header; + const body_len = INITIAL_BUFFER_SIZE * 4; + var head_buf: [64]u8 = undefined; + const head = try std.fmt.bufPrint(&head_buf, "POST /session HTTP/1.1\r\nContent-Length: {d}\r\n\r\n", .{body_len}); + try sys_net.writeAll(pair[1], head); + + // the header alone is enough to know how much room the body needs + const needed = switch (try state.parseHeader(testing.allocator, try buffer.read(pair[0]))) { + .complete => return error.UnexpectedlyComplete, + .need => |n| n, + }; + try testing.expectEqual(head.len + body_len, needed); + try buffer.ensureCapacity(needed); + try testing.expectEqual(needed, buffer.buf.len); + + // a body that can't fit is rejected without reading any of it + try testing.expectError(error.RequestTooLarge, buffer.ensureCapacity(buffer.max + 1)); +}