Skip to content

Enable hostname-based proxy routing by default when proxy set#55

Open
poupas wants to merge 1 commit intomainfrom
fix/proxy-names-auto-enable
Open

Enable hostname-based proxy routing by default when proxy set#55
poupas wants to merge 1 commit intomainfrom
fix/proxy-names-auto-enable

Conversation

@poupas
Copy link
Member

@poupas poupas commented Mar 13, 2026

No description provided.

@poupas poupas requested a review from a team as a code owner March 13, 2026 15:27
@snyk-io
Copy link
Contributor

snyk-io bot commented Mar 13, 2026

Snyk checks have passed. No issues have been found so far.

Status Scan Engine Critical High Medium Low Total (0)
Open Source Security 0 0 0 0 0 issues
Licenses 0 0 0 0 0 issues
Code Security 0 0 0 0 0 issues

💻 Catch issues earlier using the plugins for VS Code, JetBrains IDEs, Visual Studio, and Eclipse.

@bbarao
Copy link
Contributor

bbarao commented Mar 13, 2026

🤖 LLM-assisted review comment 🤖

Clean PR, two minor notes:

  1. hasProxyEnv() triggers on NO_PROXY alone (root.go:229) — If only NO_PROXY is set without any actual proxy variables, hasProxyEnv() returns true and enables hostname-based routing. This allocates an IPNameCache and causes an extra DNS re-resolution per TCP connection (the IP gets translated back to a hostname, then net.Dialer resolves it again). In practice this is harmless: the second lookup uses the same system resolver, adds negligible latency, and the NO_PROXY-only scenario is unrealistic. Mentioning it for awareness, not suggesting a change.
 func hasProxyEnv() bool {
-	for _, k := range []string{"HTTP_PROXY", "http_proxy", "HTTPS_PROXY", "https_proxy", "NO_PROXY", "no_proxy"} {
+	for _, k := range []string{"HTTP_PROXY", "http_proxy", "HTTPS_PROXY", "https_proxy"} {
 		if os.Getenv(k) != "" {
 			return true
 		}
 	}
 	return false
 }
  1. Stale doc example (dialers.go:48) — The TCPProxyDialer type comment still shows the old constructor signature without the new logger parameter.
-//	dialer := dialers.NewTCPProxyDialer(30 * time.Second)
+//	dialer := dialers.NewTCPProxyDialer(30 * time.Second, log)

bbarao

This comment was marked as duplicate.

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.

2 participants