From 025ef3eca4a83ea4127c50fd7d43784b2314933a Mon Sep 17 00:00:00 2001
From: Joel Speed <joel.speed@hotmail.co.uk>
Date: Sun, 21 Mar 2021 11:10:12 +0000
Subject: [PATCH 1/2] Fix upstream proxy appending `?` to requests
---
CHANGELOG.md | 1 +
pkg/upstream/http.go | 1 +
pkg/upstream/http_test.go | 23 +++++++++++++++++++++++
3 files changed, 25 insertions(+)
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..092823c0 100644
--- a/pkg/upstream/http.go
+++ b/pkg/upstream/http.go
@@ -153,6 +153,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..770d56a6 100644
--- a/pkg/upstream/http_test.go
+++ b/pkg/upstream/http_test.go
@@ -140,6 +140,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,
From 92ae5d9d24b61131adc68736baddf2d9f102f789 Mon Sep 17 00:00:00 2001
From: Joel Speed <joel.speed@hotmail.co.uk>
Date: Sun, 21 Mar 2021 18:14:10 +0000
Subject: [PATCH 2/2] Remove duplication in proxy directors
---
pkg/upstream/http.go | 9 +++----
pkg/upstream/http_test.go | 53 +++++++++++++++++++++++++++++++--------
2 files changed, 45 insertions(+), 17 deletions(-)
diff --git a/pkg/upstream/http.go b/pkg/upstream/http.go
index 092823c0..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 = ""
}
}
diff --git a/pkg/upstream/http_test.go b/pkg/upstream/http_test.go
index 770d56a6..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,
@@ -280,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() {