diff --git a/CHANGELOG.md b/CHANGELOG.md index 0f836c1a..57561ebe 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -16,6 +16,7 @@ ## Changes since v7.0.1 +- [#1115](https://github.com/oauth2-proxy/oauth2-proxy/pull/1115) Fix upstream proxy appending ? to requests (@JoelSpeed) - [#1117](https://github.com/oauth2-proxy/oauth2-proxy/pull/1117) Deprecate GCP HealthCheck option (@JoelSpeed) - [#1104](https://github.com/oauth2-proxy/oauth2-proxy/pull/1104) Allow custom robots text pages (@JoelSpeed) - [#1045](https://github.com/oauth2-proxy/oauth2-proxy/pull/1045) Ensure redirect URI always has a scheme (@JoelSpeed) diff --git a/pkg/upstream/http.go b/pkg/upstream/http.go index 718bcba6..a936b00b 100644 --- a/pkg/upstream/http.go +++ b/pkg/upstream/http.go @@ -116,11 +116,11 @@ func newReverseProxy(target *url.URL, upstream options.Upstream, errorHandler Pr } } - // Set the request director based on the PassHostHeader option + // Ensure we always pass the original request path + setProxyDirector(proxy) + if upstream.PassHostHeader != nil && !*upstream.PassHostHeader { setProxyUpstreamHostHeader(proxy, target) - } else { - setProxyDirector(proxy) } // Set the error handler so that upstream connection failures render the @@ -137,10 +137,7 @@ func setProxyUpstreamHostHeader(proxy *httputil.ReverseProxy, target *url.URL) { director := proxy.Director proxy.Director = func(req *http.Request) { director(req) - // use RequestURI so that we aren't unescaping encoded slashes in the request path req.Host = target.Host - req.URL.Opaque = req.RequestURI - req.URL.RawQuery = "" } } @@ -153,6 +150,7 @@ func setProxyDirector(proxy *httputil.ReverseProxy) { // use RequestURI so that we aren't unescaping encoded slashes in the request path req.URL.Opaque = req.RequestURI req.URL.RawQuery = "" + req.URL.ForceQuery = false } } diff --git a/pkg/upstream/http_test.go b/pkg/upstream/http_test.go index aecee5c7..7477ac3b 100644 --- a/pkg/upstream/http_test.go +++ b/pkg/upstream/http_test.go @@ -30,16 +30,17 @@ var _ = Describe("HTTP Upstream Suite", func() { falsum := false type httpUpstreamTableInput struct { - id string - serverAddr *string - target string - method string - body []byte - signatureData *options.SignatureData - existingHeaders map[string]string - expectedResponse testHTTPResponse - expectedUpstream string - errorHandler ProxyErrorHandler + id string + serverAddr *string + target string + method string + body []byte + passUpstreamHostHeader bool + signatureData *options.SignatureData + existingHeaders map[string]string + expectedResponse testHTTPResponse + expectedUpstream string + errorHandler ProxyErrorHandler } DescribeTable("HTTP Upstream ServeHTTP", @@ -52,6 +53,9 @@ var _ = Describe("HTTP Upstream Suite", func() { for key, value := range in.existingHeaders { req.Header.Add(key, value) } + if host := req.Header.Get("Host"); host != "" { + req.Host = host + } req = middlewareapi.AddRequestScope(req, &middlewareapi.RequestScope{}) rw := httptest.NewRecorder() @@ -60,7 +64,7 @@ var _ = Describe("HTTP Upstream Suite", func() { upstream := options.Upstream{ ID: in.id, - PassHostHeader: &truth, + PassHostHeader: &in.passUpstreamHostHeader, ProxyWebSockets: &falsum, InsecureSkipTLSVerify: false, FlushInterval: &flush, @@ -140,6 +144,29 @@ var _ = Describe("HTTP Upstream Suite", func() { }, expectedUpstream: "encodedSlashes", }), + Entry("request a path with an empty query string", &httpUpstreamTableInput{ + id: "default", + serverAddr: &serverAddr, + target: "http://example.localhost/foo?", + method: "GET", + body: []byte{}, + errorHandler: nil, + expectedResponse: testHTTPResponse{ + code: 200, + header: map[string][]string{ + contentType: {applicationJSON}, + }, + request: testHTTPRequest{ + Method: "GET", + URL: "http://example.localhost/foo?", + Header: map[string][]string{}, + Body: []byte{}, + Host: "example.localhost", + RequestURI: "http://example.localhost/foo?", + }, + }, + expectedUpstream: "default", + }), Entry("when the request has a body", &httpUpstreamTableInput{ id: "requestWithBody", serverAddr: &serverAddr, @@ -257,6 +284,33 @@ var _ = Describe("HTTP Upstream Suite", func() { }, expectedUpstream: "existingHeaders", }), + Entry("when passing the existing host header", &httpUpstreamTableInput{ + id: "passExistingHostHeader", + serverAddr: &serverAddr, + target: "/existingHostHeader", + method: "GET", + body: []byte{}, + errorHandler: nil, + passUpstreamHostHeader: true, + existingHeaders: map[string]string{ + "Host": "existing-host", + }, + expectedResponse: testHTTPResponse{ + code: 200, + header: map[string][]string{ + contentType: {applicationJSON}, + }, + request: testHTTPRequest{ + Method: "GET", + URL: "/existingHostHeader", + Header: map[string][]string{}, + Body: []byte{}, + Host: "existing-host", + RequestURI: "/existingHostHeader", + }, + }, + expectedUpstream: "passExistingHostHeader", + }), ) It("ServeHTTP, when not passing a host header", func() {