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

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

app/vmselect/main.go

@@ -168,7 +168,7 @@ func RequestHandler(w http.ResponseWriter, r *http.Request) bool {
 		_ = r.ParseForm()
 		path = strings.TrimPrefix(path, "/")
 		newURL := path + "/?" + r.Form.Encode()
-		http.Redirect(w, r, newURL, http.StatusMovedPermanently)
+		httpserver.RedirectPermanent(w, newURL)
 		return true
 	}
 	if strings.HasPrefix(path, "/vmui/") {
@@ -217,7 +217,7 @@ func RequestHandler(w http.ResponseWriter, r *http.Request) bool {
 		// vmalert access via incomplete url without `/` in the end. Redirecto to complete url.
 		// Use relative redirect, since, since the hostname and path prefix may be incorrect if VictoriaMetrics
 		// is hidden behind vmauth or similar proxy.
-		http.Redirect(w, r, "vmalert/", http.StatusMovedPermanently)
+		httpserver.RedirectPermanent(w, "vmalert/")
 		return true
 	}
 	if strings.HasPrefix(path, "/vmalert/") {

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)

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)
+}