From f192666fec4e3ca8a708f19e4b1d6191b43a2967 Mon Sep 17 00:00:00 2001 From: Aliaksandr Valialkin 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 ae9c15616..caf4b5ed4 100644 --- a/app/vmalert/web.go +++ b/app/vmalert/web.go @@ -158,7 +158,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 7cd01ac72..05301ae6f 100644 --- a/app/vmselect/main.go +++ b/app/vmselect/main.go @@ -290,7 +290,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/") { @@ -344,7 +344,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 b1af34ca0..539033ef7 100644 --- a/docs/CHANGELOG.md +++ b/docs/CHANGELOG.md @@ -21,6 +21,7 @@ The following tip changes can be tested by building VictoriaMetrics components f * BUGFIX: [vmagent](https://docs.victoriametrics.com/vmagent.html): restart all the scrape jobs during [config reload](https://docs.victoriametrics.com/vmagent.html#configuration-update) after `global` section is changed inside `-promscrape.config`. See [this issue](https://github.com/VictoriaMetrics/VictoriaMetrics/issues/2884). * 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: [vmagent](https://docs.victoriametrics.com/vmagent.html): do not split regex in [relabeling rules](https://docs.victoriametrics.com/vmagent.html#relabeling) into multiple lines if it contains groups. This fixes [this issue](https://github.com/VictoriaMetrics/VictoriaMetrics/issues/2928). +* 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.1](https://github.com/VictoriaMetrics/VictoriaMetrics/releases/tag/v1.79.1) diff --git a/lib/httpserver/httpserver.go b/lib/httpserver/httpserver.go index ddd3352e9..43f8bc49f 100644 --- a/lib/httpserver/httpserver.go +++ b/lib/httpserver/httpserver.go @@ -235,13 +235,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") @@ -327,24 +341,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. @@ -643,7 +639,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. @@ -671,3 +677,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) +}