Skip to content

Commit

Permalink
address comments
Browse files Browse the repository at this point in the history
Signed-off-by: lhy1024 <[email protected]>
  • Loading branch information
lhy1024 committed Sep 12, 2023
1 parent 0f733dc commit 5449b2d
Show file tree
Hide file tree
Showing 2 changed files with 15 additions and 7 deletions.
13 changes: 9 additions & 4 deletions pkg/utils/apiutil/apiutil.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)

Expand All @@ -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
}
Expand Down
9 changes: 6 additions & 3 deletions pkg/utils/apiutil/serverapi/middleware.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 '/'
Expand All @@ -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
}
Expand All @@ -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
Expand Down

0 comments on commit 5449b2d

Please sign in to comment.