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

net/http/httputil: SingleHostReverseProxy escaped paths are decoded #35908

Closed
dkumor opened this issue Nov 29, 2019 · 2 comments
Closed

net/http/httputil: SingleHostReverseProxy escaped paths are decoded #35908

dkumor opened this issue Nov 29, 2019 · 2 comments
Labels
FrozenDueToAge NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one.

Comments

@dkumor
Copy link

dkumor commented Nov 29, 2019

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

$ go version
go version go1.13.4 linux/amd64

Does this issue reproduce with the latest release?

yes

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

go env Output
$ go env
GO111MODULE=""
GOARCH="amd64"
GOBIN=""
GOCACHE="/home/daniel/.cache/go-build"
GOENV="/home/daniel/.config/go/env"
GOEXE=""
GOFLAGS=""
GOHOSTARCH="amd64"
GOHOSTOS="linux"
GONOPROXY=""
GONOSUMDB=""
GOOS="linux"
GOPATH="/home/daniel/go"
GOPRIVATE=""
GOPROXY="https://proxy.golang.org,direct"
GOROOT="/usr/lib/go"
GOSUMDB="sum.golang.org"
GOTMPDIR=""
GOTOOLDIR="/usr/lib/go/pkg/tool/linux_amd64"
GCCGO="gccgo"
AR="ar"
CC="gcc"
CXX="g++"
CGO_ENABLED="1"
GOMOD=""
CGO_CFLAGS="-g -O2"
CGO_CPPFLAGS=""
CGO_CXXFLAGS="-g -O2"
CGO_FFLAGS="-g -O2"
CGO_LDFLAGS="-g -O2"
PKG_CONFIG="pkg-config"
GOGCCFLAGS="-fPIC -m64 -pthread -fmessage-length=0 -fdebug-prefix-map=/tmp/go-build286555625=/tmp/go-build -gno-record-gcc-switches"

What did you do?

  1. initialize a SingleHostReverseProxy with a non-root path (for example /a)
  2. proxy a request with an escaped element: /b%2fc

What did you expect to see?

The forwarded request should be /a/b%2fc

What did you see instead?

The forwarded request is /a/b/c

Reason

This is because when joining URLs, the SingleHostReverseProxy joins just the url.Path elements (https://github.com/golang/go/blob/master/src/net/http/httputil/reverseproxy.go#L112). However, the URL also includes a RawPath which holds an encoding hint specifically to specify how to encode the path (874a605).

To fix this, RawPath also needs to be joined if it is set for either request.

@dkumor dkumor changed the title net/http/httputil: SingleHostReverseProxy url-encoded paths are decoded net/http/httputil: SingleHostReverseProxy escaped paths are decoded Nov 29, 2019
@dmitshur
Copy link
Contributor

dmitshur commented Dec 2, 2019

/cc @bradfitz per owners.

@dmitshur dmitshur added the NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. label Dec 2, 2019
@gopherbot
Copy link

Change https://golang.org/cl/213257 mentions this issue: net/http/httputil: handle escaped paths in SingleHostReverseProxy

xujianhai666 pushed a commit to xujianhai666/go-1 that referenced this issue May 21, 2020
When forwarding a request, a SingleHostReverseProxy appends the
request's path to the target URL's path. However, if certain path
elements are encoded, (such as %2F for slash in either the request or
target path), simply joining the URL.Path elements is not sufficient,
since the field holds the decoded path.

Since 87a605, the RawPath field was added which holds a decoding
hint for the URL. When joining URL paths, this decoding hint needs
to be taken into consideration.

As an example, if the target URL.Path is /a/b, and URL.RawPath
is /a%2Fb, joining the path with /c should result in /a/b/c
in URL.Path, and /a%2Fb/c in RawPath.

The added joinURLPath function combines the two URL's Paths,
while taking into account escaping, and replaces the previously used
singleJoiningSlash in NewSingleHostReverseProxy.

Fixes golang#35908

Change-Id: I45886aee548431fe4031883ab1629a41e35f1727
GitHub-Last-Rev: 7be6b8d
GitHub-Pull-Request: golang#36378
Reviewed-on: https://go-review.googlesource.com/c/go/+/213257
Run-TryBot: Ian Lance Taylor <iant@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
@golang golang locked and limited conversation to collaborators May 2, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
FrozenDueToAge NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants