From 310a579efcaa4d7826276f07ca6cae501d841444 Mon Sep 17 00:00:00 2001 From: Amilia MacIntyre Date: Tue, 21 Jul 2026 15:11:55 -0400 Subject: [PATCH] std.http: improve tolerance of request-body edge cases --- lib/std/http.zig | 65 +++++++++++++------ lib/std/http/Client.zig | 22 +++++-- lib/std/http/Server.zig | 7 +- lib/std/http/test.zig | 137 ++++++++++++++++++++++++++++++++++++++++ 4 files changed, 201 insertions(+), 30 deletions(-) diff --git a/lib/std/http.zig b/lib/std/http.zig index c2966cb2558f0c0ebee2f1e2d1bdcf6a78ad8356..801b7ab17a2d5b18808d444d5bf085b74704c063 100644 --- a/lib/std/http.zig +++ b/lib/std/http.zig @@ -310,6 +310,11 @@ pub const Connection = enum { close, }; +pub const Side = enum { + request, + response, +}; + pub const Header = struct { name: []const u8, value: []const u8, @@ -333,6 +338,8 @@ pub const Reader = struct { trailers: []const u8 = &.{}, body_err: ?BodyError = null, max_head_len: usize, + /// Whether this is a reader over a request, sent by a client; or a response, sent by a server. + side: Side, pub const RemainingChunkLen = enum(u64) { head = 0, @@ -458,8 +465,17 @@ pub const Reader = struct { }; return &reader.interface; } else { - reader.state = .body_none; - return reader.in; + return switch (reader.side) { + .response => resp: { + reader.state = .body_none; + break :resp reader.in; + }, + .request => req: { + // RFC7230: If a request has no Content-Length and no Transfer-Encoding, it has no message body. + reader.state = .ready; + break :req .ending; + }, + }; } }, } @@ -746,6 +762,9 @@ pub const BodyWriter = struct { http_protocol_output: *Writer, state: State, writer: Writer, + /// Associated `Client.Request`, if applicable, used to tell the request + /// that the body has been written and the connection is safe to reuse. + client_request: ?*Client.Request = null, pub const Error = Writer.Error; @@ -824,8 +843,13 @@ pub const BodyWriter = struct { assert(len == 0); // Trips when end() called before all bytes written. w.state = .end; }, - .none => {}, .chunk_len => return endChunkedUnflushed(w, .{}), + // Don't clear payload_pending to prevent an unframed request from + // returning to the pool + .none => return, + } + if (w.client_request) |request| { + request.payload_pending = false; } } @@ -861,26 +885,27 @@ pub const BodyWriter = struct { /// * `endUnflushed` /// * `end` pub fn endChunkedUnflushed(w: *BodyWriter, options: EndChunkedOptions) Error!void { - if (w.isEliding()) { - w.state = .end; - return; - } - const bw = w.http_protocol_output; - switch (w.state.chunk_len) { - 0 => {}, - 1 => unreachable, // Wrote more data than specified in chunk header. - 2 => try bw.writeAll("\r\n"), - else => unreachable, // An earlier write call indicated more data would follow. - } - try bw.writeAll("0\r\n"); - for (options.trailers) |trailer| { - try bw.writeAll(trailer.name); - try bw.writeAll(": "); - try bw.writeAll(trailer.value); + if (!w.isEliding()) { + const bw = w.http_protocol_output; + switch (w.state.chunk_len) { + 0 => {}, + 1 => unreachable, // Wrote more data than specified in chunk header. + 2 => try bw.writeAll("\r\n"), + else => unreachable, // An earlier write call indicated more data would follow. + } + try bw.writeAll("0\r\n"); + for (options.trailers) |trailer| { + try bw.writeAll(trailer.name); + try bw.writeAll(": "); + try bw.writeAll(trailer.value); + try bw.writeAll("\r\n"); + } try bw.writeAll("\r\n"); } - try bw.writeAll("\r\n"); w.state = .end; + if (w.client_request) |request| { + request.payload_pending = false; + } } pub fn contentLengthDrain(w: *Writer, data: []const []const u8, splat: usize) Error!usize { diff --git a/lib/std/http/Client.zig b/lib/std/http/Client.zig index b0f94616719156dc94bedd7aa5bdde4283ee82d0..28b3d007a446bef0520b0c29881398f3306422d0 100644 --- a/lib/std/http/Client.zig +++ b/lib/std/http/Client.zig @@ -731,7 +731,7 @@ pub const Response = struct { pub fn reader(response: *Response, transfer_buffer: []u8) *Reader { response.head.invalidateStrings(); const req = response.request; - if (!req.method.responseHasBody()) return .ending; + if (req.method == .HEAD) return .ending; const head = &response.head; return req.reader.bodyReader(transfer_buffer, head.transfer_encoding, head.content_length); } @@ -807,11 +807,14 @@ pub const Request = struct { headers: Headers, /// Populated in `receiveHead`; used in `deinit` to determine whether to - /// discard the body to reuse the connection. + /// discard the response body to reuse the connection. response_content_length: ?u64 = null, /// Populated in `receiveHead`; used in `deinit` to determine whether to - /// discard the body to reuse the connection. + /// discard the response body to reuse the connection. response_transfer_encoding: http.TransferEncoding = .none, + /// Used in `deinit` to determine whether the payload has been sent, making + /// it safe to reuse the connection. + payload_pending: bool = false, /// These headers are kept including when following a redirect to a /// different domain. @@ -888,8 +891,9 @@ pub const Request = struct { connection.closing = connection.closing or switch (r.reader.state) { .ready => false, .received_head => c: { - if (r.method.requestHasBody()) break :c true; - if (!r.method.responseHasBody()) break :c false; + if (r.payload_pending) break :c true; + if (r.method == .HEAD) break :c false; + const reader = r.reader.bodyReader(&.{}, r.response_transfer_encoding, r.response_content_length); _ = reader.discardRemaining() catch |err| switch (err) { error.ReadFailed => break :c true, @@ -912,7 +916,6 @@ pub const Request = struct { /// Sends but does not flush a complete request as only HTTP head, no body. pub fn sendBodilessUnflushed(r: *Request) Writer.Error!void { assert(r.transfer_encoding == .none); - assert(!r.method.requestHasBody()); try sendHead(r); } @@ -941,13 +944,14 @@ pub const Request = struct { /// See also: /// * `sendBody` pub fn sendBodyUnflushed(r: *Request, buffer: []u8) Writer.Error!http.BodyWriter { - assert(r.method.requestHasBody()); + r.payload_pending = true; try sendHead(r); const http_protocol_output = r.connection.?.writer(); return switch (r.transfer_encoding) { .chunked => .{ .http_protocol_output = http_protocol_output, .state = .init_chunked, + .client_request = r, .writer = .{ .buffer = buffer, .vtable = &.{ @@ -959,6 +963,7 @@ pub const Request = struct { .content_length => |len| .{ .http_protocol_output = http_protocol_output, .state = .{ .content_length = len }, + .client_request = r, .writer = .{ .buffer = buffer, .vtable = &.{ @@ -970,6 +975,7 @@ pub const Request = struct { .none => .{ .http_protocol_output = http_protocol_output, .state = .none, + .client_request = r, .writer = .{ .buffer = buffer, .vtable = &.{ @@ -1273,6 +1279,7 @@ pub const Request = struct { // Populated when `http.Reader.bodyReader` is called. .interface = undefined, .max_head_len = r.client.read_buffer_size, + .side = .response, }; r.redirect_behavior.subtractOne(); } @@ -1744,6 +1751,7 @@ pub fn request( // Populated when `http.Reader.bodyReader` is called. .interface = undefined, .max_head_len = client.read_buffer_size, + .side = .response, }, .keep_alive = options.keep_alive, .method = method, diff --git a/lib/std/http/Server.zig b/lib/std/http/Server.zig index c505605c03406e3ef760fabe031175358a207cd5..bd39ee29d4a95c7e95561e94e78007df15472228 100644 --- a/lib/std/http/Server.zig +++ b/lib/std/http/Server.zig @@ -30,6 +30,7 @@ pub fn init(in: *Reader, out: *Writer) Server { // Populated when `http.Reader.bodyReader` is called. .interface = undefined, .max_head_len = in.buffer.len, + .side = .request, }, .out = out, }; @@ -253,6 +254,7 @@ pub const Request = struct { .state = .received_head, .interface = undefined, .max_head_len = 4096, + .side = .request, }, .out = undefined, }; @@ -592,7 +594,7 @@ pub const Request = struct { assert(request.server.reader.state == .received_head); assert(request.head.expect == null); request.head.invalidateStrings(); - if (!request.head.method.requestHasBody()) return .ending; + if (request.head.method == .HEAD) return .ending; return request.server.reader.bodyReader(buffer, request.head.transfer_encoding, request.head.content_length); } @@ -627,8 +629,7 @@ pub const Request = struct { const r = &request.server.reader; if (keep_alive and request.head.keep_alive) switch (r.state) { .received_head => { - if (request.head.method.requestHasBody()) { - assert(request.head.transfer_encoding != .none or request.head.content_length != null); + if (request.head.transfer_encoding != .none or request.head.content_length != null) { const reader_interface = request.readerExpectContinue(&.{}) catch return false; _ = reader_interface.discardRemaining() catch return false; assert(r.state == .ready); diff --git a/lib/std/http/test.zig b/lib/std/http/test.zig index 13f2fd1158b7adf89ac31152914f481cb45779f4..92ced35bc0d00fe29eda578239bfd4e9b654db31 100644 --- a/lib/std/http/test.zig +++ b/lib/std/http/test.zig @@ -17,6 +17,7 @@ test "content length reader state update" { .interface = undefined, .state = .ready, .max_head_len = 1024, + .side = .request, }; _ = try reader.receiveHead(); @@ -1123,6 +1124,27 @@ fn echoTests(client: *http.Client, port: u16) !void { try expectEqual(.expectation_failed, response.head.status); _ = try response.reader(&.{}).discardRemaining(); } + + { // Send GET with body + const location = try std.fmt.allocPrint(gpa, "http://127.0.0.1:{d}/echo-content#get-with-body", .{port}); + defer gpa.free(location); + + var body: std.Io.Writer.Allocating = .init(gpa); + defer body.deinit(); + try body.ensureUnusedCapacity(64); + + const res = try client.fetch(.{ + .location = .{ .url = location }, + .method = .GET, + .payload = "Hello, World!\n", + .extra_headers = &.{ + .{ .name = "content-type", .value = "text/plain" }, + }, + .response_writer = &body.writer, + }); + try expectEqual(.ok, res.status); + try expectEqualStrings("Hello, World!\n", body.written()); + } } const TestServer = struct { @@ -1320,3 +1342,118 @@ test "boot failed connections from the pool" { try expectError(error.HttpConnectionClosing, client.fetch(.{ .location = .{ .uri = uri } })); } } + +test "send and discard or echo empty bodies" { + if (builtin.cpu.arch.isPowerPC64() and builtin.mode != .debug) return error.SkipZigTest; // https://github.com/llvm/llvm-project/issues/194257 + if (builtin.os.tag == .openbsd) return error.SkipZigTest; // https://codeberg.org/ziglang/zig/issues/30806 + + const io = std.testing.io; + const test_server = try createTestServer(io, struct { + fn run(test_server: *TestServer) anyerror!void { + const net_server = &test_server.net_server; + var recv_buffer: [0x100]u8 = undefined; + var send_buffer: [0x100]u8 = undefined; + + accept: while (!test_server.shutting_down) { + var stream = try net_server.accept(io); + defer stream.close(io); + + var connection_br = stream.reader(io, &recv_buffer); + var connection_bw = stream.writer(io, &send_buffer); + var http_server = http.Server.init(&connection_br.interface, &connection_bw.interface); + + while (http_server.reader.state == .ready) { + var request = http_server.receiveHead() catch |err| switch (err) { + error.HttpConnectionClosing => continue :accept, + else => |e| return e, + }; + if (mem.eql(u8, request.head.target, "/discard")) { + try request.respond("ignored", .{}); + } else if (mem.eql(u8, request.head.target, "/echo")) { + const body = try (try request.readerExpectContinue(&.{})).allocRemaining(std.testing.allocator, .unlimited); + try request.respond(body, .{}); + } + } + } + } + }); + defer test_server.destroy(); + + const gpa = std.testing.allocator; + var client: http.Client = .{ .allocator = gpa, .io = io }; + defer client.deinit(); + + const port = test_server.port(); + + const location_discard = try std.fmt.allocPrint(gpa, "http://127.0.0.1:{d}/discard", .{port}); + defer gpa.free(location_discard); + const uri_discard = try std.Uri.parse(location_discard); + + const location_echo = try std.fmt.allocPrint(gpa, "http://127.0.0.1:{d}/echo", .{port}); + defer gpa.free(location_echo); + const uri_echo = try std.Uri.parse(location_echo); + + { // Echo POST with sendBodiless + var response_buffer: [0x100]u8 = undefined; + + var request = try client.request(.POST, uri_echo, .{}); + defer request.deinit(); + try request.sendBodiless(); + var response = try request.receiveHead(&response_buffer); + const body = try response.reader(&.{}).allocRemaining(gpa, .unlimited); + defer gpa.free(body); + + try expectEqualStrings("", body); + } + + // connection has been kept alive + try expect(client.http_proxy != null or client.connection_pool.free_len == 1); + + { // Discard POST with sendBodiless + var response_buffer: [0x100]u8 = undefined; + + var request = try client.request(.POST, uri_discard, .{}); + defer request.deinit(); + try request.sendBodiless(); + var response = try request.receiveHead(&response_buffer); + const body = try response.reader(&.{}).allocRemaining(gpa, .unlimited); + defer gpa.free(body); + + try expectEqualStrings("ignored", body); + } + + // connection has been kept alive + try expect(client.http_proxy != null or client.connection_pool.free_len == 1); + + { // Discard POST with empty fetch payload + var body: std.Io.Writer.Allocating = .init(gpa); + defer body.deinit(); + + _ = try client.fetch(.{ + .method = .POST, + .location = .{ .uri = uri_discard }, + .response_writer = &body.writer, + .payload = "", + }); + try expectEqualStrings("ignored", body.written()); + } + + // connection has been kept alive + try expect(client.http_proxy != null or client.connection_pool.free_len == 1); + + { // Echo POST with empty fetch payload + var body: std.Io.Writer.Allocating = .init(gpa); + defer body.deinit(); + + _ = try client.fetch(.{ + .method = .POST, + .location = .{ .uri = uri_echo }, + .response_writer = &body.writer, + .payload = "", + }); + try expectEqualStrings("", body.written()); + } + + // connection has been kept alive + try expect(client.http_proxy != null or client.connection_pool.free_len == 1); +} -- 2.54.0