Skip to content

Commit 716eb18

Browse files
lammelpwli0755
lammel
and
pwli0755
authored
Handle static routes with trailing slash (#1747)
- Fix Static file route not working without trailing slash - Add tests for static middleware with/without trailing slash - Add tests for static middleware under group Co-authored-by: pwli <lipw0755@gmail.com>
1 parent 0bdb45c commit 716eb18

File tree

3 files changed

+233
-4
lines changed

3 files changed

+233
-4
lines changed

‎echo.go

Lines changed: 9 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -503,8 +503,15 @@ func (common) static(prefix, root string, get func(string, HandlerFunc, ...Middl
503503
}
504504
return c.File(name)
505505
}
506-
if prefix == "/" {
507-
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)
508515
}
509516
return get(prefix+"/*", h)
510517
}

‎echo_test.go

Lines changed: 76 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -105,6 +105,32 @@ func TestEchoStatic(t *testing.T) {
105105
expectHeaderLocation: "/folder/",
106106
expectBodyStartsWith: "",
107107
},
108+
{
109+
name: "Directory Redirect with non-root path",
110+
givenPrefix: "/static",
111+
givenRoot: "_fixture",
112+
whenURL: "/static",
113+
expectStatus: http.StatusMovedPermanently,
114+
expectHeaderLocation: "/static/",
115+
expectBodyStartsWith: "",
116+
},
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+
},
108134
{
109135
name: "Directory with index.html",
110136
givenPrefix: "/",
@@ -113,6 +139,22 @@ func TestEchoStatic(t *testing.T) {
113139
expectStatus: http.StatusOK,
114140
expectBodyStartsWith: "<!doctype html>",
115141
},
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+
},
116158
{
117159
name: "Sub-directory with index.html",
118160
givenPrefix: "/",
@@ -164,6 +206,40 @@ func TestEchoStatic(t *testing.T) {
164206
}
165207
}
166208

209+
func TestEchoStaticRedirectIndex(t *testing.T) {
210+
assert := assert.New(t)
211+
e := New()
212+
213+
// HandlerFunc
214+
e.Static("/static", "_fixture")
215+
216+
errCh := make(chan error)
217+
218+
go func() {
219+
errCh <- e.Start("127.0.0.1:1323")
220+
}()
221+
222+
time.Sleep(200 * time.Millisecond)
223+
224+
if resp, err := http.Get("http://127.0.0.1:1323/static"); err == nil {
225+
defer resp.Body.Close()
226+
assert.Equal(http.StatusOK, resp.StatusCode)
227+
228+
if body, err := ioutil.ReadAll(resp.Body); err == nil {
229+
assert.Equal(true, strings.HasPrefix(string(body), "<!doctype html>"))
230+
} else {
231+
assert.Fail(err.Error())
232+
}
233+
234+
} else {
235+
assert.Fail(err.Error())
236+
}
237+
238+
if err := e.Close(); err != nil {
239+
t.Fatal(err)
240+
}
241+
}
242+
167243
func TestEchoFile(t *testing.T) {
168244
e := New()
169245
e.File("/walle", "_fixture/images/walle.png")

‎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)