From 5449b2dbe4f92eadd9ea0e54b760ef2690046c2d Mon Sep 17 00:00:00 2001 From: lhy1024 Date: Tue, 12 Sep 2023 18:53:14 +0800 Subject: [PATCH] address comments Signed-off-by: lhy1024 --- pkg/utils/apiutil/apiutil.go | 13 +++++++++---- pkg/utils/apiutil/serverapi/middleware.go | 9 ++++++--- 2 files changed, 15 insertions(+), 7 deletions(-) diff --git a/pkg/utils/apiutil/apiutil.go b/pkg/utils/apiutil/apiutil.go index 8702b750df5c..bcab7c8e9e71 100644 --- a/pkg/utils/apiutil/apiutil.go +++ b/pkg/utils/apiutil/apiutil.go @@ -436,9 +436,13 @@ func (p *customReverseProxies) ServeHTTP(w http.ResponseWriter, r *http.Request) } // We need to copy the response headers before we write the header. - // Otherwise, we cannot set the header. + // Otherwise, we cannot set the header after w.WriteHeader() is called. // And we need to write the header before we copy the response body. - // Otherwise, we cannot set the status code. + // Otherwise, we cannot set the status code after w.Write() is called. + // In other words, we must perform the following steps strictly in order: + // 1. Set the response headers. + // 2. Write the response header. + // 3. Write the response body. copyHeader(w.Header(), resp.Header) w.WriteHeader(resp.StatusCode) @@ -460,10 +464,11 @@ func (p *customReverseProxies) ServeHTTP(w http.ResponseWriter, r *http.Request) http.Error(w, ErrRedirectFailed, http.StatusInternalServerError) } +// copyHeader duplicates the HTTP headers from the source `src` to the destination `dst`. +// It skips the "Content-Encoding" and "Content-Length" headers because they should be set by `http.ResponseWriter`. +// These headers may be modified after a redirect when gzip compression is enabled. func copyHeader(dst, src http.Header) { for k, vv := range src { - // skip Content-Encoding and Content-Length header - // because they need to be set by http.ResponseWriter when gzip is enabled if k == "Content-Encoding" || k == "Content-Length" { continue } diff --git a/pkg/utils/apiutil/serverapi/middleware.go b/pkg/utils/apiutil/serverapi/middleware.go index d7850029e2ed..830ff98a00ca 100644 --- a/pkg/utils/apiutil/serverapi/middleware.go +++ b/pkg/utils/apiutil/serverapi/middleware.go @@ -116,6 +116,9 @@ func (h *redirector) matchMicroServiceRedirectRules(r *http.Request) (bool, stri zap.String("path", r.URL.Path)) } // Extract parameters from the URL path + // e.g. matchPath = /pd/api/v1/operator/1 + // targetPath = /scheduling/api/v1/operator + // r.URL.Path = /scheduling/api/v1/operator/1 pathParams := strings.TrimPrefix(r.URL.Path, rule.matchPath) if len(pathParams) > 0 && pathParams[0] == '/' { pathParams = pathParams[1:] // Remove leading '/' @@ -129,10 +132,10 @@ func (h *redirector) matchMicroServiceRedirectRules(r *http.Request) (bool, stri } func (h *redirector) ServeHTTP(w http.ResponseWriter, r *http.Request, next http.HandlerFunc) { - needRedirectToMicroService, targetAddr := h.matchMicroServiceRedirectRules(r) + redirectToMicroService, targetAddr := h.matchMicroServiceRedirectRules(r) allowFollowerHandle := len(r.Header.Get(apiutil.PDAllowFollowerHandleHeader)) > 0 isLeader := h.s.GetMember().IsLeader() - if !h.s.IsClosed() && (allowFollowerHandle || isLeader) && !needRedirectToMicroService { + if !h.s.IsClosed() && (allowFollowerHandle || isLeader) && !redirectToMicroService { next(w, r) return } @@ -157,7 +160,7 @@ func (h *redirector) ServeHTTP(w http.ResponseWriter, r *http.Request, next http } var clientUrls []string - if needRedirectToMicroService { + if redirectToMicroService { if len(targetAddr) == 0 { http.Error(w, apiutil.ErrRedirectFailed, http.StatusInternalServerError) return