Ignore HTTPS_PROXY and NO_PROXY if env variable is set to empty value#1988
Merged
BillyONeal merged 3 commits intomicrosoft:mainfrom Apr 15, 2026
Merged
Ignore HTTPS_PROXY and NO_PROXY if env variable is set to empty value#1988BillyONeal merged 3 commits intomicrosoft:mainfrom
BillyONeal merged 3 commits intomicrosoft:mainfrom
Conversation
Contributor
There was a problem hiding this comment.
Pull request overview
Updates the Windows tls12-download bootstrap downloader’s proxy environment-variable handling so that an explicitly empty proxy variable is treated as “unset” (rather than triggering an error path), aligning behavior with common proxy-env conventions.
Changes:
- Add
abort_api_failure_with_last_error()to preserve the originating Win32 error code across intermediate calls. - Change
GetEnvironmentVariableW()handling forHTTPS_PROXY/NO_PROXYto treat an empty value as “no proxy / no bypass” by explicitly initializing and then checkingGetLastError().
Comment on lines
+217
to
223
| SetLastError(ERROR_SUCCESS); | ||
| DWORD get_result = GetEnvironmentVariableW(L"HTTPS_PROXY", https_proxy_env, sizeof(https_proxy_env) / sizeof(wchar_t)); | ||
| DWORD last_error = GetLastError(); | ||
| if (get_result) | ||
| { | ||
| access_type = WINHTTP_ACCESS_TYPE_NAMED_PROXY; | ||
| proxy_setting = https_proxy_env; |
There was a problem hiding this comment.
The PR title refers to HTTP_PROXY, but this code path only reads HTTPS_PROXY (and then falls back to IE proxy). If the intent is to honor/ignore HTTP_PROXY as well, please add handling for it (or clarify/update the PR title to match the implemented behavior).
Contributor
Author
There was a problem hiding this comment.
i have fixed the PR title and commit message
TobiasFunk
changed the title
TobiasFunk
changed the title
BillyONeal
reviewed
Apr 15, 2026
Member
BillyONeal
left a comment
BillyONeal
left a comment
There was a problem hiding this comment.
I submitted a nitpick and fix the formatting check for you in TobiasFunk#1
BillyONeal
added a commit
to BillyONeal/vcpkg-tool
that referenced
this pull request
Apr 15, 2026
This is a followup to microsoft#1988 where I observe that the same type of defect applies to the "normal" way vcpkg gets environment variables. Ensure that this is preserved correctly, enable use of the small string optimization for wstring, and audit all callers.
BillyONeal
changed the title
Fix formatting and remove unnecessary parens.
BillyONeal
approved these changes
Apr 15, 2026
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.
No description provided.