std.http.Client frees not owned memory when proxy is used

Zig Version

0.12.0-dev.2313+bf7ebfa67

Steps to Reproduce and Observed Behavior

The deinit() method of std.http.Client currently assumes that it owns the memory of the underlying proxy.host variable https://github.com/ziglang/zig/blob/9e684e8d1af39904055abe64a9afda69a3d44a59/lib/std/http/Client.zig#L1056-L1079 This is only true, when the proxy has been set using the loadDefaultProxies method https://github.com/ziglang/zig/blob/9e684e8d1af39904055abe64a9afda69a3d44a59/lib/std/http/Client.zig#L1083 If the proxy is set manually by the user, i.e.::

const std = @import("std");
const allocator = std.testing.allocator;
const uri = std.Uri.parse("https://ziglang.org/") catch unreachable;

pub fn main() !void {
    std.debug.print("Run `zig build test` to run the tests.\n", .{});
}

test "test proxy fails with invalid free" {
    const proxy: std.http.Client.Proxy = .{
        .headers = .{
            .allocator = allocator,
        },
        .allocator = allocator,
        .host = "localhost",
        .port = 8080,
        .protocol = .tls,
    };

    var client: std.http.Client = .{
        .allocator = allocator,
        .https_proxy = proxy,
    };
    defer client.deinit();

    var req = try client.open(.GET, uri, .{ .allocator = allocator }, .{});
    defer req.deinit();
    try req.send(.{});
    try req.wait();

    try std.testing.expect(req.response.status == .ok);
}

The code will panic with the following error:

➜  zig_proxy zig build test --summary all --verbose                                                   [10/477]
/home/muser/.zigenv/versions/builds/zig test -ODebug -Mroot=/tmp/zig_proxy/src/main.zig --cache-dir /tmp/zi
g_proxy/zig-cache --global-cache-dir /home/muser/.cache/zig --name test --listen=-
/home/muser/.zigenv/versions/builds/zig test -ODebug -Mroot=/tmp/zig_proxy/src/root.zig --cache-dir /tmp/zi
g_proxy/zig-cache --global-cache-dir /home/muser/.cache/zig --name test --listen=-
steps [3/5] zig test Debug native... /tmp/zig_proxy/zig-cache/o/38746142704976fb5d3eda73780d9ef6/test --listen
=-
test
└─ run test failure
thread 36425 panic: Invalid free
/home/muser/.zigenv/versions/builds/lib/std/heap/general_purpose_allocator.zig:642:21: 0x10a84a1 in freeLar
ge (test)
                    @panic("Invalid free");
                    ^
/home/muser/.zigenv/versions/builds/lib/std/heap/general_purpose_allocator.zig:853:31: 0x1099dda in free (t
est)
                self.freeLarge(old_mem, log2_old_align, ret_addr);
                              ^
/home/muser/.zigenv/versions/builds/lib/std/mem/Allocator.zig:98:28: 0x109f389 in free__anon_5734 (test)
    return self.vtable.free(self.ptr, buf, log2_buf_align, ret_addr);
                           ^
/home/muser/.zigenv/versions/builds/lib/std/http/Client.zig:1071:29: 0x109b49a in deinit (test)
        proxy.allocator.free(proxy.host);
                            ^
/tmp/zig_proxy/src/main.zig:24:24: 0x1098bef in test.test proxy fails with invalid free (test)
    defer client.deinit();
                       ^
/home/muser/.zigenv/versions/builds/lib/test_runner.zig:101:29: 0x10dd696 in mainServer (test)
                test_fn.func() catch |err| switch (err) {
                            ^
/home/muser/.zigenv/versions/builds/lib/test_runner.zig:34:26: 0x10b669d in main (test)
        return mainServer() catch @panic("internal test runner failure");
                         ^
/home/muser/.zigenv/versions/builds/lib/std/start.zig:575:22: 0x10a663c in posixCallMainAndExit (test)
            root.main();
                     ^
/home/muser/.zigenv/versions/builds/lib/std/start.zig:253:5: 0x10a6191 in _start (test)
    asm volatile (switch (native_arch) {

A workaround would be to force the user to allocate the host variable of the proxy on the heap and explicitly not freeing it, i.e.:

test "test proxy workaround" {
    const proxy: std.http.Client.Proxy = .{
        .headers = .{
            .allocator = allocator,
        },
        .allocator = allocator,
        .host = try allocator.dupe(u8, "localhost"),
        .port = 8080,
        .protocol = .tls,
    };

    var client: std.http.Client = .{
        .allocator = allocator,
        .https_proxy = proxy,
    };
    defer client.deinit();

    var req = try client.open(.GET, uri, .{ .allocator = allocator }, .{});
    defer req.deinit();
    try req.send(.{});
    try req.wait();

    try std.testing.expect(req.response.status == .ok);
}

Expected Behavior

The deinit() method should not attempt to free memory that it does not own (avoiding the Invalid free error). This could be achieved by tracking if loadDefaultProxies has been called and freeing the memory of proxy.host only in this case. My alternative proposal would be to change the loadDefaultProxies method such that it returns the proxy - making the user in charge of freeing the memory I.e:

test "test proxy new loadDefaultProxies method" {
    const proxies: std.http.Client.DefaultProxies = std.http.Client.loadDefaultProxies(allocator);
    defer proxies.deinit(allocator);

    var client: std.http.Client = .{
        .allocator = allocator,
        .https_proxy = proxies.https_proxy.?,
    };
    defer client.deinit();

    var req = try client.open(.GET, uri, .{ .allocator = allocator }, .{});
    defer req.deinit();
    try req.send(.{});
    try req.wait();

    try std.testing.expect(req.response.status == .ok);
}

ziglang / zig