fix(server): normalize trailing slashes so /health/ returns 200 (#21)

Author: hibare

cmd/server/middlewares/normalize_path.go

@@ -0,0 +1,67 @@
+// Copyright 2026 Thomson Reuters
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+//      http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+package middlewares
+
+import (
+	"net/http"
+	"net/url"
+)
+
+// NormalizePath is an HTTP middleware that strips trailing slashes from
+// r.URL.Path (and r.URL.RawPath, when set) before the request reaches
+// subsequent middleware or handlers. The root path "/" is preserved.
+//
+// Why this exists: chi's middleware.StripSlashes only rewrites the chi
+// RouteContext's RoutePath; it does not modify r.URL.Path. Middleware that
+// reads r.URL.Path directly (for example middleware.Heartbeat, structured
+// loggers, or OpenTelemetry span-name formatters) therefore continues to see
+// the unnormalized path. Registering NormalizePath at the top of the chain
+// closes that gap so every downstream observer sees a single canonical path.
+//
+// NormalizePath does not mutate the caller's *http.Request. If normalization
+// is required it performs a shallow clone of the request and its URL (mirroring
+// net/http.StripPrefix); otherwise it forwards the request unchanged.
+func NormalizePath(next http.Handler) http.Handler {
+	return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+		path, pathChanged := stripTrailingSlashes(r.URL.Path)
+		raw, rawChanged := stripTrailingSlashes(r.URL.RawPath)
+		if !pathChanged && !rawChanged {
+			next.ServeHTTP(w, r)
+			return
+		}
+
+		r2 := new(http.Request)
+		*r2 = *r
+		r2.URL = new(url.URL)
+		*r2.URL = *r.URL
+		r2.URL.Path = path
+		r2.URL.RawPath = raw
+		next.ServeHTTP(w, r2)
+	})
+}
+
+// stripTrailingSlashes removes every trailing '/' from p, except when p itself
+// is "/". It returns the cleaned path and whether any change was made so the
+// caller can avoid unnecessary allocations on already-canonical paths.
+func stripTrailingSlashes(p string) (string, bool) {
+	end := len(p)
+	for end > 1 && p[end-1] == '/' {
+		end--
+	}
+	if end == len(p) {
+		return p, false
+	}
+	return p[:end], true
+}

cmd/server/middlewares/normalize_path_test.go

