From 05021d3966cfbedf5e62c9d72d991d9dbbf4ba4b Mon Sep 17 00:00:00 2001 From: Vishal Rana Date: Sat, 13 Jun 2026 13:13:18 -0700 Subject: [PATCH 1/2] fix(middleware/static): don't double-unescape request path (#2599) http.Request.URL.Path is already decoded by net/http, but the static middleware unescaped it again by default, so files whose names contain a percent sign were not downloadable ("/100%25.txt" -> "invalid URL escape"). Default pathUnescape to false; only the wildcard param from a group route (set explicitly below) may still be escaped and is handled by the existing DisablePathUnescaping toggle. Co-Authored-By: Claude Opus 4.8 (1M context) --- middleware/static.go | 5 +++- middleware/static_percent_test.go | 41 +++++++++++++++++++++++++++++++ 2 files changed, 45 insertions(+), 1 deletion(-) create mode 100644 middleware/static_percent_test.go diff --git a/middleware/static.go b/middleware/static.go index a9e67b626..152ddedee 100644 --- a/middleware/static.go +++ b/middleware/static.go @@ -196,7 +196,10 @@ func (config StaticConfig) ToMiddleware() (echo.MiddlewareFunc, error) { } p := c.Request().URL.Path - pathUnescape := true + // URL.Path is already decoded by net/http, so it must not be unescaped + // again (doing so breaks file names containing '%', see #2599). Only the + // wildcard param from a group route (set below) may still be escaped. + pathUnescape := false if strings.HasSuffix(c.Path(), "*") { // When serving from a group, e.g. `/static*`. p = c.Param("*") pathUnescape = !config.DisablePathUnescaping // because router could already do PathUnescape diff --git a/middleware/static_percent_test.go b/middleware/static_percent_test.go new file mode 100644 index 000000000..647da977c --- /dev/null +++ b/middleware/static_percent_test.go @@ -0,0 +1,41 @@ +// SPDX-License-Identifier: MIT +// SPDX-FileCopyrightText: © 2015 LabStack LLC and Echo contributors + +package middleware + +import ( + "net/http" + "net/http/httptest" + "testing" + "testing/fstest" + + "github.com/labstack/echo/v5" + "github.com/stretchr/testify/assert" +) + +// Regression test for #2599: a file whose name contains a percent sign must be +// downloadable. http.Request.URL.Path is already decoded by net/http, so the +// static middleware must not unescape it a second time (which turned +// "/100%25.txt" into an "invalid URL escape" error or a missing file). +func TestStatic_servesFileWithPercentInName(t *testing.T) { + e := echo.New() + e.Use(StaticWithConfig(StaticConfig{ + Root: ".", + Filesystem: fstest.MapFS{ + "100%.txt": &fstest.MapFile{Data: []byte("hundred percent")}, + "foo%20bar.txt": &fstest.MapFile{Data: []byte("literal percent twenty")}, + }, + })) + + cases := map[string]string{ + "/100%25.txt": "hundred percent", + "/foo%2520bar.txt": "literal percent twenty", + } + for url, want := range cases { + req := httptest.NewRequest(http.MethodGet, url, nil) + rec := httptest.NewRecorder() + e.ServeHTTP(rec, req) + assert.Equal(t, http.StatusOK, rec.Code, "GET %s should serve the file", url) + assert.Equal(t, want, rec.Body.String(), "GET %s should return the file contents", url) + } +} From 0e0d1c6ec6b5fea79111078d4d97dac43e00bfad Mon Sep 17 00:00:00 2001 From: Vishal Rana Date: Sat, 13 Jun 2026 13:18:56 -0700 Subject: [PATCH 2/2] test(static): add percent-encoded traversal-protection case (#2599) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Companion test confirming that defaulting pathUnescape to false does not weaken traversal protection — single-, double-, and dot-encoded "../" all stay within the served root. Co-Authored-By: Claude Opus 4.8 (1M context) --- middleware/static_percent_test.go | 23 +++++++++++++++++++++++ 1 file changed, 23 insertions(+) diff --git a/middleware/static_percent_test.go b/middleware/static_percent_test.go index 647da977c..a65d75c90 100644 --- a/middleware/static_percent_test.go +++ b/middleware/static_percent_test.go @@ -39,3 +39,26 @@ func TestStatic_servesFileWithPercentInName(t *testing.T) { assert.Equal(t, want, rec.Body.String(), "GET %s should return the file contents", url) } } + +// Companion to #2599: not unescaping the already-decoded path must not weaken +// traversal protection. A percent-encoded "../" must not escape the served root +// (and notably must not be re-assembled from double-encoded input, as the old +// double-unescape could do). +func TestStatic_percentEncodedTraversalIsBlocked(t *testing.T) { + e := echo.New() + e.Use(StaticWithConfig(StaticConfig{ + Root: "public", + Filesystem: fstest.MapFS{ + "public/page.txt": &fstest.MapFile{Data: []byte("public page")}, + "secret.txt": &fstest.MapFile{Data: []byte("SECRET")}, + }, + })) + + for _, url := range []string{"/..%2fsecret.txt", "/..%252fsecret.txt", "/%2e%2e%2fsecret.txt"} { + req := httptest.NewRequest(http.MethodGet, url, nil) + rec := httptest.NewRecorder() + e.ServeHTTP(rec, req) + assert.NotEqual(t, http.StatusOK, rec.Code, "GET %s must not escape the served root", url) + assert.NotContains(t, rec.Body.String(), "SECRET", "GET %s must not leak files above the root", url) + } +}