std.http.Client: api does not assert methods are called in the right order

[nektro]

Zig Version

0.11.0-dev.2297+28d6dd75a

Steps to Reproduce and Observed Behavior

const std = @import("std");

pub fn main() !void {
    var arena = std.heap.ArenaAllocator.init(std.heap.page_allocator);
    const alloc = arena.allocator();

    var client = std.http.Client{ .allocator = alloc };
    try client.ca_bundle.rescan(alloc);

    const the_url = "http://ipv4.download.thinkbroadband.com/512MB.zip";

    const url = try std.Uri.parse(the_url);
    var req = try client.request(url, .{}, .{});

    const length = req.response.headers.content_length.?;
    std.log.debug("length: {d}", .{length});
}

[meghan@nixos]$ zig2 run test.zig 
debug: length: 12297829382473034410

Expected Behavior

HTTP/1.1 200 OK
Server: nginx/1.18.0
Date: Tue, 28 Mar 2023 01:40:50 GMT
Content-Type: application/zip
Content-Length: 536870912
Last-Modified: Fri, 30 May 2008 14:27:49 GMT
Connection: keep-alive
ETag: "48400ee5-20000000"
Access-Control-Allow-Origin: *
Accept-Ranges: bytes

[ehaas]

The API expects you to call the read function on the request object (e.g. const bytes_read = try req.read(buf);. The response init functions (initDynamic / initStatic) explicitly set the response headers to undefined, so that's why you're seeing that before you read any response data in.

[nektro]

req.read should only be for getting the body, not initializing the headers too. or it should be like zfetch and have a req.do()

[jcalabro]

I ran in to a similar issue today, and also would love to see a req.do() similar to go's http.Client.Do.

[jcalabro]

After a bit of poking around, I think that the intended usage was probably something like the following, which appears to do the right thing. I can submit a Request.do function for PR that essentially wraps this functionality if you think that's the best course of action?

const std = @import("std");
const http = std.http;

pub fn main() !void {
    var alloc = std.heap.ArenaAllocator.init(std.heap.page_allocator);
    defer alloc.deinit();
    var allocator = alloc.allocator();

    const uri = try std.Uri.parse("http://localhost:8080/my/url");

    var client = http.Client{ .allocator = allocator };
    defer client.deinit();

    var req = try client.request(uri, .{ .method = .POST }, .{});
    defer req.deinit();

    // after this call, the response headers are populated and can be used
    try req.waitForCompleteHead();

    try req.finish();

    var buf = try allocator.alloc(u8, req.response.headers.content_length.?);
    defer allocator.free(buf);

    _ = try req.read(buf);

    std.debug.print("got response: {s}\n", .{buf});
    std.debug.print("status: {d}\n", .{req.response.headers.status});
}

[nektro]

ah i was unaware of req.waitForCompleteHead(), lemme try using that in my original code. if that works im personally happy to leave it as is until the stdlib audit comes around

[jcalabro]

Sounds good to me.

[truemedian]

I'll admit waitForCompleteHead was a rather... shortsighted name that just never got changed. It's going to change as part of some other reworking changes in #15123

[andrewrk]

Can we get an issue title that correctly reflects whether or not this is a bug?