From a1e49606ed1bbccbc64104cd6d338b94972c1167 Mon Sep 17 00:00:00 2001 From: Aliaksandr Valialkin <valyala@victoriametrics.com> Date: Tue, 2 Aug 2022 12:59:03 +0300 Subject: [PATCH] app/{vmselect,vmalert}: properly generate http redirects if `-http.pathPrefix` command-line flag is set Updates https://github.com/VictoriaMetrics/VictoriaMetrics/issues/2918 --- app/vmalert/web.go | 2 +- app/vmselect/main.go | 4 +-- docs/CHANGELOG.md | 1 + lib/httpserver/httpserver.go | 65 ++++++++++++++++++++++-------------- 4 files changed, 44 insertions(+), 28 deletions(-) diff --git a/app/vmalert/web.go b/app/vmalert/web.go index 8ca37270c..733faa24f 100644 --- a/app/vmalert/web.go +++ b/app/vmalert/web.go @@ -160,7 +160,7 @@ func (rh *requestHandler) handler(w http.ResponseWriter, r *http.Request) bool { if strings.HasPrefix(r.URL.Path, "/api/v1/") { redirectURL = alert.APILink() } - http.Redirect(w, r, "/"+redirectURL, http.StatusPermanentRedirect) + httpserver.RedirectPermanent(w, "/"+redirectURL) return true } } diff --git a/app/vmselect/main.go b/app/vmselect/main.go index 2d27ea3d5..15e82c655 100644 --- a/app/vmselect/main.go +++ b/app/vmselect/main.go @@ -292,7 +292,7 @@ func selectHandler(qt *querytracer.Tracer, startTime time.Time, w http.ResponseW _ = r.ParseForm() suffix := strings.Replace(p.Suffix, "prometheus/", "../prometheus/", 1) newURL := suffix + "/?" + r.Form.Encode() - http.Redirect(w, r, newURL, http.StatusMovedPermanently) + httpserver.RedirectPermanent(w, newURL) return true } if strings.HasPrefix(p.Suffix, "vmui/") || strings.HasPrefix(p.Suffix, "prometheus/vmui/") { @@ -346,7 +346,7 @@ func selectHandler(qt *querytracer.Tracer, startTime time.Time, w http.ResponseW if p.Suffix == "prometheus/vmalert" { path := "../" + p.Suffix + "/" - http.Redirect(w, r, path, http.StatusMovedPermanently) + httpserver.RedirectPermanent(w, path) return true } if strings.HasPrefix(p.Suffix, "prometheus/vmalert/") { diff --git a/docs/CHANGELOG.md b/docs/CHANGELOG.md index 65367251a..0ddd72cc7 100644 --- a/docs/CHANGELOG.md +++ b/docs/CHANGELOG.md @@ -27,6 +27,7 @@ The following tip changes can be tested by building VictoriaMetrics components f * BUGFIX: [vmagent](https://docs.victoriametrics.com/vmagent.html): properly assume role with AWS ECS credentials. See [this issue](https://github.com/VictoriaMetrics/VictoriaMetrics/issues/2875). Thanks to @transacid for [the fix](https://github.com/VictoriaMetrics/VictoriaMetrics/pull/2876). * BUGFIX: [MetricsQL](https://docs.victoriametrics.com/MetricsQL.html): return series from `q1` if `q2` doesn't return matching time series in the query `q1 ifnot q2`. Previously series from `q1` weren't returned in this case. * BUGFIX: [vmui](https://docs.victoriametrics.com/#vmui): properly show date picker at `Table` tab. See [this issue](https://github.com/VictoriaMetrics/VictoriaMetrics/issues/2874). +* BUGFIX: properly generate http redirects if `-http.pathPrefix` command-line flag is set. See [this issue](https://github.com/VictoriaMetrics/VictoriaMetrics/issues/2918). ## [v1.79.0](https://github.com/VictoriaMetrics/VictoriaMetrics/releases/tag/v1.79.0) diff --git a/lib/httpserver/httpserver.go b/lib/httpserver/httpserver.go index eaa4bc645..f3e461766 100644 --- a/lib/httpserver/httpserver.go +++ b/lib/httpserver/httpserver.go @@ -236,13 +236,27 @@ func handlerWrapper(s *server, w http.ResponseWriter, r *http.Request, rh Reques connTimeoutClosedConns.Inc() w.Header().Set("Connection", "close") } - path, err := getCanonicalPath(r.URL.Path) - if err != nil { - Errorf(w, r, "cannot get canonical path: %s", err) - unsupportedRequestErrors.Inc() - return + path := r.URL.Path + prefix := GetPathPrefix() + if prefix != "" { + // Trim -http.pathPrefix from path + prefixNoTrailingSlash := strings.TrimSuffix(prefix, "/") + if path == prefixNoTrailingSlash { + // Redirect to url with / at the end. + // This is needed for proper handling of relative urls in web browsers. + // Intentionally ignore query args, since it is expected that the requested url + // is composed by a human, so it doesn't contain query args. + RedirectPermanent(w, prefix) + return + } + if !strings.HasPrefix(path, prefix) { + Errorf(w, r, "missing -http.pathPrefix=%q in the requested path %q", *pathPrefix, path) + unsupportedRequestErrors.Inc() + return + } + path = path[len(prefix)-1:] + r.URL.Path = path } - r.URL.Path = path switch r.URL.Path { case "/health": w.Header().Set("Content-Type", "text/plain; charset=utf-8") @@ -328,24 +342,6 @@ func handlerWrapper(s *server, w http.ResponseWriter, r *http.Request, rh Reques } } -func getCanonicalPath(path string) (string, error) { - if len(*pathPrefix) == 0 || path == "/" { - return path, nil - } - if *pathPrefix == path { - return "/", nil - } - prefix := *pathPrefix - if !strings.HasSuffix(prefix, "/") { - prefix = prefix + "/" - } - if !strings.HasPrefix(path, prefix) { - return "", fmt.Errorf("missing `-pathPrefix=%q` in the requested path: %q", *pathPrefix, path) - } - path = path[len(prefix)-1:] - return path, nil -} - func checkBasicAuth(w http.ResponseWriter, r *http.Request) bool { if len(*httpAuthUsername) == 0 { // HTTP Basic Auth is disabled. @@ -644,7 +640,17 @@ func IsTLS() bool { // GetPathPrefix - returns http server path prefix. func GetPathPrefix() string { - return *pathPrefix + prefix := *pathPrefix + if prefix == "" { + return "" + } + if !strings.HasPrefix(prefix, "/") { + prefix = "/" + prefix + } + if !strings.HasSuffix(prefix, "/") { + prefix += "/" + } + return prefix } // WriteAPIHelp writes pathList to w in HTML format. @@ -672,3 +678,12 @@ func GetRequestURI(r *http.Request) string { } return requestURI + delimiter + queryArgs } + +// RedirectPermanent redirects to the given url using 301 status code. +func RedirectPermanent(w http.ResponseWriter, url string) { + // Do not use http.Redirect, since it breaks relative redirects + // if the http.Request.URL contains unexpected url. + // See https://github.com/VictoriaMetrics/VictoriaMetrics/issues/2918 + w.Header().Set("Location", url) + w.WriteHeader(http.StatusMovedPermanently) +}