Skip to content

http.Client: fix handling of 204 No Content responses #25254

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
jedisct1 wants to merge 1 commit into
base: master
Choose a base branch
from
+139 −5
Open

http.Client: fix handling of 204 No Content responses #25254

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
57 changes: 52 additions & 5 deletions lib/std/http/Client.zig
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}

Expand All @@ -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,
Expand Down Expand Up @@ -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.
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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
Expand All @@ -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;
Expand All @@ -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;
}

Expand All @@ -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;
Expand Down
87 changes: 87 additions & 0 deletions lib/std/http/test.zig
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down