fix(http): fix proxy memory leak and HTTPS proxy tunnel not triggered from env vars#13
Merged
Merged
Conversation
… 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
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
When running
zvm remote(or any command that fetches from a remote URL) with an HTTP proxy configured via environment variables (http_proxy,https_proxy), two issues occur:DebugAllocatorreports leaked memory fromcreateProxyFromEnvVarandinitDefaultProxiesinstd.http.Client.Failed to fetch version mapbecause HTTPS requests go through the brokenconnectProxied()code path that returns HTTP 400.Root Cause
Memory Leak
std.http.Client.deinit()does not freehttp_proxy/https_proxyobjects -- the standard library comments them as "Pointer to externally-owned memory". TheinitDefaultProxies()function's API is designed for the caller to pass anArenaAllocatorwhose lifetime the caller manages. However, the code was passing the rawDebugAllocatordirectly, so the proxy objects allocated on the heap were never freed.Fetch Failure
The codebase has a workaround for
connectProxied()returning HTTP 400: it manually establishes an HTTP CONNECT tunnel for HTTPS through proxies. But this workaround was gated onproxy_url.len > 0-- i.e., only triggered when a proxy was explicitly configured viazvm proxy set. When the proxy was auto-detected from environment variables (the common case), the condition was false, and the request fell through to the brokenclient.fetch()->connectProxied()path.Fix
ArenaAllocator for proxy allocations
Create an
ArenaAllocator(proxy_arena) in each function that creates anstd.http.Client, pass its allocator toinitClientProxy, anddefer proxy_arena.deinit()to release all proxy memory when the function exits.Expand CONNECT tunnel condition
Change the condition from:
to:
and use
resolveProxy()(which already checks both explicit URL and environment variables) to determine whether a proxy is available. This ensures the CONNECT tunnel workaround applies regardless of proxy source.Affected Files
src/network/http_client.zig--downloadToFileWithProxy,downloadToMemoryWithProxysrc/network/mirror_probe.zig--probeThreadMainWindows(same pattern, Windows-only code path)Testing
zig build-- passeszig build test-- passeszig build run -- remote-- works correctly withhttp_proxyset, no memory leak errors, no fetch failures