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

@@ -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
 	}
 }

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/") {

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)

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)
+	path := r.URL.Path
-	if err != nil {
+	prefix := GetPathPrefix()
-		Errorf(w, r, "cannot get canonical path: %s", err)
+	if prefix != "" {
-		unsupportedRequestErrors.Inc()
+		// Trim -http.pathPrefix from path
-		return
+		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)
+}