fix: throttle before auth, plugin request fields, bounded metrics label, host parsing

README.md

@@ -398,7 +398,7 @@ Optional, can be turned on with `--mgmt.enabled`. Exposes 2 endpoints on `mgmt.l
 - `GET /routes` - list of all discovered routes
 - `GET /metrics` - returns prometheus metrics (`http_requests_total`, `response_status` and `http_response_time_seconds`)
 
-By default, `http_response_time_seconds` uses raw request paths as labels, which can cause high cardinality with dynamic URLs (e.g., `/api/users/123`, `/api/users/456`). Use `--mgmt.low-cardinality` to switch to route patterns (e.g., `^/api/users/(.*)`) instead, significantly reducing metrics cardinality.
+By default, `http_response_time_seconds` uses raw request paths as labels, which can cause high cardinality with dynamic URLs (e.g., `/api/users/123`, `/api/users/456`). Use `--mgmt.low-cardinality` to switch to route patterns (e.g., `^/api/users/(.*)`) instead, significantly reducing metrics cardinality. In this mode the `server` label of `http_requests_total` is the matched route's server (e.g., `*` or `example.com`) rather than the request's Host, and `[unmatched]` for requests without a route, so clients can't add series by sending new Host values.
 
 _see also [examples/metrics](https://github.com/umputun/reproxy/tree/master/examples/metrics)_
 
@@ -410,7 +410,7 @@ Reproxy returns 502 (Bad Gateway) error in case if request doesn't match to any
 
 Reproxy allows to define system level max req/sec value for the overall system activity as well as per user. 0 values (default) treated as unlimited.
 
-The user is identified by the client address, the same one reproxy sends in `X-Real-IP` (see [client address and forwarded headers](#client-address-and-forwarded-headers)), so a client can't escape the limit by changing `X-Forwarded-For` or `X-Real-IP`.
+The user is identified by the client address, the same one reproxy sends in `X-Real-IP` (see [client address and forwarded headers](#client-address-and-forwarded-headers)), so a client can't escape the limit by changing `X-Forwarded-For` or `X-Real-IP`. Limits are checked before global and per-route basic auth, so requests with wrong credentials count against them too.
 
 User activity limited for both matched and unmatched routes. All unmatched routes considered as a "single destination group" and get a common limiter which is `rate*3`. It means if 10 (req/sec) defined with `--throttle.user=10` the end user will be able to perform up to 30 request pers second for either static assets or unmatched routes. For matched routes this limiter maintained per destination (route), i.e. request proxied to s1.example.com/api will allow 10 r/s and the request proxied to s2.example.com will allow another 10 r/s.
 
@@ -492,11 +492,18 @@ The check uses the same client address reproxy sends to the backend in `X-Real-I
 
 ## Plugins support
 
