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

Jump to bottom

net/http/httputil: ReverseProxy doesn't handle empty path #25710

Open
shellfly opened this issue Jun 3, 2018 · 6 comments
Open

net/http/httputil: ReverseProxy doesn't handle empty path #25710

shellfly opened this issue Jun 3, 2018 · 6 comments
Labels
NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one.
Milestone
Backlog

Comments

@shellfly
Copy link

shellfly commented Jun 3, 2018

Please answer these questions before submitting your issue. Thanks!

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

go version go1.10.1 darwin/amd64

Does this issue reproduce with the latest release?

yes

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

GOARCH="amd64"
GOBIN=""
GOCACHE="/Users/hao/Library/Caches/go-build"
GOEXE=""
GOHOSTARCH="amd64"
GOHOSTOS="darwin"
GOOS="darwin"
GOPATH="/Users/hao/go"
GORACE=""
GOROOT="/usr/local/Cellar/go/1.10.1/libexec"
GOTMPDIR=""
GOTOOLDIR="/usr/local/Cellar/go/1.10.1/libexec/pkg/tool/darwin_amd64"
GCCGO="gccgo"
CC="clang"
CXX="clang++"
CGO_ENABLED="1"
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 -fno-caret-diagnostics -Qunused-arguments -fmessage-length=0 -fdebug-prefix-map=/var/folders/wc/gl6jp38s3lqdprky1txx23mr0000gn/T/go-build181870961=/tmp/go-build -gno-record-gcc-switches -fno-common"

What did you do?

package main

import (
	"net/http"
	"net/http/httputil"
	"net/url"
)

func main() {
	url, _ := url.Parse("http://localhost:9000/b")
	http.Handle("/api/a", http.StripPrefix("/api/a", httputil.NewSingleHostReverseProxy(url)))
	http.ListenAndServe(":9001", nil)
}

What did you expect to see?

proxy to http://localhost:8000/b

What did you see instead?

proxy to http://localhost:8000/b/, adding an extra slash.

this line add a slash even if url path is empty. is this expected behavior or a bug?
@meirf
Copy link
Contributor

meirf commented Jun 4, 2018 •
edited

this line add a slash even if url path is empty.

Actually, url path is not "". url path is "/". So it's joining: "/b" + "/" == "/b/". But I agree that this is an unexpected result. Though if we fixed this, it might break existing code.

Here's a more complete, minimal reproduction:

origin:

func main() {
	http.HandleFunc("/b", func(rw http.ResponseWriter, r *http.Request) {
		rw.Write([]byte("handle /b"))
	})
	http.HandleFunc("/b/", func(rw http.ResponseWriter, r *http.Request) {
		rw.Write([]byte("handle /b/"))
	})
	http.ListenAndServe(":9000", nil)
}

proxy:

func main() {
	url, _ := url.Parse("http://localhost:9000/b")
	http.Handle("/", httputil.NewSingleHostReverseProxy(url))
	http.ListenAndServe(":9001", nil)
}

request:
curl http://localhost:9001

@shellfly
Copy link
Author

shellfly commented Jun 4, 2018

@meirf if you use StripPrefix it will generate an empty url Path ""

@meirf
Copy link
Contributor

meirf commented Jun 4, 2018 •
edited

@shellfly Sorry I conflated your example with my repro. But I think my repro is similar.

Your case: "/b" + "". Expect "/b", actual "/b/". See code.
My case "/b" + "/". Expect "/b", actual "/b/". See code.

@agnivade agnivade added the NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. label Jun 4, 2018
@agnivade agnivade added this to the Go1.12 milestone Jun 4, 2018
@agnivade
Copy link
Contributor

agnivade commented Jun 4, 2018

/cc @bradfitz

@joekyo
Copy link
Contributor

joekyo commented Jun 4, 2018

The empty url Path "" generated by StripPrefix is OK. If "/" is generated instead, the request to /b will become /b/ when it reach the backend server.
To make your code works as you expected, I guess the function singleJoiningSlash(a, b string) should check b first, and return a directly if b is an empty string.

@shellfly
Copy link
Author

shellfly commented Jun 5, 2018

@joekyo yes, this is what I thought, or we can do this check before singleJoiningSlash, since the function itself doing the correct thing of joining two paths.

@andybons andybons modified the milestones: Go1.12, Go1.13 Feb 12, 2019
@andybons andybons modified the milestones: Go1.13, Go1.14 Jul 8, 2019
@rsc rsc modified the milestones: Go1.14, Backlog Oct 9, 2019
@toothrot toothrot mentioned this issue Feb 18, 2021
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one.
Projects
None yet
Milestone
Backlog
Development

No branches or pull requests
6 participants
@rsc @andybons @shellfly @agnivade @meirf @joekyo