Merge pull request #248 from Notifiarr/dn2_no_gorilla

Remove gorilla mux, replace with sodlib.

Author: davidnewhall

README.md

@@ -59,7 +59,7 @@ server {
     auth_request_set $auth_idnt $upstream_http_X_UserID;
 
     proxy_set_header host $redirect_host;
-    proxy_set_header x-api-key $remote_api_key;
+    proxy_set_header X-Api-Key $remote_api_key;
     proxy_pass $server$request_uri;
   }
 
@@ -68,7 +68,7 @@ server {
     proxy_pass_request_body off;
     proxy_set_header Content-Length "";
     proxy_set_header X-Original-URI $request_uri;
-    proxy_set_header X-API-Key $incoming_api_key;
+    proxy_set_header X-Api-Key $incoming_api_key;
     proxy_set_header X-Server $http_X_Server;
     proxy_pass $authproxy/auth;
   }

docs/api_docs.go

@@ -17,7 +17,7 @@ const docTemplateapi = `{
     "paths": {
         "/auth": {
             "get": {
-                "description": "Retrieve the environment for an API Key or Server ID. This endpoint is designed for auth proxy requests from Nginx.\nOne of X-Server, X-API-Key or X-Original-URI (with an api key in it) must be provided.",
+                "description": "Retrieve the environment for an API Key or Server ID. This endpoint is designed for auth proxy requests from Nginx.\nOne of X-Server, X-Api-Key or X-Original-URI (with an api key in it) must be provided.",
                 "tags": [
                     "auth"
                 ],
@@ -38,7 +38,7 @@ const docTemplateapi = `{
                     {
                         "type": "string",
                         "description": "User's API Key to route. May also be provided in X-Original-URI header.",
-                        "name": "X-API-Key",
+                        "name": "X-Api-Key",
                         "in": "header"
                     },
                     {
@@ -56,7 +56,7 @@ const docTemplateapi = `{
                                 "type": "string",
                                 "description": "How long this information has been in the cache."
                             },
-                            "X-API-Key": {
+                            "X-Api-Key": {
                                 "type": "string",
                                 "description": "API Key parsed from request."
                             },
@@ -80,7 +80,7 @@ const docTemplateapi = `{
                             "type": "string"
                         },
                         "headers": {
-                            "X-API-Key": {
+                            "X-Api-Key": {
                                 "type": "string",
                                 "description": "API Key parsed from request."
                             }
@@ -108,7 +108,7 @@ const docTemplateapi = `{
                     {
                         "type": "string",
                         "description": "Comma separated list of API keys to delete.",
-                        "name": "X-API-Keys",
+                        "name": "X-Api-Keys",
                         "in": "header",
                         "required": true
                     }

go.mod

@@ -4,7 +4,6 @@ go 1.26.1
 
 require (
 	github.com/go-sql-driver/mysql v1.9.3
-	github.com/gorilla/mux v1.8.1
 	github.com/prometheus/client_golang v1.23.2
 	github.com/swaggo/swag v1.16.6
 	golift.io/cache v1.0.1-0.20260406025202-1176587c97ab

go.sum

@@ -42,8 +42,6 @@ github.com/go-sql-driver/mysql v1.9.3 h1:U/N249h2WzJ3Ukj8SowVFjdtZKfu9vlLZxjPXV1
 github.com/go-sql-driver/mysql v1.9.3/go.mod h1:qn46aNg1333BRMNU69Lq93t8du/dwxI64Gl8i5p1WMU=
 github.com/google/go-cmp v0.7.0 h1:wk8382ETsv4JYUZwIsn6YpYiWiBsYLSJiTsyBybVuN8=
 github.com/google/go-cmp v0.7.0/go.mod h1:pXiqmnSA92OHEEa9HXL2W4E7lf9JzCmGVUdgjX3N/iU=
-github.com/gorilla/mux v1.8.1 h1:TuBL49tXwgrFYWhqrNgrUNEY92u81SPhu7sTdzQEiWY=
-github.com/gorilla/mux v1.8.1/go.mod h1:AKf9I4AEqPTmMytcMc0KkNouC66V3BtZ4qD5fmWSiMQ=
 github.com/klauspost/compress v1.18.0 h1:c/Cqfb0r+Yi+JtIEq73FWXVkRonBlf0CRNYc8Zttxdo=
 github.com/klauspost/compress v1.18.0/go.mod h1:2Pp+KzxcywXVXMr50+X0Q/Lsb43OQHYWRCY2AiWywWQ=
 github.com/kr/pretty v0.3.1 h1:flRD4NNwYAUpkphVc1HcthR4KEIFJ65n8Mw5qdRn3LE=

pkg/exp/metrics.go

@@ -143,3 +143,22 @@ func warmHTTPMetrics(metrics *Metrics) {
 		metrics.HTTPResponse.WithLabelValues(strconv.Itoa(code))
 	}
 }
+
+// CountRequest increments the appropriate HTTP request and response metrics for a web request.
+func (m *Metrics) CountRequest(req *http.Request, statusCode string) {
+	if m == nil {
+		return
+	}
+
+	m.HTTPRequests.WithLabelValues(HTTPEventTotal).Inc()
+
+	if req.Method == http.MethodDelete {
+		m.HTTPRequests.WithLabelValues(HTTPEventDelete).Inc()
+	}
+
+	if len(req.Header["X-Server"]) > 0 {
+		m.HTTPRequests.WithLabelValues(HTTPEventXServer).Inc()
+	}
+
+	m.HTTPResponse.WithLabelValues(statusCode).Inc()
+}

pkg/webserver/accesslog.go

@@ -24,15 +24,15 @@ type captureWriter struct {
 }
 
 func (c *captureWriter) WriteHeader(code int) {
-	if c.status == 0 {
+	if c.status == 0 { // cannot set it twice.
 		c.status = code
 	}
 
 	c.ResponseWriter.WriteHeader(code)
 }
 
 func (c *captureWriter) Write(p []byte) (int, error) {
-	if c.status == 0 {
+	if c.status == 0 { // if you write without setting the status, it's a 200.
 		c.status = http.StatusOK
 	}
 
@@ -42,37 +42,29 @@ func (c *captureWriter) Write(p []byte) (int, error) {
 	return n, err //nolint:wrapcheck // delegate to underlying ResponseWriter
 }
 
-func (c *captureWriter) statusCode() int {
+func (c *captureWriter) statusCode() string {
 	if c.status == 0 {
-		return http.StatusOK
-	}
-
-	return c.status
-}
-
-// Get returns the first value for a response header field. key must already be in
-// canonical form (http.CanonicalHeaderKey). Unlike Header.Get it does not allocate
-// or re-canonicalize key on each call.
-func (c *captureWriter) Get(key string) string {
-	if v := c.Header()[key]; len(v) > 0 {
-		return v[0]
+		return "200"
 	}
 
-	return ""
+	return strconv.Itoa(c.status)
 }
 
-//nolint:gochecknoglobals // one pool per process for hot-path access log strings.Builder reuse
-var alBuilder = sync.Pool{New: func() any { return &strings.Builder{} }}
-
-// accessLogWrap writes one Apache-style line per request to dst (same field order as the former alFmt).
-func accessLogWrap(next http.Handler, dst io.Writer) http.Handler {
-	return http.HandlerFunc(func(w http.ResponseWriter, req *http.Request) {
-		capture := &captureWriter{ResponseWriter: w, start: time.Now()}
+// accessLogWrap writes one Apache-style line per request to dst (same field order as the former alFmt)
+// and records HTTP request/response Prometheus counters when metrics is non-nil.
+func (s *server) accessLogWrap(next http.Handler, dst io.Writer) http.Handler {
+	return http.HandlerFunc(func(resp http.ResponseWriter, req *http.Request) {
+		capture := &captureWriter{ResponseWriter: resp, start: time.Now()}
 		next.ServeHTTP(capture, req)
 		capture.writeAccessLogLine(req, dst)
+		// Update Prometheus metrics for the request.
+		s.metrics.CountRequest(req, capture.statusCode())
 	})
 }
 
+//nolint:gochecknoglobals // one pool per process for hot-path access log strings.Builder reuse
+var alBuilder = sync.Pool{New: func() any { return &strings.Builder{} }}
+
 func (c *captureWriter) writeAccessLogLine(req *http.Request, dst io.Writer) {
 	//nolint:forcetypeassert
 	builder := alBuilder.Get().(*strings.Builder) // Get a string buffer:
@@ -85,6 +77,7 @@ func (c *captureWriter) writeAccessLogLine(req *http.Request, dst io.Writer) {
 }
 
 func (c *captureWriter) writeAccessLogLinePrefix(builder *strings.Builder, req *http.Request) {
+	respHeader := c.Header()
 	// %V
 	builder.WriteString(req.Host)
 	builder.WriteByte(' ')
@@ -93,10 +86,10 @@ func (c *captureWriter) writeAccessLogLinePrefix(builder *strings.Builder, req *
 	builder.WriteByte(' ')
 	// "%{X-Username}o"
 	builder.WriteByte('"')
-	builder.WriteString(c.Get("X-Username"))
+	builder.WriteString(getHeader(respHeader, HeaderXUsername))
 	builder.WriteString("\" ")
 	// %{X-UserID}o
-	builder.WriteString(c.Get("X-Userid"))
+	builder.WriteString(getHeader(respHeader, HeaderXUserid))
 	builder.WriteByte(' ')
 	// %t — [02/Jan/2006:15:04:05 -0700]
 	builder.WriteByte('[')
@@ -115,7 +108,7 @@ func (c *captureWriter) writeAccessLogLinePrefix(builder *strings.Builder, req *
 
 	builder.WriteString(" HTTP/1.1\" ")
 	// %>s
-	builder.WriteString(strconv.Itoa(c.statusCode()))
+	builder.WriteString(c.statusCode())
 	builder.WriteByte(' ')
 	// %b — response body size (0 when none).
 	builder.WriteString(strconv.FormatInt(c.size, 10))
@@ -130,28 +123,33 @@ func (c *captureWriter) writeAccessLogLinePrefix(builder *strings.Builder, req *
 }
 
 func (c *captureWriter) writeAccessLogLineTail(builder *strings.Builder, req *http.Request) {
+	respHeader := c.Header()
 	builder.WriteString(" req:")
 	// %{ms}T — elapsed milliseconds (same as apache-logformat request duration).
 	builder.WriteString(strconv.FormatInt(time.Since(c.start).Milliseconds(), 10))
 	builder.WriteString("ms age:")
-	builder.WriteString(c.Get("Age"))
+	builder.WriteString(getHeader(respHeader, HeaderAge))
 	builder.WriteString(" env:")
-	builder.WriteString(c.Get("X-Environment"))
+	builder.WriteString(getHeader(respHeader, HeaderEnvironment))
 	builder.WriteString(" key:")
 
-	masked, keyLenStr := c.maskedAPIKeyFromResponse()
+	masked, keyLenStr := maskedAPIKeyForLog(req, respHeader)
 	builder.WriteString(masked)
 	builder.WriteByte('(')
 	builder.WriteString(keyLenStr)
 	builder.WriteString(") \"srv:")
-	builder.WriteString(req.Header.Get("X-Server"))
+	builder.WriteString(getHeader(req.Header, HeaderXServer))
 	builder.WriteString("\"\n")
 }
 
-// maskedAPIKeyFromResponse returns maskAPIKey(w X-Api-Key) for the access log, or ("", "")
-// when the handler did not set that response header.
-func (c *captureWriter) maskedAPIKeyFromResponse() (string, string) {
-	key := c.Get("X-Api-Key")
+// maskedAPIKeyForLog returns maskAPIKey(X-Api-Key) from the response, else from the parsed request
+// context, or ("", "") if neither is set.
+func maskedAPIKeyForLog(req *http.Request, resp http.Header) (string, string) {
+	key := getHeader(resp, HeaderXAPIKey)
+	if key == "" {
+		key = apiKeyFromRequest(req)
+	}
+
 	if key == "" {
 		return "", ""
 	}
@@ -164,7 +162,7 @@ func (c *captureWriter) maskedAPIKeyFromResponse() (string, string) {
 // If the path has fewer than keyPosition+1 segments, it returns the full path (still without query).
 // When X-Original-Uri is missing, empty, or only a query string, it returns "".
 func RefererPathForLog(header http.Header) string {
-	pathPart, _, _ := strings.Cut(header.Get("X-Original-Uri"), "?")
+	pathPart, _, _ := strings.Cut(getHeader(header, HeaderXOriginalURI), "?")
 	if pathPart == "" {
 		return ""
 	}
@@ -189,7 +187,7 @@ func RefererPathForLog(header http.Header) string {
 
 // ClientIPForLog returns the client IP for access logs (same rules as the former fixForwardedFor middleware).
 func ClientIPForLog(req *http.Request) string {
-	forwarded := req.Header.Get("X-Forwarded-For")
+	forwarded := getHeader(req.Header, HeaderXForwardedFor)
 	if forwarded == "" {
 		host, _, err := net.SplitHostPort(req.RemoteAddr)
 		if err != nil {

pkg/webserver/accesslog_internal_test.go

@@ -28,8 +28,8 @@ func TestResponseWriter(t *testing.T) {
 	rec := httptest.NewRecorder()
 	capWriter := &captureWriter{ResponseWriter: rec}
 
-	if capWriter.statusCode() != http.StatusOK {
-		t.Fatalf("statusCode before any write = %d, want 200", capWriter.statusCode())
+	if capWriter.statusCode() != "200" {
+		t.Fatalf("statusCode before any write = %s, want 200", capWriter.statusCode())
 	}
 
 	capWriter.WriteHeader(http.StatusTeapot)
@@ -57,8 +57,8 @@ func TestResponseWriter(t *testing.T) {
 		t.Fatalf("size = %d, want 2", capWriter.size)
 	}
 
-	if capWriter.statusCode() != http.StatusTeapot {
-		t.Fatalf("statusCode = %d, want %d", capWriter.statusCode(), http.StatusTeapot)
+	if capWriter.statusCode() != "418" {
+		t.Fatalf("statusCode = %s, want 418", capWriter.statusCode())
 	}
 }
 
@@ -77,8 +77,8 @@ func TestResponseWriter_WriteImpliesOK(t *testing.T) {
 		t.Fatalf("Write without WriteHeader: status = %d, want 200", capWriter.status)
 	}
 
-	if capWriter.statusCode() != http.StatusOK {
-		t.Fatalf("statusCode = %d, want 200", capWriter.statusCode())
+	if capWriter.statusCode() != "200" {
+		t.Fatalf("statusCode = %s, want 200", capWriter.statusCode())
 	}
 }
 
@@ -91,16 +91,16 @@ func TestWriteAccessLogLine(t *testing.T) {
 	req.RequestURI = "/p?q=1"
 	req.RemoteAddr = "192.0.2.1:9999"
 	req.Header.Set("User-Agent", "test-agent/1")
-	req.Header.Set("X-Server", "srv-99")
-	req.Header.Set("X-Original-Uri", "/api/v1/route/method/"+TestAccessLogAPIKey)
+	req.Header.Set(HeaderXServer, "srv-99")
+	req.Header.Set(HeaderXOriginalURI, "/api/v1/route/method/"+TestAccessLogAPIKey)
 
 	rec := httptest.NewRecorder()
 	capWriter := &captureWriter{ResponseWriter: rec}
 	capWriter.start = time.Now().Add(-elapsed)
-	capWriter.Header().Set("X-Username", "alice")
-	capWriter.Header().Set("X-Userid", "1001")
-	capWriter.Header().Set("Age", "60")
-	capWriter.Header().Set("X-Environment", "live")
+	capWriter.Header().Set(HeaderXUsername, "alice")
+	capWriter.Header().Set(HeaderXUserid, "1001")
+	capWriter.Header().Set(HeaderAge, "60")
+	capWriter.Header().Set(HeaderEnvironment, "live")
 	capWriter.WriteHeader(http.StatusOK)
 	_, _ = capWriter.Write([]byte("body"))
 
@@ -168,12 +168,12 @@ func TestWriteAccessLogLine_MaskedKeyFromResponseHeader(t *testing.T) {
 
 	start := time.Date(2025, 1, 1, 0, 0, 0, 0, time.UTC)
 	req := httptest.NewRequestWithContext(context.Background(), http.MethodPost, "http://x/", nil)
-	req.Header.Set("X-Server", "")
+	req.Header.Set(HeaderXServer, "")
 
 	rec := httptest.NewRecorder()
 	capWriter := &captureWriter{ResponseWriter: rec}
 	capWriter.start = start
-	capWriter.Header().Set("X-Api-Key", TestAccessLogAPIKey)
+	capWriter.Header().Set(HeaderXAPIKey, TestAccessLogAPIKey)
 
 	builder := &strings.Builder{}
 
@@ -210,19 +210,20 @@ func TestAccessLogWrap(t *testing.T) {
 
 	var dst bytes.Buffer
 
-	handler := accessLogWrap(http.HandlerFunc(func(resp http.ResponseWriter, _ *http.Request) {
-		resp.Header().Set("X-Username", "wrap-user")
-		resp.Header().Set("X-Userid", "55")
-		resp.Header().Set("Age", "3")
-		resp.Header().Set("X-Environment", "dev")
+	srv := &server{}
+	handler := srv.accessLogWrap(http.HandlerFunc(func(resp http.ResponseWriter, _ *http.Request) {
+		resp.Header().Set(HeaderXUsername, "wrap-user")
+		resp.Header().Set(HeaderXUserid, "55")
+		resp.Header().Set(HeaderAge, "3")
+		resp.Header().Set(HeaderEnvironment, "dev")
 		resp.WriteHeader(http.StatusNoContent)
 	}), &dst)
 
 	req := httptest.NewRequestWithContext(context.Background(), http.MethodGet, "http://proxy.test/auth", nil)
 	req.RequestURI = "/auth"
 	req.RemoteAddr = "198.51.100.2:4444"
-	req.Header.Set("X-Forwarded-For", "203.0.113.5")
-	req.Header.Set("X-Server", "discord-srv")
+	req.Header.Set(HeaderXForwardedFor, "203.0.113.5")
+	req.Header.Set(HeaderXServer, "discord-srv")
 	req.Header.Set("User-Agent", "ua-wrap")
 
 	rec := httptest.NewRecorder()
@@ -255,8 +256,9 @@ func TestAccessLogWrap_MaskedKeyFromResponseHeader(t *testing.T) {
 
 	var dst bytes.Buffer
 
-	handler := accessLogWrap(http.HandlerFunc(func(resp http.ResponseWriter, _ *http.Request) {
-		resp.Header().Set("X-Api-Key", TestAccessLogAPIKey)
+	srv := &server{}
+	handler := srv.accessLogWrap(http.HandlerFunc(func(resp http.ResponseWriter, _ *http.Request) {
+		resp.Header().Set(HeaderXAPIKey, TestAccessLogAPIKey)
 		resp.WriteHeader(http.StatusUnauthorized)
 	}), &dst)
 

pkg/webserver/accesslog_test.go

@@ -51,7 +51,7 @@ func TestRefererPathForLog(t *testing.T) {
 			t.Parallel()
 
 			h := http.Header{}
-			h.Set("X-Original-Uri", testCase.origURI)
+			h.Set(webserver.HeaderXOriginalURI, testCase.origURI)
 
 			if got := webserver.RefererPathForLog(h); got != testCase.want {
 				t.Fatalf("RefererPathForLog = %q, want %q (X-Original-Uri=%q)", got, testCase.want, testCase.origURI)
@@ -74,7 +74,7 @@ func TestClientIPForLog(t *testing.T) {
 		t.Fatalf("ClientIPForLog = %q, want 192.0.2.1", got)
 	}
 
-	req.Header.Set("X-Forwarded-For", " 203.0.113.9 , 198.51.100.1 ")
+	req.Header.Set(webserver.HeaderXForwardedFor, " 203.0.113.9 , 198.51.100.1 ")
 
 	if got := webserver.ClientIPForLog(req); got != "203.0.113.9" {
 		t.Fatalf("ClientIPForLog with XFF = %q, want 203.0.113.9", got)