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 32a81e3
Show file tree
Hide file tree
Showing 2 changed files with 22 additions and 12 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
21 changes: 13 additions & 8 deletions pkg/utils/apiutil/serverapi/middleware.go
Original file line number Diff line number Diff line change
Expand Up @@ -112,27 +112,32 @@ func (h *redirector) matchMicroServiceRedirectRules(r *http.Request) (bool, stri
if strings.HasPrefix(r.URL.Path, rule.matchPath) {
addr, ok := h.s.GetServicePrimaryAddr(r.Context(), rule.targetServiceName)
if !ok || addr == "" {
log.Warn("failed to get the service primary addr when try match redirect rules",
log.Warn("failed to get the service primary addr when trying to match redirect rules",
zap.String("path", r.URL.Path))
}
// Extract parameters from the URL path
// e.g. r.URL.Path = /pd/api/v1/operators/1 (before redirect)
// matchPath = /pd/api/v1/operators
// targetPath = /scheduling/api/v1/operators
// r.URL.Path = /scheduling/api/v1/operator/1 (after redirect)
pathParams := strings.TrimPrefix(r.URL.Path, rule.matchPath)
if len(pathParams) > 0 && pathParams[0] == '/' {
pathParams = pathParams[1:] // Remove leading '/'
pathParams = strings.Trim(pathParams, "/") // Remove leading and trailing '/'
if len(pathParams) > 0 {
r.URL.Path = rule.targetPath + "/" + pathParams
} else {
r.URL.Path = rule.targetPath
}
r.URL.Path = rule.targetPath + "/" + pathParams
r.URL.Path = strings.TrimRight(r.URL.Path, "/")
return true, addr
}
}
return false, ""
}

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 +162,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 32a81e3

Please sign in to comment.