@@ -0,0 +1,155 @@
+// Copyright 2026 Thomson Reuters
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+//      http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+package middlewares
+
+import (
+	"net/http"
+	"net/http/httptest"
+	"net/url"
+	"testing"
+
+	"github.com/stretchr/testify/assert"
+)
+
+func TestStripTrailingSlashes(t *testing.T) {
+	t.Parallel()
+
+	tests := []struct {
+		name        string
+		in          string
+		want        string
+		wantChanged bool
+	}{
+		{name: "empty", in: "", want: "", wantChanged: false},
+		{name: "root preserved", in: "/", want: "/", wantChanged: false},
+		{name: "no trailing slash", in: "/foo", want: "/foo", wantChanged: false},
+		{name: "single trailing slash", in: "/foo/", want: "/foo", wantChanged: true},
+		{name: "multiple trailing slashes", in: "/foo///", want: "/foo", wantChanged: true},
+		{name: "nested path", in: "/api/v1/info/", want: "/api/v1/info", wantChanged: true},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			t.Parallel()
+
+			got, changed := stripTrailingSlashes(tt.in)
+			assert.Equal(t, tt.want, got)
+			assert.Equal(t, tt.wantChanged, changed)
+		})
+	}
+}
+
+func TestNormalizePath(t *testing.T) {
+	t.Parallel()
+
+	tests := []struct {
+		name        string
+		path        string
+		rawPath     string
+		wantPath    string
+		wantRawPath string
+	}{
+		{name: "trailing slash stripped", path: "/health/", wantPath: "/health"},
+		{name: "multiple trailing slashes stripped", path: "/health///", wantPath: "/health"},
+		{name: "no trailing slash", path: "/health", wantPath: "/health"},
+		{name: "root preserved", path: "/", wantPath: "/"},
+		{name: "nested trailing slash", path: "/api/v1/info/", wantPath: "/api/v1/info"},
+		{
+			name:        "raw path stripped in lock-step with path",
+			path:        "/api/v1/foo bar/",
+			rawPath:     "/api/v1/foo%20bar/",
+			wantPath:    "/api/v1/foo bar",
+			wantRawPath: "/api/v1/foo%20bar",
+		},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			t.Parallel()
+
+			var (
+				gotPath    string
+				gotRawPath string
+			)
+
+			handler := NormalizePath(http.HandlerFunc(func(_ http.ResponseWriter, r *http.Request) {
+				gotPath = r.URL.Path
+				gotRawPath = r.URL.RawPath
+			}))
+
+			req := httptest.NewRequestWithContext(t.Context(), http.MethodGet, "/", nil)
+			req.URL = &url.URL{Path: tt.path, RawPath: tt.rawPath}
+			w := httptest.NewRecorder()
+
+			handler.ServeHTTP(w, req)
+
+			assert.Equal(t, tt.wantPath, gotPath)
+			assert.Equal(t, tt.wantRawPath, gotRawPath)
+		})
+	}
+}
+
+func TestNormalizePath_PreservesCallerRequest(t *testing.T) {
+	t.Parallel()
+
+	handler := NormalizePath(http.HandlerFunc(func(_ http.ResponseWriter, _ *http.Request) {}))
+
+	originalURL := &url.URL{Path: "/health/", RawPath: "/health/", RawQuery: "x=1"}
+	req := httptest.NewRequestWithContext(t.Context(), http.MethodGet, "/", nil)
+	req.URL = originalURL
+
+	handler.ServeHTTP(httptest.NewRecorder(), req)
+
+	assert.Equal(t, "/health/", originalURL.Path, "caller's URL.Path must not be mutated")
+	assert.Equal(t, "/health/", originalURL.RawPath, "caller's URL.RawPath must not be mutated")
+	assert.Same(t, originalURL, req.URL, "caller's *http.Request must still reference the original *url.URL")
+}
+
+func TestNormalizePath_PreservesQueryAndOtherFields(t *testing.T) {
+	t.Parallel()
+
+	var got *http.Request
+	handler := NormalizePath(http.HandlerFunc(func(_ http.ResponseWriter, r *http.Request) {
+		got = r
+	}))
+
+	req := httptest.NewRequestWithContext(t.Context(), http.MethodPost, "/", nil)
+	req.URL = &url.URL{Path: "/api/v1/exchange/", RawQuery: "code=abc&state=xyz", Fragment: "frag"}
+	req.Header.Set("X-Test", "preserved")
+
+	handler.ServeHTTP(httptest.NewRecorder(), req)
+
+	assert.Equal(t, "/api/v1/exchange", got.URL.Path)
+	assert.Equal(t, "code=abc&state=xyz", got.URL.RawQuery, "query string must be preserved")
+	assert.Equal(t, "frag", got.URL.Fragment, "fragment must be preserved")
+	assert.Equal(t, http.MethodPost, got.Method, "method must be preserved")
+	assert.Equal(t, "preserved", got.Header.Get("X-Test"), "headers must be preserved")
+}
+
+func TestNormalizePath_FastPathSkipsClone(t *testing.T) {
+	t.Parallel()
+
+	var observed *http.Request
+	handler := NormalizePath(http.HandlerFunc(func(_ http.ResponseWriter, r *http.Request) {
+		observed = r
+	}))
+
+	req := httptest.NewRequestWithContext(t.Context(), http.MethodGet, "/", nil)
+	req.URL = &url.URL{Path: "/health"}
+
+	handler.ServeHTTP(httptest.NewRecorder(), req)
+
+	assert.Same(t, req, observed, "when no normalization is required the original request must be forwarded as-is")
+}

cmd/server/server.go

@@ -94,6 +94,8 @@ func (s *Server) Init() error {
 	}
 
 	s.router = chi.NewRouter()
+	s.router.Use(middlewares.NormalizePath)
+	s.router.Use(middleware.CleanPath)
 	s.router.Use(middleware.Heartbeat(healthPath))
 	s.router.Use(otelhttp.NewMiddleware(constants.ProgramIdentifier))
 	s.router.Use(middlewares.ChiRouteLabeler)
@@ -106,8 +108,6 @@ func (s *Server) Init() error {
 		},
 	}))
 	s.router.Use(middleware.Timeout(s.cfg.Server.RequestTimeout))
-	s.router.Use(middleware.StripSlashes)
-	s.router.Use(middleware.CleanPath)
 	s.router.Use(middleware.RequestSize(maxBodyBytes))
 	s.router.Use(middlewares.SecurityHeaders)