From 33d2a706a255d6754b1230d9bc644875d27b0944 Mon Sep 17 00:00:00 2001 From: Frank Denis Date: Tue, 16 Sep 2025 14:02:51 +0200 Subject: [PATCH] http.Client: fix handling of 204 No Content responses The HTTP client would hang when trying to read a response body for 204 No Content responses with keep-alive connections. This is because the client incorrectly attempted to read a body based solely on the request method, without considering the response status code. Per RFC 9110 Section 6.3, responses with status codes 204 (No Content), 304 (Not Modified), and 1xx (Informational) must not contain a message body, regardless of what headers might suggest. Fixes #25181 --- lib/std/http/Client.zig | 57 ++++++++++++++++++++++++--- lib/std/http/test.zig | 87 +++++++++++++++++++++++++++++++++++++++++ 2 files changed, 139 insertions(+), 5 deletions(-) diff --git a/lib/std/http/Client.zig b/lib/std/http/Client.zig index 5d6a75cb2841..ad39b3196726 100644 --- a/lib/std/http/Client.zig +++ b/lib/std/http/Client.zig @@ -712,8 +712,21 @@ 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; const head = &response.head; + + // Check if the response should have a body based on method and status code + // Per RFC 9110 Section 6.3: + // Any response to a HEAD request and any response with a 1xx (Informational), + // 204 (No Content), or 304 (Not Modified) status code is always terminated + // by the first empty line after the header fields + if (!req.method.responseHasBody() or + head.status.class() == .informational or + head.status == .no_content or + head.status == .not_modified) + { + return .ending; + } + return req.reader.bodyReader(transfer_buffer, head.transfer_encoding, head.content_length); } @@ -733,8 +746,23 @@ pub const Response = struct { decompress_buffer: []u8, ) *Reader { response.head.invalidateStrings(); + const req = response.request; const head = &response.head; - return response.request.reader.bodyReaderDecompressing( + + // Check if the response should have a body based on method and status code + // Per RFC 9110 Section 6.3: + // Any response to a HEAD request and any response with a 1xx (Informational), + // 204 (No Content), or 304 (Not Modified) status code is always terminated + // by the first empty line after the header fields + if (!req.method.responseHasBody() or + head.status.class() == .informational or + head.status == .no_content or + head.status == .not_modified) + { + return .ending; + } + + return req.reader.bodyReaderDecompressing( transfer_buffer, head.transfer_encoding, head.content_length, @@ -793,6 +821,9 @@ pub const Request = struct { /// Populated in `receiveHead`; used in `deinit` to determine whether to /// discard the body to reuse the connection. response_transfer_encoding: http.TransferEncoding = .none, + /// Populated in `receiveHead`; used in `deinit` to determine whether to + /// discard the body to reuse the connection. + response_status: ?http.Status = null, /// These headers are kept including when following a redirect to a /// different domain. @@ -869,7 +900,18 @@ pub const Request = struct { .ready => false, .received_head => c: { if (r.method.requestHasBody()) break :c true; - if (!r.method.responseHasBody()) break :c false; + + // Check if response should have a body based on method and status + const has_body = if (r.response_status) |status| + r.method.responseHasBody() and + status.class() != .informational and + status != .no_content and + status != .not_modified + else + r.method.responseHasBody(); + + if (!has_body) 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, @@ -1117,6 +1159,7 @@ pub const Request = struct { if (head.status == .@"continue") { if (r.handle_continue) continue; + r.response_status = head.status; r.response_transfer_encoding = head.transfer_encoding; r.response_content_length = head.content_length; return response; // we're not handling the 100-continue @@ -1130,6 +1173,7 @@ pub const Request = struct { if (r.method == .CONNECT and head.status.class() == .success) { // This connection is no longer doing HTTP. connection.closing = false; + r.response_status = head.status; r.response_transfer_encoding = head.transfer_encoding; r.response_content_length = head.content_length; return response; @@ -1145,8 +1189,10 @@ pub const Request = struct { if (r.method == .HEAD or head.status.class() == .informational or head.status == .no_content or head.status == .not_modified) { - r.response_transfer_encoding = head.transfer_encoding; - r.response_content_length = head.content_length; + r.response_status = head.status; + // For these status codes, there is no body even if headers suggest otherwise + r.response_transfer_encoding = .none; + r.response_content_length = 0; return response; } @@ -1167,6 +1213,7 @@ pub const Request = struct { if (!r.accept_encoding[@intFromEnum(head.content_encoding)]) return error.HttpContentEncodingUnsupported; + r.response_status = head.status; r.response_transfer_encoding = head.transfer_encoding; r.response_content_length = head.content_length; return response; diff --git a/lib/std/http/test.zig b/lib/std/http/test.zig index 91a57dbbe2e9..9e1997691b5e 100644 --- a/lib/std/http/test.zig +++ b/lib/std/http/test.zig @@ -91,6 +91,93 @@ test "trailers" { try expect(client.connection_pool.free_len == 1); } +test "HTTP client handles 204 No Content response correctly" { + const test_server = try createTestServer(struct { + fn run(test_server: *TestServer) anyerror!void { + const net_server = &test_server.net_server; + var recv_buffer: [1024]u8 = undefined; + var send_buffer: [1024]u8 = undefined; + + const connection = try net_server.accept(); + defer connection.stream.close(); + + var connection_br = connection.stream.reader(&recv_buffer); + var connection_bw = connection.stream.writer(&send_buffer); + var server = http.Server.init(connection_br.interface(), &connection_bw.interface); + + // Test both DELETE and GET methods with 204 response on the same connection + var remaining: usize = 2; + while (remaining != 0) : (remaining -= 1) { + try expectEqual(.ready, server.reader.state); + var request = try server.receiveHead(); + + // Send 204 No Content response + try request.respond("", .{ + .status = .no_content, + .keep_alive = true, + }); + + try expectEqual(.ready, server.reader.state); + } + } + }); + defer test_server.destroy(); + + const gpa = std.testing.allocator; + + var client: http.Client = .{ .allocator = gpa }; + defer client.deinit(); + + const location = try std.fmt.allocPrint(gpa, "http://127.0.0.1:{d}/test204", .{ + test_server.port(), + }); + defer gpa.free(location); + const uri = try std.Uri.parse(location); + + // Test with DELETE method (which normally could have a response body) + { + var req = try client.request(.DELETE, uri, .{}); + defer req.deinit(); + + try req.sendBodiless(); + var response = try req.receiveHead(&.{}); + + try expectEqual(.no_content, response.head.status); + + // The reader should return .ending immediately without trying to read a body + var transfer_buffer: [1024]u8 = undefined; + const reader = response.reader(&transfer_buffer); + + // Ensure no body is read + const body = try reader.allocRemaining(gpa, .unlimited); + defer gpa.free(body); + try expectEqualStrings("", body); + } + + // Test with GET method as well + { + var req = try client.request(.GET, uri, .{}); + defer req.deinit(); + + try req.sendBodiless(); + var response = try req.receiveHead(&.{}); + + try expectEqual(.no_content, response.head.status); + + // The reader should return .ending immediately without trying to read a body + var transfer_buffer: [1024]u8 = undefined; + const reader = response.reader(&transfer_buffer); + + // Ensure no body is read + const body = try reader.allocRemaining(gpa, .unlimited); + defer gpa.free(body); + try expectEqualStrings("", body); + } + + // The connection should have been kept alive and reused + try expect(client.connection_pool.free_len == 1); +} + test "HTTP server handles a chunked transfer coding request" { const test_server = try createTestServer(struct { fn run(test_server: *TestServer) anyerror!void {