diff --git a/pkg/gofr/gofr.go b/pkg/gofr/gofr.go index d3d090744a..898b1fb2c1 100644 --- a/pkg/gofr/gofr.go +++ b/pkg/gofr/gofr.go @@ -472,7 +472,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 bea2318fc9..54fc12964b 100644 --- a/pkg/gofr/gofr_test.go +++ b/pkg/gofr/gofr_test.go @@ -1358,6 +1358,43 @@ 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)) + + // 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, 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{wantEndpoint, wantEndpoint + "/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) diff --git a/pkg/gofr/http/router.go b/pkg/gofr/http/router.go index b968d3c32e..9143278858 100644 --- a/pkg/gofr/http/router.go +++ b/pkg/gofr/http/router.go @@ -473,6 +473,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 —