From 7913dd750c0ab4c4e77a51fe6987ced805c42b99 Mon Sep 17 00:00:00 2001 From: Sreejit Pradhan Date: Thu, 3 Sep 2026 21:04:54 +0530 Subject: [PATCH 1/3] fix: normalize trailing slash in static file endpoints --- pkg/gofr/gofr.go | 2 +- pkg/gofr/gofr_test.go | 35 +++++++++++++++++++++++++++++++++++ 2 files changed, 36 insertions(+), 1 deletion(-) diff --git a/pkg/gofr/gofr.go b/pkg/gofr/gofr.go index d2d4a57b29..9c1750443d 100644 --- a/pkg/gofr/gofr.go +++ b/pkg/gofr/gofr.go @@ -407,7 +407,7 @@ func (a *App) AddStaticFiles(endpoint, filePath string) { // Normalize the endpoint up front so every error log below reports the same // '/endpoint' form (previously the getwd and os.Stat logs disagreed). - endpoint = "/" + strings.TrimPrefix(endpoint, "/") + endpoint = "/" + strings.Trim(endpoint, "/") if !strings.HasPrefix(filePath, "./") && !filepath.IsAbs(filePath) { filePath = "./" + filePath diff --git a/pkg/gofr/gofr_test.go b/pkg/gofr/gofr_test.go index 23d0a8de37..281a971474 100644 --- a/pkg/gofr/gofr_test.go +++ b/pkg/gofr/gofr_test.go @@ -12,6 +12,7 @@ import ( "os" "path/filepath" "strconv" + "strings" "sync" "testing" "time" @@ -1329,6 +1330,40 @@ func TestStaticHandlerGetwdError(t *testing.T) { assert.Contains(t, logs, "error in registering '/gofrTest' static endpoint") } +// TestAddStaticFilesEndpointForms covers the endpoint forms a caller can pass to +// App.AddStaticFiles. The normalization used to strip only a leading slash, so a +// trailing slash survived into the stored endpoint and Router.AddStaticFiles registered +// Path("/static/") + PathPrefix("/static//") — neither matches once ServeHTTP normalizes +// the request with path.Clean, so every request under the endpoint 404'd. +func TestAddStaticFilesEndpointForms(t *testing.T) { + testutil.NewServerConfigs(t) + + dir := t.TempDir() + require.NoError(t, os.WriteFile(filepath.Join(dir, "index.html"), []byte("Index"), 0600)) + + for _, endpoint := range []string{"static", "/static", "static/", "/static/"} { + t.Run(endpoint, func(t *testing.T) { + app := New() + app.AddStaticFiles(endpoint, dir) + + stored := app.httpServer.staticFiles[dir] + assert.Equal(t, "/static", stored, "endpoint %q should normalize to /static", endpoint) + + // Wire the stored endpoint exactly as App.Run does and prove it serves. + router := gofrHTTP.NewRouter() + router.AddStaticFiles(app.Logger(), stored, dir) + + for _, path := range []string{"/static", "/static/index.html"} { + w := httptest.NewRecorder() + router.ServeHTTP(w, httptest.NewRequestWithContext(t.Context(), http.MethodGet, path, http.NoBody)) + + assert.Equal(t, http.StatusOK, w.Code, "endpoint %q: GET %s", endpoint, path) + assert.Equal(t, "Index", strings.TrimSpace(w.Body.String()), "endpoint %q: GET %s", endpoint, path) + } + }) + } +} + func TestNewSetsHTTPRegisteredWhenStaticDirExists(t *testing.T) { testutil.NewServerConfigs(t) From 3538ad9d1c7d38dd37c3433f00636c14eee4452d Mon Sep 17 00:00:00 2001 From: Sreejit Pradhan Date: Tue, 8 Sep 2026 10:38:51 +0530 Subject: [PATCH 2/3] fix: normalize endpoint in Router.AddStaticFiles for direct callers --- pkg/gofr/gofr_test.go | 7 ++++-- pkg/gofr/http/router.go | 6 ++++++ pkg/gofr/http/router_test.go | 42 ++++++++++++++++++++++++++++++++++++ 3 files changed, 53 insertions(+), 2 deletions(-) diff --git a/pkg/gofr/gofr_test.go b/pkg/gofr/gofr_test.go index 281a971474..de2bb12417 100644 --- a/pkg/gofr/gofr_test.go +++ b/pkg/gofr/gofr_test.go @@ -1341,19 +1341,22 @@ func TestAddStaticFilesEndpointForms(t *testing.T) { dir := t.TempDir() require.NoError(t, os.WriteFile(filepath.Join(dir, "index.html"), []byte("Index"), 0600)) + // A const (rather than a repeated literal) keeps goconst quiet about the request paths. + const wantEndpoint = "/static" + for _, endpoint := range []string{"static", "/static", "static/", "/static/"} { t.Run(endpoint, func(t *testing.T) { app := New() app.AddStaticFiles(endpoint, dir) stored := app.httpServer.staticFiles[dir] - assert.Equal(t, "/static", stored, "endpoint %q should normalize to /static", endpoint) + assert.Equal(t, wantEndpoint, stored, "endpoint %q should normalize to /static", endpoint) // Wire the stored endpoint exactly as App.Run does and prove it serves. router := gofrHTTP.NewRouter() router.AddStaticFiles(app.Logger(), stored, dir) - for _, path := range []string{"/static", "/static/index.html"} { + for _, path := range []string{wantEndpoint, wantEndpoint + "/index.html"} { w := httptest.NewRecorder() router.ServeHTTP(w, httptest.NewRequestWithContext(t.Context(), http.MethodGet, path, http.NoBody)) diff --git a/pkg/gofr/http/router.go b/pkg/gofr/http/router.go index e94352c039..60eb98f710 100644 --- a/pkg/gofr/http/router.go +++ b/pkg/gofr/http/router.go @@ -342,6 +342,12 @@ type staticFileConfig struct { } func (rou *Router) AddStaticFiles(logger logging.Logger, endpoint, dirName string) { + // The route patterns below are built from endpoint verbatim, and ServeHTTP normalizes + // incoming paths with path.Clean — so an endpoint carrying a leading or trailing slash + // registers a pattern no request can ever match. Normalize here, where the patterns are + // built, so a direct caller cannot register a dead route either. + endpoint = "/" + strings.Trim(endpoint, "/") + // staticHandler resolves each request to an absolute, cleaned path and the containment check // compares it against directoryName as a string, so the two have to be in the same form. // Resolving once here covers a relative name and an absolute one carrying a trailing separator diff --git a/pkg/gofr/http/router_test.go b/pkg/gofr/http/router_test.go index 2cea498e42..cca96db34f 100644 --- a/pkg/gofr/http/router_test.go +++ b/pkg/gofr/http/router_test.go @@ -567,6 +567,48 @@ func Test_StaticFileServing_DirectoryNameForms(t *testing.T) { } } +// staticTestEndpoint is the normalized form every endpoint spelling below must converge to. +// A const (rather than a repeated literal) keeps goconst quiet about the request paths. +const staticTestEndpoint = "/static" + +// Test_StaticFileServing_EndpointForms covers the endpoint forms a direct caller can pass +// to Router.AddStaticFiles. The route patterns are built from endpoint verbatim while +// ServeHTTP normalizes incoming paths with path.Clean, so a leading or trailing slash +// registers a pattern no request can ever match — every request under the endpoint 404s +// even though registration logs success. (App.AddStaticFiles normalizes as well, so its +// error logs agree on one form; this is the guarantee at the site where the patterns +// are built.) +func Test_StaticFileServing_EndpointForms(t *testing.T) { + tempDir := t.TempDir() + + require.NoError(t, os.WriteFile(filepath.Join(tempDir, "index.html"), []byte("Index"), 0600)) + + tests := []struct { + name string + endpoint string + }{ + {"bare", "static"}, + {"leading slash", "/static"}, + {"trailing slash", "static/"}, + {"leading and trailing slash", "/static/"}, + } + + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + for _, path := range []string{staticTestEndpoint, staticTestEndpoint + "/index.html"} { + router := NewRouter() + router.AddStaticFiles(logging.NewMockLogger(logging.DEBUG), tc.endpoint, tempDir) + + w := httptest.NewRecorder() + router.ServeHTTP(w, httptest.NewRequestWithContext(t.Context(), http.MethodGet, path, http.NoBody)) + + assert.Equal(t, http.StatusOK, w.Code, "endpoint %q: GET %s", tc.endpoint, path) + assert.Equal(t, "Index", strings.TrimSpace(w.Body.String()), "endpoint %q: GET %s", tc.endpoint, path) + } + }) + } +} + // Test_StaticFileServing_MethodNotAllowed covers the methods a static endpoint does not implement. // The routes carry no .Methods() matcher — deliberately, because restricting them there drops the // request to the catch-all and answers 404, which contradicts the 200 the same path gives for GET — From 842431de05d544a9eda927f91299eea0e67f6614 Mon Sep 17 00:00:00 2001 From: Sreejit Pradhan Date: Fri, 25 Sep 2026 14:52:07 +0530 Subject: [PATCH 3/3] chore: trigger CI for trailing-slash static fix