From 9648f1cb7ace807662686a05e926702fae3ca3d5 Mon Sep 17 00:00:00 2001 From: Jordan Liggitt Date: Wed, 6 Sep 2017 21:51:46 -0400 Subject: [PATCH] Fix proxied request-uri to be valid HTTP requests --- pkg/registry/core/node/rest/proxy.go | 2 +- pkg/registry/core/pod/rest/subresources.go | 2 +- pkg/registry/core/service/proxy.go | 2 +- .../pkg/util/proxy/upgradeaware.go | 14 +++++-- .../pkg/util/proxy/upgradeaware_test.go | 39 +++++++------------ 5 files changed, 26 insertions(+), 33 deletions(-) diff --git a/pkg/registry/core/node/rest/proxy.go b/pkg/registry/core/node/rest/proxy.go index 5da6392c60a91..d8d4738b03f1c 100644 --- a/pkg/registry/core/node/rest/proxy.go +++ b/pkg/registry/core/node/rest/proxy.go @@ -70,7 +70,7 @@ func (r *ProxyREST) Connect(ctx genericapirequest.Context, id string, opts runti if err != nil { return nil, err } - location.Path = path.Join(location.Path, proxyOpts.Path) + location.Path = path.Join("/", location.Path, proxyOpts.Path) // Return a proxy handler that uses the desired transport, wrapped with additional proxy handling (to get URL rewriting, X-Forwarded-* headers, etc) return newThrottledUpgradeAwareProxyHandler(location, transport, true, false, responder), nil } diff --git a/pkg/registry/core/pod/rest/subresources.go b/pkg/registry/core/pod/rest/subresources.go index 76e789a5a6f58..a71f8bbd571b3 100644 --- a/pkg/registry/core/pod/rest/subresources.go +++ b/pkg/registry/core/pod/rest/subresources.go @@ -71,7 +71,7 @@ func (r *ProxyREST) Connect(ctx genericapirequest.Context, id string, opts runti if err != nil { return nil, err } - location.Path = path.Join(location.Path, proxyOpts.Path) + location.Path = path.Join("/", location.Path, proxyOpts.Path) // Return a proxy handler that uses the desired transport, wrapped with additional proxy handling (to get URL rewriting, X-Forwarded-* headers, etc) return newThrottledUpgradeAwareProxyHandler(location, transport, true, false, false, responder), nil } diff --git a/pkg/registry/core/service/proxy.go b/pkg/registry/core/service/proxy.go index 4b24847478ebd..a826892e6a961 100644 --- a/pkg/registry/core/service/proxy.go +++ b/pkg/registry/core/service/proxy.go @@ -66,7 +66,7 @@ func (r *ProxyREST) Connect(ctx genericapirequest.Context, id string, opts runti if err != nil { return nil, err } - location.Path = path.Join(location.Path, proxyOpts.Path) + location.Path = path.Join("/", location.Path, proxyOpts.Path) // Return a proxy handler that uses the desired transport, wrapped with additional proxy handling (to get URL rewriting, X-Forwarded-* headers, etc) return newThrottledUpgradeAwareProxyHandler(location, transport, true, false, responder), nil } diff --git a/staging/src/k8s.io/apimachinery/pkg/util/proxy/upgradeaware.go b/staging/src/k8s.io/apimachinery/pkg/util/proxy/upgradeaware.go index 70896907e6623..2bc19655eb8ce 100644 --- a/staging/src/k8s.io/apimachinery/pkg/util/proxy/upgradeaware.go +++ b/staging/src/k8s.io/apimachinery/pkg/util/proxy/upgradeaware.go @@ -159,11 +159,20 @@ func NewUpgradeRequestRoundTripper(connection, request http.RoundTripper) Upgrad } } +// normalizeLocation returns the result of parsing the full URL, with scheme set to http if missing +func normalizeLocation(location *url.URL) *url.URL { + normalized, _ := url.Parse(location.String()) + if len(normalized.Scheme) == 0 { + normalized.Scheme = "http" + } + return normalized +} + // NewUpgradeAwareHandler creates a new proxy handler with a default flush interval. Responder is required for returning // errors to the caller. func NewUpgradeAwareHandler(location *url.URL, transport http.RoundTripper, wrapTransport, upgradeRequired bool, responder ErrorResponder) *UpgradeAwareHandler { return &UpgradeAwareHandler{ - Location: location, + Location: normalizeLocation(location), Transport: transport, WrapTransport: wrapTransport, UpgradeRequired: upgradeRequired, @@ -174,9 +183,6 @@ func NewUpgradeAwareHandler(location *url.URL, transport http.RoundTripper, wrap // ServeHTTP handles the proxy request func (h *UpgradeAwareHandler) ServeHTTP(w http.ResponseWriter, req *http.Request) { - if len(h.Location.Scheme) == 0 { - h.Location.Scheme = "http" - } if h.tryUpgrade(w, req) { return } diff --git a/staging/src/k8s.io/apimachinery/pkg/util/proxy/upgradeaware_test.go b/staging/src/k8s.io/apimachinery/pkg/util/proxy/upgradeaware_test.go index 78ce9870c77d7..a33b10e581e45 100644 --- a/staging/src/k8s.io/apimachinery/pkg/util/proxy/upgradeaware_test.go +++ b/staging/src/k8s.io/apimachinery/pkg/util/proxy/upgradeaware_test.go @@ -241,11 +241,7 @@ func TestServeHTTP(t *testing.T) { responder := &fakeResponder{t: t} backendURL, _ := url.Parse(backendServer.URL) backendURL.Path = test.requestPath - proxyHandler := &UpgradeAwareHandler{ - Location: backendURL, - Responder: responder, - UpgradeRequired: test.upgradeRequired, - } + proxyHandler := NewUpgradeAwareHandler(backendURL, nil, false, test.upgradeRequired, responder) proxyServer := httptest.NewServer(proxyHandler) defer proxyServer.Close() proxyURL, _ := url.Parse(proxyServer.URL) @@ -457,13 +453,9 @@ func TestProxyUpgrade(t *testing.T) { serverURL, _ := url.Parse(backendServer.URL) serverURL.Path = backendPath - proxyHandler := &UpgradeAwareHandler{ - Location: serverURL, - Transport: tc.ProxyTransport, - UpgradeTransport: tc.UpgradeTransport, - InterceptRedirects: redirect, - Responder: &noErrorsAllowed{t: t}, - } + proxyHandler := NewUpgradeAwareHandler(serverURL, tc.ProxyTransport, false, false, &noErrorsAllowed{t: t}) + proxyHandler.UpgradeTransport = tc.UpgradeTransport + proxyHandler.InterceptRedirects = redirect proxy := httptest.NewServer(proxyHandler) defer proxy.Close() @@ -509,14 +501,15 @@ func TestProxyUpgradeErrorResponse(t *testing.T) { return &fakeConn{err: expectedErr}, nil } responder = &fakeResponder{t: t, w: w} - proxyHandler := &UpgradeAwareHandler{ - Location: &url.URL{ + proxyHandler := NewUpgradeAwareHandler( + &url.URL{ Host: "fake-backend", }, - UpgradeRequired: true, - Responder: responder, - Transport: transport, - } + transport, + false, + true, + responder, + ) proxyHandler.ServeHTTP(w, r) })) defer proxy.Close() @@ -575,9 +568,7 @@ func TestDefaultProxyTransport(t *testing.T) { for _, test := range tests { locURL, _ := url.Parse(test.location) URL, _ := url.Parse(test.url) - h := UpgradeAwareHandler{ - Location: locURL, - } + h := NewUpgradeAwareHandler(locURL, nil, false, false, nil) result := h.defaultProxyTransport(URL, nil) transport := result.(*corsRemovingTransport).RoundTripper.(*Transport) if transport.Scheme != test.expectedScheme { @@ -751,11 +742,7 @@ func TestProxyRequestContentLengthAndTransferEncoding(t *testing.T) { responder := &fakeResponder{t: t} backendURL, _ := url.Parse(downstreamServer.URL) - proxyHandler := &UpgradeAwareHandler{ - Location: backendURL, - Responder: responder, - UpgradeRequired: false, - } + proxyHandler := NewUpgradeAwareHandler(backendURL, nil, false, false, responder) proxyServer := httptest.NewServer(proxyHandler) defer proxyServer.Close()