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.

Sign up for GitHub

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

Jump to bottom

x/net/http/httpproxy: http.ProxyFunc ignoring url.Parse error #46264

Closed
maksim77 opened this issue May 19, 2021 · 1 comment
Closed

x/net/http/httpproxy: http.ProxyFunc ignoring url.Parse error #46264

maksim77 opened this issue May 19, 2021 · 1 comment
Labels
FrozenDueToAge
Milestone
Unreleased

Comments

@maksim77
Copy link

maksim77 commented May 19, 2021 •
edited

What version of Go are you using (go version)?

go version go1.16.3 darwin/arm64

Does this issue reproduce with the latest release?

Yes.

What operating system and processor architecture are you using (go env)?

go env Output
GO111MODULE=""
GOARCH="arm64"
GOBIN=""
GOCACHE="/Users/maksim/Library/Caches/go-build"
GOENV="/Users/maksim/Library/Application Support/go/env"
GOEXE=""
GOFLAGS=""
GOHOSTARCH="arm64"
GOHOSTOS="darwin"
GOINSECURE=""
GOMODCACHE="/Users/maksim/go/pkg/mod"
GONOPROXY=""
GONOSUMDB="gitlab.services.mts.ru/*"
GOOS="darwin"
GOPATH="/Users/maksim/go"
GOPRIVATE=""
GOPROXY="https://proxy.golang.org,direct"
GOROOT="/usr/local/go"
GOSUMDB="sum.golang.org"
GOTMPDIR=""
GOTOOLDIR="/usr/local/go/pkg/tool/darwin_arm64"
GOVCS=""
GOVERSION="go1.16.3"
GCCGO="gccgo"
AR="ar"
CC="clang"
CXX="clang++"
CGO_ENABLED="1"
GOMOD="/Users/maksim/Yandex.Disk/dev/go/proxyTest/go.mod"
CGO_CFLAGS="-g -O2"
CGO_CPPFLAGS=""
CGO_CXXFLAGS="-g -O2"
CGO_FFLAGS="-g -O2"
CGO_LDFLAGS="-g -O2"
PKG_CONFIG="pkg-config"
GOGCCFLAGS="-fPIC -arch arm64 -pthread -fno-caret-diagnostics -Qunused-arguments -fmessage-length=0 -fdebug-prefix-map=/var/folders/1l/1qn0db_57n97z1pv3lczt9ym0000gn/T/go-build1406398690=/tmp/go-build -gno-record-gcc-switches -fno-common"

What did you do?

Go Playground

The http.ProxyFromEnvironment function does not return an error if the environment variables contain incorrect values. Worse, it returns <nil> / <nil>. Because of this, the http.Client{} silently ignores the value of the variables defining the proxy.

The problem is that in the package x/net/http/httpproxy function httpproxy.FromEnvironment().ProxyFunc() simply ignores the errors that the url.Parse() returns.

go/src/vendor/golang.org/x/net/http/httpproxy/proxy.go

Lines 118 to 125 in 658b5e6

func (cfg *Config) ProxyFunc() func(reqURL *url.URL) (*url.URL, error) {
// Preprocess the Config settings for more efficient evaluation.
cfg1 := &config{
Config: *cfg,
}
cfg1.init()
return cfg1.proxyForURL
}

go/src/vendor/golang.org/x/net/http/httpproxy/proxy.go

Lines 208 to 215 in 658b5e6

func (c *config) init() {
if parsed, err := parseProxy(c.HTTPProxy); err == nil {
c.httpProxy = parsed
}
if parsed, err := parseProxy(c.HTTPSProxy); err == nil {
c.httpsProxy = parsed
}

I see two problems here:

  1. http.ProxyFromEnvironment is used when building the default client client := &http.Client{} and it is not clear how to report an error in this case
  2. The value of the environment variable in my example is correct from the point of view of for example curl. The password can contain a percent sign. On the other hand, in URLs, the percentage means something completely different...

What did you expect to see?

Silently ignoring the error

What did you see instead?

An error returned explicitly
@gopherbot gopherbot added this to the Unreleased milestone May 19, 2021
@seankhliao seankhliao changed the title x/net/http/httpproxy: http.ProxyFromEnvironment ignoring url.Parse error x/net/http/httpproxy: http.ProxyFunc ignoring url.Parse error May 20, 2021
@seankhliao
Copy link
Member

Duplicate of #38166

@seankhliao seankhliao marked this as a duplicate of #38166 May 20, 2021
@golang golang locked and limited conversation to collaborators May 20, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees
No one assigned
Labels
FrozenDueToAge
Projects
None yet
Milestone
Unreleased
Development

No branches or pull requests
3 participants
@maksim77 @gopherbot @seankhliao