Merge pull request #95128 from ii/remove-unwanted-redirects

Limit Apiserver Proxy Redirects
This commit is contained in:
Kubernetes Prow Robot 2021-10-11 17:23:46 -07:00 committed by GitHub
commit d2f6eb6339
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
2 changed files with 93 additions and 11 deletions

View File

@ -192,6 +192,26 @@ func NewUpgradeAwareHandler(location *url.URL, transport http.RoundTripper, wrap
}
}
func proxyRedirectsforRootPath(path string, w http.ResponseWriter, req *http.Request) bool {
redirect := false
method := req.Method
// From pkg/genericapiserver/endpoints/handlers/proxy.go#ServeHTTP:
// Redirect requests with an empty path to a location that ends with a '/'
// This is essentially a hack for http://issue.k8s.io/4958.
// Note: Keep this code after tryUpgrade to not break that flow.
if len(path) == 0 && (method == http.MethodGet || method == http.MethodHead) {
var queryPart string
if len(req.URL.RawQuery) > 0 {
queryPart = "?" + req.URL.RawQuery
}
w.Header().Set("Location", req.URL.Path+"/"+queryPart)
w.WriteHeader(http.StatusMovedPermanently)
redirect = true
}
return redirect
}
// ServeHTTP handles the proxy request
func (h *UpgradeAwareHandler) ServeHTTP(w http.ResponseWriter, req *http.Request) {
if h.tryUpgrade(w, req) {
@ -211,17 +231,8 @@ func (h *UpgradeAwareHandler) ServeHTTP(w http.ResponseWriter, req *http.Request
loc.Path += "/"
}
// From pkg/genericapiserver/endpoints/handlers/proxy.go#ServeHTTP:
// Redirect requests with an empty path to a location that ends with a '/'
// This is essentially a hack for http://issue.k8s.io/4958.
// Note: Keep this code after tryUpgrade to not break that flow.
if len(loc.Path) == 0 {
var queryPart string
if len(req.URL.RawQuery) > 0 {
queryPart = "?" + req.URL.RawQuery
}
w.Header().Set("Location", req.URL.Path+"/"+queryPart)
w.WriteHeader(http.StatusMovedPermanently)
proxyRedirect := proxyRedirectsforRootPath(loc.Path, w, req)
if proxyRedirect {
return
}

View File

@ -1055,6 +1055,77 @@ func TestErrorPropagation(t *testing.T) {
}
}
func TestProxyRedirectsforRootPath(t *testing.T) {
tests := []struct {
name string
method string
requestPath string
expectedHeader http.Header
expectedStatusCode int
redirect bool
}{
{
name: "root path, simple get",
method: "GET",
requestPath: "",
redirect: true,
expectedStatusCode: 301,
expectedHeader: http.Header{
"Location": []string{"/"},
},
},
{
name: "root path, simple put",
method: "PUT",
requestPath: "",
redirect: false,
expectedStatusCode: 200,
},
{
name: "root path, simple head",
method: "HEAD",
requestPath: "",
redirect: true,
expectedStatusCode: 301,
expectedHeader: http.Header{
"Location": []string{"/"},
},
},
{
name: "root path, simple delete with params",
method: "DELETE",
requestPath: "",
redirect: false,
expectedStatusCode: 200,
},
}
for _, test := range tests {
func() {
w := httptest.NewRecorder()
req, err := http.NewRequest(test.method, test.requestPath, nil)
if err != nil {
t.Fatal(err)
}
redirect := proxyRedirectsforRootPath(test.requestPath, w, req)
if got, want := redirect, test.redirect; got != want {
t.Errorf("Expected redirect state %v; got %v", want, got)
}
res := w.Result()
if got, want := res.StatusCode, test.expectedStatusCode; got != want {
t.Errorf("Expected status code %d; got %d", want, got)
}
if res.StatusCode == 301 && !reflect.DeepEqual(res.Header, test.expectedHeader) {
t.Errorf("Expected location header to be %v, got %v", test.expectedHeader, res.Header)
}
}()
}
}
// exampleCert was generated from crypto/tls/generate_cert.go with the following command:
// go run generate_cert.go --rsa-bits 1024 --host example.com --ca --start-date "Jan 1 00:00:00 1970" --duration=1000000h
var exampleCert = []byte(`-----BEGIN CERTIFICATE-----