Skip to content

Commit 94a391a

Browse files
committed
fix(http): fix proxy memory leak and HTTPS proxy tunnel not triggered from env vars
Use ArenaAllocator for proxy-related allocations in std.http.Client. Client.deinit() does not free proxy objects (they're externally-owned), so without an arena those allocations leak — detected by DebugAllocator. Also expand the CONNECT tunnel workaround to check environment variables (http_proxy, https_proxy, etc.), not just the explicit proxy_url setting. Previously, when a proxy was auto-detected from environment variables, HTTPS requests fell through to the broken connectProxied() path that returns HTTP 400. Fixes two issues seen when running 'zvm remote' with http_proxy set: - DebugAllocator reports leaked proxy memory - 'Failed to fetch version map' due to connectProxied() returning 400 Affected functions: - http_client: downloadToFileWithProxy, downloadToMemoryWithProxy - mirror_probe: probeThreadMainWindows
1 parent e3ef3a9 commit 94a391a

2 files changed

Lines changed: 30 additions & 13 deletions

File tree

src/network/http_client.zig

Lines changed: 24 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -53,17 +53,25 @@ pub fn downloadToFileWithProxy(
5353
const uri = try std.Uri.parse(url);
5454
const protocol = std.http.Client.Protocol.fromUri(uri) orelse return error.DownloadFailed;
5555

56-
// For HTTPS with proxy, manually establish a CONNECT tunnel.
57-
if (protocol == .tls and proxy_url.len > 0) {
58-
const proxy_info = proxy_tunnel.resolveProxy(allocator, environ_map, proxy_url);
56+
// Arena for proxy-related allocations (Proxy objects, host strings).
57+
// std.http.Client.deinit() does not free proxy objects (they're externally-owned),
58+
// so we manage their lifetime via this arena.
59+
var proxy_arena: std.heap.ArenaAllocator = .init(allocator);
60+
defer proxy_arena.deinit();
61+
62+
// For HTTPS through a proxy, manually establish a CONNECT tunnel.
63+
// Workaround for Zig's std.http.Client.connectProxied() returning 400.
64+
// Checks both explicit proxy_url and environment variables (http_proxy, etc.).
65+
if (protocol == .tls) {
66+
const proxy_info = proxy_tunnel.resolveProxy(proxy_arena.allocator(), environ_map, proxy_url);
5967
if (proxy_info) |pi| {
6068
return downloadFileViaProxyTunnel(allocator, io, uri, pi.host, pi.port, dest_path, progress_writer);
6169
}
6270
}
6371

6472
var client: std.http.Client = .{ .allocator = allocator, .io = io };
6573
defer client.deinit();
66-
initClientProxy(&client, allocator, environ_map, proxy_url);
74+
initClientProxy(&client, proxy_arena.allocator(), environ_map, proxy_url);
6775

6876
var req = try client.request(.GET, uri, .{
6977
.redirect_behavior = .init(5),
@@ -263,23 +271,28 @@ pub fn downloadToMemoryWithProxy(
263271
url: []const u8,
264272
proxy_url: []const u8,
265273
) ![]const u8 {
266-
var client: std.http.Client = .{ .allocator = allocator, .io = io };
267-
defer client.deinit();
274+
var proxy_arena: std.heap.ArenaAllocator = .init(allocator);
275+
defer proxy_arena.deinit();
268276

269277
const uri = try std.Uri.parse(url);
270278
const protocol = std.http.Client.Protocol.fromUri(uri) orelse return error.DownloadFailed;
271279

272-
// For HTTPS with proxy, manually establish a CONNECT tunnel.
280+
// For HTTPS through a proxy, manually establish a CONNECT tunnel.
273281
// Workaround for Zig's std.http.Client.connectProxied() returning 400.
274-
if (protocol == .tls and proxy_url.len > 0) {
275-
const proxy_info = proxy_tunnel.resolveProxy(allocator, environ_map, proxy_url);
282+
// Checks both explicit proxy_url and environment variables (http_proxy, etc.).
283+
if (protocol == .tls) {
284+
const proxy_info = proxy_tunnel.resolveProxy(proxy_arena.allocator(), environ_map, proxy_url);
276285
if (proxy_info) |pi| {
277286
return downloadViaProxyTunnel(allocator, io, uri, pi.host, pi.port);
278287
}
279288
}
280289

281-
// No proxy — use client's built-in proxy support
282-
initClientProxy(&client, allocator, environ_map, proxy_url);
290+
var client: std.http.Client = .{ .allocator = allocator, .io = io };
291+
defer client.deinit();
292+
// No proxy for HTTPS, or plain HTTP — use client's built-in proxy support.
293+
// Use proxy_arena for proxy allocations: std.http.Client.deinit() does not free
294+
// proxy objects (they're externally-owned), so we manage their lifetime via the arena.
295+
initClientProxy(&client, proxy_arena.allocator(), environ_map, proxy_url);
283296

284297
var body_buf: [10 * 1024 * 1024]u8 = undefined;
285298
var body_writer: std.Io.Writer = .fixed(&body_buf);

src/network/mirror_probe.zig

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -178,12 +178,16 @@ fn probeThreadMainWindows(ctx: *ProbeThreadContext) void {
178178
const io = ctx.io orelse return;
179179
const environ_map = ctx.environ_map orelse return;
180180

181+
// Arena for proxy-related allocations: std.http.Client.deinit() does not free
182+
// proxy objects (they're externally-owned), so we manage their lifetime via the arena.
183+
var proxy_arena: std.heap.ArenaAllocator = .init(ctx.allocator);
181184
var client: std.http.Client = .{ .allocator = ctx.allocator, .io = io };
185+
defer proxy_arena.deinit();
182186
defer client.deinit();
183187
if (ctx.proxy.len > 0) {
184-
proxy_tunnel.setProxyFromUrl(&client, ctx.allocator, ctx.proxy) catch {};
188+
proxy_tunnel.setProxyFromUrl(&client, proxy_arena.allocator(), ctx.proxy) catch {};
185189
} else {
186-
client.initDefaultProxies(ctx.allocator, environ_map) catch {};
190+
client.initDefaultProxies(proxy_arena.allocator(), environ_map) catch {};
187191
}
188192

189193
const uri = std.Uri.parse(ctx.url) catch return;

0 commit comments

Comments
 (0)