Repository: labstack/echo
Pull request #1747 (2021-01-02 23:09:53)

Handle static routes with trailing slash
This PR superceedes PR #1723 (which resolves #1671) with contributions from @pwli0755 adding required tests and a more fitting solution.


codecov[bot]: # [Codecov](https://codecov.io/gh/labstack/echo/pull/1747?src=pr&el=h1) Report
> Merging [#1747](https://codecov.io/gh/labstack/echo/pull/1747?src=pr&el=desc) (cc6a6a7) into [master](https://codecov.io/gh/labstack/echo/commit/0bdb45c583df594a9991950bc065b801bce5533d?el=desc) (0bdb45c) will **increase** coverage by `0.01%`.
> The diff coverage is `100.00%`.

[![Impacted file tree graph](https://codecov.io/gh/labstack/echo/pull/1747/graphs/tree.svg?width=650&height=150&src=pr&token=vqnT76l79i)](https://codecov.io/gh/labstack/echo/pull/1747?src=pr&el=tree)

```diff
@@            Coverage Diff             @@
##           master    #1747      +/-   ##
==========================================
+ Coverage   86.62%   86.63%   +0.01%     
==========================================
  Files          29       29              
  Lines        2063     2065       +2     
==========================================
+ Hits         1787     1789       +2     
  Misses        176      176              
  Partials      100      100              
```


| [Impacted Files](https://codecov.io/gh/labstack/echo/pull/1747?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [echo.go](https://codecov.io/gh/labstack/echo/pull/1747/diff?src=pr&el=tree#diff-ZWNoby5nbw==) | `91.53% <100.00%> (+0.05%)` | :arrow_up: |

------

[Continue to review full report at Codecov](https://codecov.io/gh/labstack/echo/pull/1747?src=pr&el=continue).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/labstack/echo/pull/1747?src=pr&el=footer). Last update [0bdb45c...cc6a6a7](https://codecov.io/gh/labstack/echo/pull/1747?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).


Patch:
diff --git a/echo.go b/echo.go
index 64e64c2..6db485d 100644
--- a/echo.go
+++ b/echo.go
@@ -503,8 +503,15 @@ func (common) static(prefix, root string, get func(string, HandlerFunc, ...Middl
 		}
 		return c.File(name)
 	}
-	if prefix == "/" {
-		return get(prefix+"*", h)
+	// Handle added routes based on trailing slash:
+	// 	/prefix  => exact route "/prefix" + any route "/prefix/*"
+	// 	/prefix/ => only any route "/prefix/*"
+	if prefix != "" {
+		if prefix[len(prefix)-1] == '/' {
+			// Only add any route for intentional trailing slash
+			return get(prefix+"*", h)
+		}
+		get(prefix, h)
 	}
 	return get(prefix+"/*", h)
 }


Test patch:
diff --git a/echo_test.go b/echo_test.go
index 7f35974..781b901 100644
--- a/echo_test.go
+++ b/echo_test.go
@@ -105,6 +105,32 @@ func TestEchoStatic(t *testing.T) {
 			expectHeaderLocation: "/folder/",
 			expectBodyStartsWith: "",
 		},
+		{
+			name:                 "Directory Redirect with non-root path",
+			givenPrefix:          "/static",
+			givenRoot:            "_fixture",
+			whenURL:              "/static",
+			expectStatus:         http.StatusMovedPermanently,
+			expectHeaderLocation: "/static/",
+			expectBodyStartsWith: "",
+		},
+		{
+			name:                 "Prefixed directory 404 (request URL without slash)",
+			givenPrefix:          "/folder/", // trailing slash will intentionally not match "/folder"
+			givenRoot:            "_fixture",
+			whenURL:              "/folder", // no trailing slash
+			expectStatus:         http.StatusNotFound,
+			expectBodyStartsWith: "{\"message\":\"Not Found\"}\n",
+		},
+		{
+			name:                 "Prefixed directory redirect (without slash redirect to slash)",
+			givenPrefix:          "/folder", // no trailing slash shall match /folder and /folder/*
+			givenRoot:            "_fixture",
+			whenURL:              "/folder", // no trailing slash
+			expectStatus:         http.StatusMovedPermanently,
+			expectHeaderLocation: "/folder/",
+			expectBodyStartsWith: "",
+		},
 		{
 			name:                 "Directory with index.html",
 			givenPrefix:          "/",
@@ -113,6 +139,22 @@ func TestEchoStatic(t *testing.T) {
 			expectStatus:         http.StatusOK,
 			expectBodyStartsWith: "<!doctype html>",
 		},
+		{
+			name:                 "Prefixed directory with index.html (prefix ending with slash)",
+			givenPrefix:          "/assets/",
+			givenRoot:            "_fixture",
+			whenURL:              "/assets/",
+			expectStatus:         http.StatusOK,
+			expectBodyStartsWith: "<!doctype html>",
+		},
+		{
+			name:                 "Prefixed directory with index.html (prefix ending without slash)",
+			givenPrefix:          "/assets",
+			givenRoot:            "_fixture",
+			whenURL:              "/assets/",
+			expectStatus:         http.StatusOK,
+			expectBodyStartsWith: "<!doctype html>",
+		},
 		{
 			name:                 "Sub-directory with index.html",
 			givenPrefix:          "/",
@@ -164,6 +206,40 @@ func TestEchoStatic(t *testing.T) {
 	}
 }
 
+func TestEchoStaticRedirectIndex(t *testing.T) {
+	assert := assert.New(t)
+	e := New()
+
+	// HandlerFunc
+	e.Static("/static", "_fixture")
+
+	errCh := make(chan error)
+
+	go func() {
+		errCh <- e.Start("127.0.0.1:1323")
+	}()
+
+	time.Sleep(200 * time.Millisecond)
+
+	if resp, err := http.Get("http://127.0.0.1:1323/static"); err == nil {
+		defer resp.Body.Close()
+		assert.Equal(http.StatusOK, resp.StatusCode)
+
+		if body, err := ioutil.ReadAll(resp.Body); err == nil {
+			assert.Equal(true, strings.HasPrefix(string(body), "<!doctype html>"))
+		} else {
+			assert.Fail(err.Error())
+		}
+
+	} else {
+		assert.Fail(err.Error())
+	}
+
+	if err := e.Close(); err != nil {
+		t.Fatal(err)
+	}
+}
+
 func TestEchoFile(t *testing.T) {
 	e := New()
 	e.File("/walle", "_fixture/images/walle.png")
diff --git a/middleware/static_test.go b/middleware/static_test.go
index 3e6ca56..8c0c97d 100644
--- a/middleware/static_test.go
+++ b/middleware/static_test.go
@@ -1,11 +1,13 @@
 package middleware
 
 import (
-	"github.com/labstack/echo/v4"
-	"github.com/stretchr/testify/assert"
 	"net/http"
 	"net/http/httptest"
+	"strings"
 	"testing"
+
+	"github.com/labstack/echo/v4"
+	"github.com/stretchr/testify/assert"
 )
 
 func TestStatic(t *testing.T) {
@@ -131,3 +133,147 @@ func TestStatic(t *testing.T) {
 		})
 	}
 }
+
+func TestStatic_GroupWithStatic(t *testing.T) {
+	var testCases = []struct {
+		name                 string
+		givenGroup           string
+		givenPrefix          string
+		givenRoot            string
+		whenURL              string
+		expectStatus         int
+		expectHeaderLocation string
+		expectBodyStartsWith string
+	}{
+		{
+			name:                 "ok",
+			givenPrefix:          "/images",
+			givenRoot:            "../_fixture/images",
+			whenURL:              "/group/images/walle.png",
+			expectStatus:         http.StatusOK,
+			expectBodyStartsWith: string([]byte{0x89, 0x50, 0x4e, 0x47}),
+		},
+		{
+			name:                 "No file",
+			givenPrefix:          "/images",
+			givenRoot:            "../_fixture/scripts",
+			whenURL:              "/group/images/bolt.png",
+			expectStatus:         http.StatusNotFound,
+			expectBodyStartsWith: "{\"message\":\"Not Found\"}\n",
+		},
+		{
+			name:                 "Directory not found (no trailing slash)",
+			givenPrefix:          "/images",
+			givenRoot:            "../_fixture/images",
+			whenURL:              "/group/images/",
+			expectStatus:         http.StatusNotFound,
+			expectBodyStartsWith: "{\"message\":\"Not Found\"}\n",
+		},
+		{
+			name:                 "Directory redirect",
+			givenPrefix:          "/",
+			givenRoot:            "../_fixture",
+			whenURL:              "/group/folder",
+			expectStatus:         http.StatusMovedPermanently,
+			expectHeaderLocation: "/group/folder/",
+			expectBodyStartsWith: "",
+		},
+		{
+			name:                 "Prefixed directory 404 (request URL without slash)",
+			givenGroup:           "_fixture",
+			givenPrefix:          "/folder/", // trailing slash will intentionally not match "/folder"
+			givenRoot:            "../_fixture",
+			whenURL:              "/_fixture/folder", // no trailing slash
+			expectStatus:         http.StatusNotFound,
+			expectBodyStartsWith: "{\"message\":\"Not Found\"}\n",
+		},
+		{
+			name:                 "Prefixed directory redirect (without slash redirect to slash)",
+			givenGroup:           "_fixture",
+			givenPrefix:          "/folder", // no trailing slash shall match /folder and /folder/*
+			givenRoot:            "../_fixture",
+			whenURL:              "/_fixture/folder", // no trailing slash
+			expectStatus:         http.StatusMovedPermanently,
+			expectHeaderLocation: "/_fixture/folder/",
+			expectBodyStartsWith: "",
+		},
+		{
+			name:                 "Directory with index.html",
+			givenPrefix:          "/",
+			givenRoot:            "../_fixture",
+			whenURL:              "/group/",
+			expectStatus:         http.StatusOK,
+			expectBodyStartsWith: "<!doctype html>",
+		},
+		{
+			name:                 "Prefixed directory with index.html (prefix ending with slash)",
+			givenPrefix:          "/assets/",
+			givenRoot:            "../_fixture",
+			whenURL:              "/group/assets/",
+			expectStatus:         http.StatusOK,
+			expectBodyStartsWith: "<!doctype html>",
+		},
+		{
+			name:                 "Prefixed directory with index.html (prefix ending without slash)",
+			givenPrefix:          "/assets",
+			givenRoot:            "../_fixture",
+			whenURL:              "/group/assets/",
+			expectStatus:         http.StatusOK,
+			expectBodyStartsWith: "<!doctype html>",
+		},
+		{
+			name:                 "Sub-directory with index.html",
+			givenPrefix:          "/",
+			givenRoot:            "../_fixture",
+			whenURL:              "/group/folder/",
+			expectStatus:         http.StatusOK,
+			expectBodyStartsWith: "<!doctype html>",
+		},
+		{
+			name:                 "do not allow directory traversal (backslash - windows separator)",
+			givenPrefix:          "/",
+			givenRoot:            "../_fixture/",
+			whenURL:              `/group/..\\middleware/basic_auth.go`,
+			expectStatus:         http.StatusNotFound,
+			expectBodyStartsWith: "{\"message\":\"Not Found\"}\n",
+		},
+		{
+			name:                 "do not allow directory traversal (slash - unix separator)",
+			givenPrefix:          "/",
+			givenRoot:            "../_fixture/",
+			whenURL:              `/group/../middleware/basic_auth.go`,
+			expectStatus:         http.StatusNotFound,
+			expectBodyStartsWith: "{\"message\":\"Not Found\"}\n",
+		},
+	}
+
+	for _, tc := range testCases {
+		t.Run(tc.name, func(t *testing.T) {
+			e := echo.New()
+			group := "/group"
+			if tc.givenGroup != "" {
+				group = tc.givenGroup
+			}
+			g := e.Group(group)
+			g.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.Header().Get(echo.HeaderLocation))
+			} else {
+				_, ok := rec.Result().Header[echo.HeaderLocation]
+				assert.False(t, ok)
+			}
+		})
+	}
+}