Skip to content

prefs: Handle uppercase HTTP_PROXY variable#41268

Merged
yezhizhen merged 3 commits intoservo:mainfrom
treeshateorcs:patch-1
Dec 15, 2025
Merged

prefs: Handle uppercase HTTP_PROXY variable#41268
yezhizhen merged 3 commits intoservo:mainfrom
treeshateorcs:patch-1

Conversation

@treeshateorcs
Copy link
Copy Markdown
Contributor

@treeshateorcs treeshateorcs commented Dec 15, 2025

follow up to #41209

#41209 does not read HTTP_PROXY (all caps) or `HtTp_PrOxY" or w/e. i personally have the variable in all caps. I propose we check both "HTTP_PROXY" and "http_proxy", but not mixed case. Firefox does just that at https://github.com/mozilla-firefox/firefox/blob/bef781bbd7a225c428c2444d7d02e9f6eb327e94/security/sandbox/chromium/base/environment.cc#L29-L42

@servo-highfive servo-highfive added the S-awaiting-review There is new code that needs to be reviewed. label Dec 15, 2025
Copy link
Copy Markdown
Member

@yezhizhen yezhizhen left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Firefox does just that at https://github.com/mozilla-firefox/firefox/blob/bef781bbd7a225c428c2444d7d02e9f6eb327e94/security/sandbox/chromium/base/environment.cc#L29-L42

Firefox only do this when default one not found. Besides, lowercase http_proxy is more widely recognized.

does not read HTTP_PROXY (all caps) or `HtTp_PrOxY" or w/e.

For Windows, environment variable are case-insensitive. But for linux it is case-sensitive: but still, only http_proxy or HTTP_PROXY are valid but not all combinations :)

@servo-highfive servo-highfive added S-needs-code-changes Changes have not yet been made that were requested by a reviewer. and removed S-awaiting-review There is new code that needs to be reviewed. labels Dec 15, 2025
Co-authored-by: Euclid Ye <[email protected]>
Signed-off-by: tho <[email protected]>
@servo-highfive servo-highfive added S-awaiting-review There is new code that needs to be reviewed. and removed S-needs-code-changes Changes have not yet been made that were requested by a reviewer. labels Dec 15, 2025
@servo-highfive servo-highfive removed the S-awaiting-review There is new code that needs to be reviewed. label Dec 15, 2025
@yezhizhen yezhizhen enabled auto-merge December 15, 2025 06:30
Signed-off-by: Euclid Ye <[email protected]>
@servo-highfive servo-highfive added the S-awaiting-review There is new code that needs to be reviewed. label Dec 15, 2025
@yezhizhen yezhizhen changed the title Handle uppercase HTTP_PROXY variable prefs: Handle uppercase HTTP_PROXY variable Dec 15, 2025
@yezhizhen yezhizhen added this pull request to the merge queue Dec 15, 2025
@servo-highfive servo-highfive added the S-awaiting-merge The PR is in the process of compiling and running tests on the automated CI. label Dec 15, 2025
Merged via the queue into servo:main with commit 349674f Dec 15, 2025
33 checks passed
@servo-highfive servo-highfive removed the S-awaiting-merge The PR is in the process of compiling and running tests on the automated CI. label Dec 15, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

S-awaiting-review There is new code that needs to be reviewed.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants