Simplify code of Add/Remove trailing slash and fix bug (#1275)

* Simplify code of Add/Remove trailing slash

- simplify code (more informative / understanding)
- assert collides with imported package name (in tests)
- fix unhandled errors

* add tests for #1275 (comment)

Author: im-kulikov

middleware/slash.go

@@ -1,6 +1,8 @@
 package middleware
 
 import (
+	"strings"
+
 	"github.com/labstack/echo/v4"
 )
 
@@ -49,7 +51,7 @@ func AddTrailingSlashWithConfig(config TrailingSlashConfig) echo.MiddlewareFunc
 			url := req.URL
 			path := url.Path
 			qs := c.QueryString()
-			if path != "/" && path[len(path)-1] != '/' {
+			if !strings.HasSuffix(path, "/") {
 				path += "/"
 				uri := path
 				if qs != "" {
@@ -97,7 +99,7 @@ func RemoveTrailingSlashWithConfig(config TrailingSlashConfig) echo.MiddlewareFu
 			path := url.Path
 			qs := c.QueryString()
 			l := len(path) - 1
-			if l >= 0 && path != "/" && path[l] == '/' {
+			if l > 0 && strings.HasSuffix(path, "/") {
 				path = path[:l]
 				uri := path
 				if qs != "" {

middleware/slash_test.go

@@ -10,18 +10,28 @@ import (
 )
 
 func TestAddTrailingSlash(t *testing.T) {
+	is := assert.New(t)
 	e := echo.New()
 	req := httptest.NewRequest(http.MethodGet, "/add-slash", nil)
 	rec := httptest.NewRecorder()
 	c := e.NewContext(req, rec)
 	h := AddTrailingSlash()(func(c echo.Context) error {
 		return nil
 	})
-	h(c)
+	is.NoError(h(c))
+	is.Equal("/add-slash/", req.URL.Path)
+	is.Equal("/add-slash/", req.RequestURI)
 
-	assert := assert.New(t)
-	assert.Equal("/add-slash/", req.URL.Path)
-	assert.Equal("/add-slash/", req.RequestURI)
+	// Method Connect must not fail:
+	req = httptest.NewRequest(http.MethodConnect, "", nil)
+	rec = httptest.NewRecorder()
+	c = e.NewContext(req, rec)
+	h = AddTrailingSlash()(func(c echo.Context) error {
+		return nil
+	})
+	is.NoError(h(c))
+	is.Equal("/", req.URL.Path)
+	is.Equal("/", req.RequestURI)
 
 	// With config
 	req = httptest.NewRequest(http.MethodGet, "/add-slash?key=value", nil)
@@ -32,25 +42,34 @@ func TestAddTrailingSlash(t *testing.T) {
 	})(func(c echo.Context) error {
 		return nil
 	})
-	h(c)
-	assert.Equal(http.StatusMovedPermanently, rec.Code)
-	assert.Equal("/add-slash/?key=value", rec.Header().Get(echo.HeaderLocation))
+	is.NoError(h(c))
+	is.Equal(http.StatusMovedPermanently, rec.Code)
+	is.Equal("/add-slash/?key=value", rec.Header().Get(echo.HeaderLocation))
 }
 
 func TestRemoveTrailingSlash(t *testing.T) {
+	is := assert.New(t)
 	e := echo.New()
 	req := httptest.NewRequest(http.MethodGet, "/remove-slash/", nil)
 	rec := httptest.NewRecorder()
 	c := e.NewContext(req, rec)
 	h := RemoveTrailingSlash()(func(c echo.Context) error {
 		return nil
 	})
-	h(c)
-
-	assert := assert.New(t)
+	is.NoError(h(c))
+	is.Equal("/remove-slash", req.URL.Path)
+	is.Equal("/remove-slash", req.RequestURI)
 
-	assert.Equal("/remove-slash", req.URL.Path)
-	assert.Equal("/remove-slash", req.RequestURI)
+	// Method Connect must not fail:
+	req = httptest.NewRequest(http.MethodConnect, "", nil)
+	rec = httptest.NewRecorder()
+	c = e.NewContext(req, rec)
+	h = RemoveTrailingSlash()(func(c echo.Context) error {
+		return nil
+	})
+	is.NoError(h(c))
+	is.Equal("", req.URL.Path)
+	is.Equal("", req.RequestURI)
 
 	// With config
 	req = httptest.NewRequest(http.MethodGet, "/remove-slash/?key=value", nil)
@@ -61,9 +80,9 @@ func TestRemoveTrailingSlash(t *testing.T) {
 	})(func(c echo.Context) error {
 		return nil
 	})
-	h(c)
-	assert.Equal(http.StatusMovedPermanently, rec.Code)
-	assert.Equal("/remove-slash?key=value", rec.Header().Get(echo.HeaderLocation))
+	is.NoError(h(c))
+	is.Equal(http.StatusMovedPermanently, rec.Code)
+	is.Equal("/remove-slash?key=value", rec.Header().Get(echo.HeaderLocation))
 
 	// With bare URL
 	req = httptest.NewRequest(http.MethodGet, "http://localhost", nil)
@@ -72,6 +91,6 @@ func TestRemoveTrailingSlash(t *testing.T) {
 	h = RemoveTrailingSlash()(func(c echo.Context) error {
 		return nil
 	})
-	h(c)
-	assert.Equal("", req.URL.Path)
+	is.NoError(h(c))
+	is.Equal("", req.URL.Path)
 }