-The core functionality of reproxy can be extended with external plugins. Each plugin is an independent process/container implementing [rpc server](https://golang.org/pkg/net/rpc/). Plugins are registered with reproxy conductor and added to the chain of the middlewares. Each plugin receives request with the original url, headers and all matching route info and responds with the headers and the status code. Any status code >= 400 treated as an error response and terminates flow immediately with the proxy error. There are two types of headers plugins can set:
+The core functionality of reproxy can be extended with external plugins. Each plugin is an independent process/container implementing [rpc server](https://golang.org/pkg/net/rpc/). Plugins are registered with reproxy conductor and added to the chain of the middlewares. Each plugin receives the request as route matching saw it, with headers and all matching route info, and responds with the headers and the status code. Any status code >= 400 treated as an error response and terminates flow immediately with the proxy error. There are two types of headers plugins can set:
 
 - `HeadersIn` - incoming headers. Those will be sent to the proxied url
 - `HeadersOut` - outgoing headers. Will be sent back to the client 
 
+The request fields describe the request the way route matching saw it:
+
+- `URL` - path and query, with the path in canonical escaping, so `/%66ail` arrives as `/fail`
+- `Host` - request host without the port, e.g. `example.com` or `::1`
+- `RemoteAddr` - the TCP peer as `ip:port`, which is a proxy's address when reproxy runs behind one
+- `Header` - the request headers; `X-Real-IP` is replaced with the client address resolved through `--trusted-proxy` before the first plugin runs, while `X-Forwarded-For` stays as received and is trustworthy only for hops from trusted proxies. A plugin earlier in the chain can still change any of them via `HeadersIn`
+
 `X-Real-IP`, `X-Forwarded-For`, `Forwarded` and `X-Forwarded-Host/Proto/Port` set through `HeadersIn` are handled like the ones sent by the client, see [client address and forwarded headers](#client-address-and-forwarded-headers). For example, `X-Forwarded-Proto` set by a plugin reaches the backend only when the request came from a trusted proxy.
 
 By default headers set by a plugin will be mixed with the original headers. In case if plugin need to control all the headers, for example drop some of them, `OverrideHeaders*` field can be set by a plugin indicating to the core reproxy process the need to overwrite all the headers instead of mixing them in.

app/discovery/discovery.go

@@ -200,6 +200,19 @@ func isDefaultServer(server string) bool {
 	return server == "*" || server == ""
 }
 
+// HostName returns the server name of a Host header value: a bracketed IPv6 literal without brackets
+// and port, anything else up to the first colon. url.URL.Hostname keeps a non-numeric port such as
+// "admin.example.com:x" in the name, which would miss its exact or wildcard server.
+func HostName(host string) string {
+	if strings.HasPrefix(host, "[") {
+		if end := strings.IndexByte(host, ']'); end > 0 {
+			return host[1:end]
+		}
+	}
+	name, _, _ := strings.Cut(host, ":")
+	return name
+}
+
 // hasConcreteRoute reports whether any route is served by a concrete (non-default) server
 func hasConcreteRoute(routes []MatchedRoute) bool {
 	for _, r := range routes {

app/discovery/discovery_test.go

@@ -1277,3 +1277,25 @@ func TestParseAuth(t *testing.T) {
 		})
 	}
 }
+
+func TestHostName(t *testing.T) {
+	tbl := []struct{ host, want string }{
+		{host: "", want: ""},
+		{host: "example.com", want: "example.com"},
+		{host: "example.com:8080", want: "example.com"},
+		{host: "example.com:", want: "example.com"},
+		{host: "ADMIN.example.com:x", want: "ADMIN.example.com"},
+		{host: "a.example.com:8080:9090", want: "a.example.com"},
+		{host: "127.0.0.1:80", want: "127.0.0.1"},
+		{host: "[::1]:8080", want: "::1"},
+		{host: "[::1]", want: "::1"},
+		{host: "[::1]:x", want: "::1"},
+		{host: "[::1", want: "["},
+	}
+
+	for _, tt := range tbl {
+		t.Run(tt.host, func(t *testing.T) {
+			assert.Equal(t, tt.want, HostName(tt.host))
+		})
+	}
+}

app/mgmt/metrics.go

@@ -8,7 +8,6 @@ import (
 	"net"
 	"net/http"
 	"strconv"
-	"strings"
 
 	"github.com/prometheus/client_golang/prometheus"
 
@@ -80,10 +79,7 @@ func (m *Metrics) Middleware(next http.Handler) http.Handler {
 			path = m.getRoutePattern(r)
 		}
 
-		server := r.URL.Hostname()
-		if server == "" {
-			server = strings.Split(r.Host, ":")[0]
-		}
+		server := m.serverLabel(r)
 
 		timer := prometheus.NewTimer(m.httpDuration.WithLabelValues(path))
 		rw := NewResponseWriter(w)
@@ -106,6 +102,18 @@ func (m *Metrics) getRoutePattern(r *http.Request) string {
 	return "[unmatched]"
 }
 
+// serverLabel returns the server label: the matched route's server in low cardinality mode, since any
+// valid Host passes matching and would add a series, otherwise the request host without port.
+func (m *Metrics) serverLabel(r *http.Request) string {
+	if m.lowCardinality {
+		if v, ok := r.Context().Value(plugin.CtxMatch).(discovery.MatchedRoute); ok {
+			return v.Mapper.Server
+		}
+		return "[unmatched]"
+	}
+	return discovery.HostName(r.Host)
+}
+
 type responseWriter struct {
 	http.ResponseWriter
 	statusCode int

app/mgmt/server_test.go

@@ -325,3 +325,34 @@ func TestResponseWriter_HijackSupported(t *testing.T) {
 	assert.True(t, hw.hijacked)
 	_ = conn.Close()
 }
+
+func TestMetrics_serverLabel(t *testing.T) {
+	match := discovery.MatchedRoute{Mapper: discovery.URLMapper{Server: "*.example.com"}}
+	tbl := []struct {
+		name, target, host     string
+		lowCardinality, routed bool
+		want                   string
+	}{
+		{name: "low cardinality uses matched server", target: "/x", host: "a1.example.com", lowCardinality: true, routed: true, want: "*.example.com"},
+		{name: "low cardinality without match", target: "/x", host: "a1.example.com", lowCardinality: true, want: "[unmatched]"},
+		{name: "default mode uses request host", target: "/x", host: "a1.example.com:8080", want: "a1.example.com"},
+		{name: "default mode ipv6 host", target: "/x", host: "[::1]:8080", want: "::1"},
+		{name: "default mode non-numeric port", target: "/x", host: "a1.example.com:x", want: "a1.example.com"},
+		{name: "default mode absolute-form target", target: "http://api.example.com/x", host: "api.example.com", want: "api.example.com"},
+		{name: "default mode absolute-form extra colons", target: "foo://a.wild.example:1:2/x", want: "a.wild.example"},
+	}
+
+	for _, tt := range tbl {
+		t.Run(tt.name, func(t *testing.T) {
+			r := httptest.NewRequest("GET", tt.target, http.NoBody)
+			if tt.host != "" {
+				r.Host = tt.host
+			}
+			if tt.routed {
+				r = r.WithContext(context.WithValue(r.Context(), plugin.CtxMatch, match))
+			}
+			m := &Metrics{lowCardinality: tt.lowCardinality}
+			assert.Equal(t, tt.want, m.serverLabel(r))
+		})
+	}
+}

app/plugin/conductor.go

@@ -7,6 +7,7 @@ import (
 	"encoding/json"
 	"fmt"
 	"net/http"
+	"net/url"
 	"strings"
 	"sync"
 	"time"
@@ -140,9 +141,10 @@ func (c *Conductor) Middleware(next http.Handler) http.Handler {
 func (c *Conductor) makeRequest(r *http.Request) lib.Request {
 	ctx := r.Context()
 	res := lib.Request{
-		URL:        r.URL.String(),
+		// the client's escaping is dropped so plugins check the same path route matching and the upstream see
+		URL:        (&url.URL{Path: r.URL.Path, RawQuery: r.URL.RawQuery}).String(),
 		RemoteAddr: r.RemoteAddr,
-		Host:       r.URL.Hostname(),
+		Host:       discovery.HostName(r.Host),
 		Header:     r.Header,
 	}
 

app/plugin/conductor_test.go

@@ -497,3 +497,30 @@ func TestConductor_MiddlewareReleasesLockDuringCall(t *testing.T) {
 		t.Fatal("middleware did not reach next handler after plugin call returned")
 	}
 }
+
+func TestConductor_makeRequest(t *testing.T) {
+	tbl := []struct {
+		name, target, host, url, server string
+	}{
+		{name: "escaped path is canonical", target: "/%66ail?a=%20b", host: "example.com", url: "/fail?a=%20b", server: "example.com"},
+		{name: "host with port", target: "/x", host: "example.com:8080", url: "/x", server: "example.com"},
+		{name: "ipv6 host with port", target: "/x", host: "[::1]:8080", url: "/x", server: "::1"},
+		{name: "non-numeric port", target: "/x", host: "ADMIN.example:x", url: "/x", server: "ADMIN.example"},
+		{name: "absolute-form target", target: "http://api.example.com/v1", host: "api.example.com", url: "/v1", server: "api.example.com"},
+		{name: "absolute-form extra colons", target: "foo://a.wild.example:1:2/v1", url: "/v1", server: "a.wild.example"},
+	}
+
+	for _, tt := range tbl {
+		t.Run(tt.name, func(t *testing.T) {
+			r := httptest.NewRequest("GET", tt.target, http.NoBody)
+			if tt.host != "" {
+				r.Host = tt.host
+			}
+			r.RemoteAddr = "192.0.2.1:1234"
+			req := (&Conductor{}).makeRequest(r)
+			assert.Equal(t, tt.url, req.URL)
+			assert.Equal(t, tt.server, req.Host)
+			assert.Equal(t, "192.0.2.1:1234", req.RemoteAddr)
+		})
+	}
+}

app/proxy/health.go

@@ -87,11 +87,7 @@ func (h *Http) pingHandler(next http.Handler) http.Handler {
 
 // shouldForwardHealthChecks checks if the request matches a route with ForwardHealthChecks enabled
 func (h *Http) shouldForwardHealthChecks(r *http.Request) bool {
-	server := r.URL.Hostname()
-	if server == "" {
-		server = strings.Split(r.Host, ":")[0]
-	}
-	matches := h.Match(server, r.URL.EscapedPath())
+	matches := h.Match(discovery.HostName(r.Host), r.URL.EscapedPath())
 	for _, route := range matches.Routes {
 		if route.Mapper.ForwardHealthChecks {
 			return true

app/proxy/proxy.go

@@ -145,10 +145,10 @@ func (h *Http) Run(ctx context.Context) error {
 		h.matchHandler,                                       // set matched routes to context
 		routeTimeoutHandler,                                  // apply per-route request deadline if set on matched mapper
 		h.OnlyFrom.Handler,                                   // limit source (remote) IPs if defined
+		limiterSystemHandler(h.ThrottleSystem),               // limit total requests/sec, before auth so failed logins count
+		limiterUserHandler(h.ThrottleUser, h.TrustedProxies), // req/seq per user/route match
 		perRouteAuthHandler,                                  // per-route basic auth (if route has auth configured)
 		h.basicAuthHandler(),                                 // global basic auth (skipped if per-route auth is set)
-		limiterSystemHandler(h.ThrottleSystem),               // limit total requests/sec
-		limiterUserHandler(h.ThrottleUser, h.TrustedProxies), // req/seq per user/route match
 		h.mgmtHandler(),                                      // handles /metrics and /routes for prometheus
 		h.pluginHandler(),                                    // prc to external plugins
 		headersHandler(h.ProxyHeaders, h.DropHeader),         // add response headers and delete some request headers
@@ -374,10 +374,8 @@ func (h *Http) matchHandler(next http.Handler) http.Handler {
 			h.Reporter.Report(w, http.StatusBadRequest)
 			return
 		}
-		server := r.URL.Hostname()
-		if server == "" {
-			server = strings.Split(r.Host, ":")[0] // drop port
-		}
+		// r.Host holds an absolute-form target's host too; r.URL.Hostname keeps "x.com:1" of foo://x.com:1:2/
+		server := discovery.HostName(r.Host)
 		// normalize from decoded Path so alternate encodings cannot bypass route auth or IP policies
 		canonicalPath := (&url.URL{Path: r.URL.Path}).EscapedPath()
 		matches := h.Match(server, canonicalPath) // get all matches for the server:path pair
@@ -489,7 +487,14 @@ func (h *Http) pluginHandler() func(next http.Handler) http.Handler {
 		return passThroughHandler
 	}
 	log.Printf("[INFO] plugin support enabled")
-	return h.PluginConductor.Middleware
+	return func(next http.Handler) http.Handler {
+		plugins := h.PluginConductor.Middleware(next)
+		return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+			// until Rewrite sets it on the outgoing request, X-Real-IP holds whatever the client sent
+			h.setXRealIP(r, r)
+			plugins.ServeHTTP(w, r)
+		})
+	}
 }
 
 func (h *Http) mgmtHandler() func(next http.Handler) http.Handler {