Skip to content

Commit cc6a6a7

Browse files
lammellammel
committed
Fix handling of static routes with/without trailing slash with added tests
1 parent fd7db74 commit cc6a6a7

File tree

3 files changed

+190
-5
lines changed

3 files changed

+190
-5
lines changed

‎echo.go

Lines changed: 9 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -503,9 +503,15 @@ func (common) static(prefix, root string, get func(string, HandlerFunc, ...Middl
503503
}
504504
return c.File(name)
505505
}
506-
get(prefix, h)
507-
if prefix == "/" {
508-
return get(prefix+"*", h)
506+
// Handle added routes based on trailing slash:
507+
// /prefix => exact route "/prefix" + any route "/prefix/*"
508+
// /prefix/ => only any route "/prefix/*"
509+
if prefix != "" {
510+
if prefix[len(prefix)-1] == '/' {
511+
// Only add any route for intentional trailing slash
512+
return get(prefix+"*", h)
513+
}
514+
get(prefix, h)
509515
}
510516
return get(prefix+"/*", h)
511517
}

‎echo_test.go

Lines changed: 33 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -114,6 +114,23 @@ func TestEchoStatic(t *testing.T) {
114114
expectHeaderLocation: "/static/",
115115
expectBodyStartsWith: "",
116116
},
117+
{
118+
name: "Prefixed directory 404 (request URL without slash)",
119+
givenPrefix: "/folder/", // trailing slash will intentionally not match "/folder"
120+
givenRoot: "_fixture",
121+
whenURL: "/folder", // no trailing slash
122+
expectStatus: http.StatusNotFound,
123+
expectBodyStartsWith: "{\"message\":\"Not Found\"}\n",
124+
},
125+
{
126+
name: "Prefixed directory redirect (without slash redirect to slash)",
127+
givenPrefix: "/folder", // no trailing slash shall match /folder and /folder/*
128+
givenRoot: "_fixture",
129+
whenURL: "/folder", // no trailing slash
130+
expectStatus: http.StatusMovedPermanently,
131+
expectHeaderLocation: "/folder/",
132+
expectBodyStartsWith: "",
133+
},
117134
{
118135
name: "Directory with index.html",
119136
givenPrefix: "/",
@@ -122,6 +139,22 @@ func TestEchoStatic(t *testing.T) {
122139
expectStatus: http.StatusOK,
123140
expectBodyStartsWith: "<!doctype html>",
124141
},
142+
{
143+
name: "Prefixed directory with index.html (prefix ending with slash)",
144+
givenPrefix: "/assets/",
145+
givenRoot: "_fixture",
146+
whenURL: "/assets/",
147+
expectStatus: http.StatusOK,
148+
expectBodyStartsWith: "<!doctype html>",
149+
},
150+
{
151+
name: "Prefixed directory with index.html (prefix ending without slash)",
152+
givenPrefix: "/assets",
153+
givenRoot: "_fixture",
154+
whenURL: "/assets/",
155+
expectStatus: http.StatusOK,
156+
expectBodyStartsWith: "<!doctype html>",
157+
},
125158
{
126159
name: "Sub-directory with index.html",
127160
givenPrefix: "/",

‎middleware/static_test.go

Lines changed: 148 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,11 +1,13 @@
11
package middleware
22

33
import (
4-
"github.com/labstack/echo/v4"
5-
"github.com/stretchr/testify/assert"
64
"net/http"
75
"net/http/httptest"
6+
"strings"
87
"testing"
8+
9+
"github.com/labstack/echo/v4"
10+
"github.com/stretchr/testify/assert"
911
)
1012

1113
func TestStatic(t *testing.T) {
@@ -131,3 +133,147 @@ func TestStatic(t *testing.T) {
131133
})
132134
}
133135
}
136+
137+
func TestStatic_GroupWithStatic(t *testing.T) {
138+
var testCases = []struct {
139+
name string
140+
givenGroup string
141+
givenPrefix string
142+
givenRoot string
143+
whenURL string
144+
expectStatus int
145+
expectHeaderLocation string
146+
expectBodyStartsWith string
147+
}{
148+
{
149+
name: "ok",
150+
givenPrefix: "/images",
151+
givenRoot: "../_fixture/images",
152+
whenURL: "/group/images/walle.png",
153+
expectStatus: http.StatusOK,
154+
expectBodyStartsWith: string([]byte{0x89, 0x50, 0x4e, 0x47}),
155+
},
156+
{
157+
name: "No file",
158+
givenPrefix: "/images",
159+
givenRoot: "../_fixture/scripts",
160+
whenURL: "/group/images/bolt.png",
161+
expectStatus: http.StatusNotFound,
162+
expectBodyStartsWith: "{\"message\":\"Not Found\"}\n",
163+
},
164+
{
165+
name: "Directory not found (no trailing slash)",
166+
givenPrefix: "/images",
167+
givenRoot: "../_fixture/images",
168+
whenURL: "/group/images/",
169+
expectStatus: http.StatusNotFound,
170+
expectBodyStartsWith: "{\"message\":\"Not Found\"}\n",
171+
},
172+
{
173+
name: "Directory redirect",
174+
givenPrefix: "/",
175+
givenRoot: "../_fixture",
176+
whenURL: "/group/folder",
177+
expectStatus: http.StatusMovedPermanently,
178+
expectHeaderLocation: "/group/folder/",
179+
expectBodyStartsWith: "",
180+
},
181+
{
182+
name: "Prefixed directory 404 (request URL without slash)",
183+
givenGroup: "_fixture",
184+
givenPrefix: "/folder/", // trailing slash will intentionally not match "/folder"
185+
givenRoot: "../_fixture",
186+
whenURL: "/_fixture/folder", // no trailing slash
187+
expectStatus: http.StatusNotFound,
188+
expectBodyStartsWith: "{\"message\":\"Not Found\"}\n",
189+
},
190+
{
191+
name: "Prefixed directory redirect (without slash redirect to slash)",
192+
givenGroup: "_fixture",
193+
givenPrefix: "/folder", // no trailing slash shall match /folder and /folder/*
194+
givenRoot: "../_fixture",
195+
whenURL: "/_fixture/folder", // no trailing slash
196+
expectStatus: http.StatusMovedPermanently,
197+
expectHeaderLocation: "/_fixture/folder/",
198+
expectBodyStartsWith: "",
199+
},
200+
{
201+
name: "Directory with index.html",
202+
givenPrefix: "/",
203+
givenRoot: "../_fixture",
204+
whenURL: "/group/",
205+
expectStatus: http.StatusOK,
206+
expectBodyStartsWith: "<!doctype html>",
207+
},
208+
{
209+
name: "Prefixed directory with index.html (prefix ending with slash)",
210+
givenPrefix: "/assets/",
211+
givenRoot: "../_fixture",
212+
whenURL: "/group/assets/",
213+
expectStatus: http.StatusOK,
214+
expectBodyStartsWith: "<!doctype html>",
215+
},
216+
{
217+
name: "Prefixed directory with index.html (prefix ending without slash)",
218+
givenPrefix: "/assets",
219+
givenRoot: "../_fixture",
220+
whenURL: "/group/assets/",
221+
expectStatus: http.StatusOK,
222+
expectBodyStartsWith: "<!doctype html>",
223+
},
224+
{
225+
name: "Sub-directory with index.html",
226+
givenPrefix: "/",
227+
givenRoot: "../_fixture",
228+
whenURL: "/group/folder/",
229+
expectStatus: http.StatusOK,
230+
expectBodyStartsWith: "<!doctype html>",
231+
},
232+
{
233+
name: "do not allow directory traversal (backslash - windows separator)",
234+
givenPrefix: "/",
235+
givenRoot: "../_fixture/",
236+
whenURL: `/group/..\\middleware/basic_auth.go`,
237+
expectStatus: http.StatusNotFound,
238+
expectBodyStartsWith: "{\"message\":\"Not Found\"}\n",
239+
},
240+
{
241+
name: "do not allow directory traversal (slash - unix separator)",
242+
givenPrefix: "/",
243+
givenRoot: "../_fixture/",
244+
whenURL: `/group/../middleware/basic_auth.go`,
245+
expectStatus: http.StatusNotFound,
246+
expectBodyStartsWith: "{\"message\":\"Not Found\"}\n",
247+
},
248+
}
249+
250+
for _, tc := range testCases {
251+
t.Run(tc.name, func(t *testing.T) {
252+
e := echo.New()
253+
group := "/group"
254+
if tc.givenGroup != "" {
255+
group = tc.givenGroup
256+
}
257+
g := e.Group(group)
258+
g.Static(tc.givenPrefix, tc.givenRoot)
259+
260+
req := httptest.NewRequest(http.MethodGet, tc.whenURL, nil)
261+
rec := httptest.NewRecorder()
262+
e.ServeHTTP(rec, req)
263+
assert.Equal(t, tc.expectStatus, rec.Code)
264+
body := rec.Body.String()
265+
if tc.expectBodyStartsWith != "" {
266+
assert.True(t, strings.HasPrefix(body, tc.expectBodyStartsWith))
267+
} else {
268+
assert.Equal(t, "", body)
269+
}
270+
271+
if tc.expectHeaderLocation != "" {
272+
assert.Equal(t, tc.expectHeaderLocation, rec.Header().Get(echo.HeaderLocation))
273+
} else {
274+
_, ok := rec.Result().Header[echo.HeaderLocation]
275+
assert.False(t, ok)
276+
}
277+
})
278+
}
279+
}

0 commit comments

Comments
 (0)