Skip to content

Commit

Permalink
static middleware: do not unescape path twice
Browse files Browse the repository at this point in the history
The URL path is already unescaped by the http server. Unescaping again
could lead to FileNotFound errors if the file on disk contains '%' chars.
  • Loading branch information
georgmu committed Feb 28, 2024
1 parent fa70db8 commit 852dede
Show file tree
Hide file tree
Showing 4 changed files with 27 additions and 5 deletions.
1 change: 1 addition & 0 deletions _fixture/special/file with spaces.txt
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
file with spaces in filename
1 change: 1 addition & 0 deletions _fixture/special/file%20with%20percent.txt
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
file with percents in filename
5 changes: 0 additions & 5 deletions middleware/static.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,6 @@ import (
"fmt"
"html/template"
"net/http"
"net/url"
"os"
"path"
"strings"
Expand Down Expand Up @@ -171,10 +170,6 @@ func StaticWithConfig(config StaticConfig) echo.MiddlewareFunc {
if strings.HasSuffix(c.Path(), "*") { // When serving from a group, e.g. `/static*`.
p = c.Param("*")
}
p, err = url.PathUnescape(p)
if err != nil {
return
}
name := path.Join(config.Root, path.Clean("/"+p)) // "/"+ for security

if config.IgnoreBase {
Expand Down
25 changes: 25 additions & 0 deletions middleware/static_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import (
"io/fs"
"net/http"
"net/http/httptest"
"net/url"
"os"
"strings"
"testing"
Expand Down Expand Up @@ -123,6 +124,30 @@ func TestStatic(t *testing.T) {
expectCode: http.StatusOK,
expectContains: "<title>Echo</title>",
},
{
name: "ok, serve file with whitespace on disk",
whenURL: "/special/" + url.PathEscape("file with spaces.txt"),
expectCode: http.StatusOK,
expectContains: "file with spaces in filename",
},
{
name: "nok, when filename with spaces on disk is decoded twice",
whenURL: "/special/" + url.PathEscape("file%20with%20spaces.txt"),
expectCode: http.StatusNotFound,
expectContains: "{\"message\":\"Not Found\"}\n",
},
{
name: "ok, serve file with '%20' in filename on disk",
whenURL: "/special/" + url.PathEscape("file%20with%20percent.txt"), // file%2520with%2520percent.txt
expectCode: http.StatusOK,
expectContains: "file with percents in filename",
},
{
name: "nok, when filename with '%20' on disk is decoded twice",
whenURL: "/special/file%20with%20percent.txt",
expectCode: http.StatusNotFound,
expectContains: "{\"message\":\"Not Found\"}\n",
},
}

for _, tc := range testCases {
Expand Down

0 comments on commit 852dede

Please sign in to comment.