Fix e.File() being picky with relative paths after 4.7.0 introduced echo.Fs support (Go1.16+)

Author: aldas

echo_fs_go1.16.go

@@ -113,7 +113,7 @@ func newDefaultFS() *defaultFS {
 }
 
 func (fs defaultFS) Open(name string) (fs.File, error) {
-	return fs.fs.Open(name)
+	return fs.fs.Open(filepath.Clean(name))
 }
 
 func subFS(currentFs fs.FS, root string) (fs.FS, error) {

echo_fs_go1.16_test.go

@@ -263,3 +263,48 @@ func TestEcho_StaticPanic(t *testing.T) {
 		})
 	}
 }
+
+func TestEchoStatic16(t *testing.T) { // when we drop Go1.15 support merge these testcases with `TestEchoStatic()`
+	var testCases = []struct {
+		name                 string
+		givenPrefix          string
+		givenRoot            string
+		whenURL              string
+		expectStatus         int
+		expectHeaderLocation string
+		expectBodyStartsWith string
+	}{
+		{ // `e.Static` is not meant to work by pointing `root` to file. This would be insecure.
+			name:                 "nok, should not work when relative path for root points to file",
+			givenPrefix:          "/images",
+			givenRoot:            "./_fixture/images/walle.png",
+			whenURL:              "/images",
+			expectStatus:         http.StatusNotFound,
+			expectBodyStartsWith: "{\"message\":\"Not Found\"}\n",
+		},
+	}
+
+	for _, tc := range testCases {
+		t.Run(tc.name, func(t *testing.T) {
+			e := New()
+			e.Static(tc.givenPrefix, tc.givenRoot)
+			req := httptest.NewRequest(http.MethodGet, tc.whenURL, nil)
+			rec := httptest.NewRecorder()
+			e.ServeHTTP(rec, req)
+			assert.Equal(t, tc.expectStatus, rec.Code)
+			body := rec.Body.String()
+			if tc.expectBodyStartsWith != "" {
+				assert.True(t, strings.HasPrefix(body, tc.expectBodyStartsWith))
+			} else {
+				assert.Equal(t, "", body)
+			}
+
+			if tc.expectHeaderLocation != "" {
+				assert.Equal(t, tc.expectHeaderLocation, rec.Result().Header["Location"][0])
+			} else {
+				_, ok := rec.Result().Header["Location"]
+				assert.False(t, ok)
+			}
+		})
+	}
+}

echo_test.go

@@ -84,6 +84,14 @@ func TestEchoStatic(t *testing.T) {
 			expectStatus:         http.StatusOK,
 			expectBodyStartsWith: string([]byte{0x89, 0x50, 0x4e, 0x47}),
 		},
+		{
+			name:                 "ok with relative path for root points to directory",
+			givenPrefix:          "/images",
+			givenRoot:            "./_fixture/images",
+			whenURL:              "/images/walle.png",
+			expectStatus:         http.StatusOK,
+			expectBodyStartsWith: string([]byte{0x89, 0x50, 0x4e, 0x47}),
+		},
 		{
 			name:                 "No file",
 			givenPrefix:          "/images",
@@ -246,11 +254,54 @@ func TestEchoStaticRedirectIndex(t *testing.T) {
 }
 
 func TestEchoFile(t *testing.T) {
-	e := New()
-	e.File("/walle", "_fixture/images/walle.png")
-	c, b := request(http.MethodGet, "/walle", e)
-	assert.Equal(t, http.StatusOK, c)
-	assert.NotEmpty(t, b)
+	var testCases = []struct {
+		name             string
+		givenPath        string
+		givenFile        string
+		whenPath         string
+		expectCode       int
+		expectStartsWith string
+	}{
+		{
+			name:             "ok",
+			givenPath:        "/walle",
+			givenFile:        "_fixture/images/walle.png",
+			whenPath:         "/walle",
+			expectCode:       http.StatusOK,
+			expectStartsWith: string([]byte{0x89, 0x50, 0x4e}),
+		},
+		{
+			name:             "ok with relative path",
+			givenPath:        "/",
+			givenFile:        "./go.mod",
+			whenPath:         "/",
+			expectCode:       http.StatusOK,
+			expectStartsWith: "module github.com/labstack/echo/v",
+		},
+		{
+			name:             "nok file does not exist",
+			givenPath:        "/",
+			givenFile:        "./this-file-does-not-exist",
+			whenPath:         "/",
+			expectCode:       http.StatusNotFound,
+			expectStartsWith: "{\"message\":\"Not Found\"}\n",
+		},
+	}
+
+	for _, tc := range testCases {
+		t.Run(tc.name, func(t *testing.T) {
+			e := New() // we are using echo.defaultFS instance
+			e.File(tc.givenPath, tc.givenFile)
+
+			c, b := request(http.MethodGet, tc.whenPath, e)
+			assert.Equal(t, tc.expectCode, c)
+
+			if len(b) > len(tc.expectStartsWith) {
+				b = b[:len(tc.expectStartsWith)]
+			}
+			assert.Equal(t, tc.expectStartsWith, b)
+		})
+	}
 }
 
 func TestEchoMiddleware(t *testing.T) {