Skip to content

Commit

Permalink
httpproxy: allow any scheme
Browse files Browse the repository at this point in the history
currently only http/https/socks5 scheme are allowed. However, any scheme
could be possible if user provides their own implementation.
Specifically, the widely used "socks5h://localhost" is parsed as
Scheme="http" Host="socks5h:", which does not make sense because host
name cannot contain ":".

This patch allows any scheme to appear in the proxy config. And only
fallback to http scheme if parsed scheme or host is empty.

url.Parse() result of fallback cases:

localhost      => Scheme="localhost"
localhost:1234 => Scheme="localhost" Opaque="1234"
example.com    => Path="example.com"

Updates golang/go#24135

Change-Id: Ia2c041e37e2ac61be16220fd41d6cb6fabeeca3d
Reviewed-on: https://go-review.googlesource.com/c/net/+/525257
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Run-TryBot: Damien Neil <dneil@google.com>
Reviewed-by: Michael Knyszek <mknyszek@google.com>
Reviewed-by: Damien Neil <dneil@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Auto-Submit: Damien Neil <dneil@google.com>
  • Loading branch information
huww98 authored and gopherbot committed Mar 8, 2024
1 parent ab271c3 commit 8c07e20
Show file tree
Hide file tree
Showing 2 changed files with 13 additions and 4 deletions.
5 changes: 1 addition & 4 deletions http/httpproxy/proxy.go
Original file line number Diff line number Diff line change
Expand Up @@ -149,10 +149,7 @@ func parseProxy(proxy string) (*url.URL, error) {
}

proxyURL, err := url.Parse(proxy)
if err != nil ||
(proxyURL.Scheme != "http" &&
proxyURL.Scheme != "https" &&
proxyURL.Scheme != "socks5") {
if err != nil || proxyURL.Scheme == "" || proxyURL.Host == "" {
// proxy was bogus. Try prepending "http://" to it and
// see if that parses correctly. If not, we fall
// through and complain about the original one.
Expand Down
12 changes: 12 additions & 0 deletions http/httpproxy/proxy_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -68,6 +68,12 @@ var proxyForURLTests = []proxyForURLTest{{
HTTPProxy: "cache.corp.example.com",
},
want: "http://cache.corp.example.com",
}, {
// single label domain is recognized as scheme by url.Parse
cfg: httpproxy.Config{
HTTPProxy: "localhost",
},
want: "http://localhost",
}, {
cfg: httpproxy.Config{
HTTPProxy: "https://cache.corp.example.com",
Expand All @@ -88,6 +94,12 @@ var proxyForURLTests = []proxyForURLTest{{
HTTPProxy: "socks5://127.0.0.1",
},
want: "socks5://127.0.0.1",
}, {
// Preserve unknown schemes.
cfg: httpproxy.Config{
HTTPProxy: "foo://host",
},
want: "foo://host",
}, {
// Don't use secure for http
cfg: httpproxy.Config{
Expand Down

0 comments on commit 8c07e20

Please sign in to comment.