Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

ensure HTTP requests use proxy env vars #597

Merged
merged 2 commits into from
Dec 2, 2024

Conversation

ncsc-ie-devs
Copy link
Contributor

Description

This merge request resolves Issue 596 by ensuring that all HTTP and HTTPS traffic uses the proxy configuration specified in the environment variables (HTTP_PROXY, HTTPS_PROXY, NO_PROXY).

Changes Made

  • Updated all instances of http.Transport to include the Proxy field:
    Proxy: http.ProxyFromEnvironment
  • Ensured compatibility with existing TLSClientConfig settings, preserving secure connections.

Motivation

The issue described in Issue 596 identified a shortfall where proxy settings were ignored, causing failures in proxied environments. This fix ensures compliance with Go's standard proxy behavior and improves usability for users in corporate or secured networks.

Verification

  1. Environment Setup:

    • Configured proxy settings using HTTP_PROXY, HTTPS_PROXY, and NO_PROXY.
    • Verified traffic routing through the proxy and bypassing for domains listed in NO_PROXY.
  2. Test Cases:

    • Successful request routing through a proxy server.
    • Proper bypassing of proxy for NO_PROXY-listed domains.

Impact

  • Resolves connection issues for users in proxied environments.
  • Aligns the application with standard Go behavior for net/http proxy handling.

Closes

Closes Issue 596.

Updated all instances of `http.Transport` to include the `Proxy` field set to `http.ProxyFromEnvironment`. This ensures that the application respects proxy configuration defined by the `HTTP_PROXY`, `HTTPS_PROXY`, and `NO_PROXY` environment variables.

### Changes:
- Modified `http.Transport` initialization across the codebase to use:
  ```go
  Proxy: http.ProxyFromEnvironment
  ```
- Ensured TLS configurations remain intact by preserving `TLSClientConfig`.

### Why:
- Previously, HTTP requests bypassed proxy settings due to missing configuration in the transport layer.
- This fix enables compatibility with proxied environments, aligning with standard Go behavior.

### Impact:
- All HTTP and HTTPS traffic now adheres to proxy settings.
- Domains listed in `NO_PROXY` bypass the proxy as expected.

### Verification:
- Tested with proxy environment variables set (`HTTP_PROXY`, `HTTPS_PROXY`).
- Verified requests route through the proxy and `NO_PROXY` works as intended.
@s-l-teichmann s-l-teichmann self-requested a review November 25, 2024 08:13
s-l-teichmann
s-l-teichmann previously approved these changes Nov 25, 2024
Copy link
Contributor

@s-l-teichmann s-l-teichmann left a comment

Choose a reason for hiding this comment

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

LGTM

@koplas
Copy link
Contributor

koplas commented Nov 25, 2024

@ncsc-ie-devs Can you run go fmt ./... on your code?

@tschmidtb51 tschmidtb51 linked an issue Nov 26, 2024 that may be closed by this pull request
@ncsc-ie-devs
Copy link
Contributor Author

@ncsc-ie-devs Can you run go fmt ./... on your code?

That should be done now - sorry about the delay.

@koplas
Copy link
Contributor

koplas commented Nov 29, 2024

LGTM & works

@bernhardreiter
Copy link
Member

@koplas should we additionally add a remark in a general place in the documenation?

Copy link
Contributor

@s-l-teichmann s-l-teichmann left a comment

Choose a reason for hiding this comment

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

LGTM

@s-l-teichmann s-l-teichmann merged commit 1daaed2 into gocsaf:main Dec 2, 2024
3 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

http proxy environment variables not used
4 participants