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

@@ -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,

src/help.zon

@@ -28,8 +28,9 @@
     \\          Maximum number of simultaneous CDP connections.
     \\          Defaults to 16.
     \\  --cdp-max-http-message-size <INT>
-    \\          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 <INT>
     \\          Maximum allowed incoming websocket message size.
     \\          Defaults to 1048576 (1MB)

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);
 }

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,

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));
+}