Repository: labstack/echo
Pull request #2490 (2023-07-21 06:55:16)

CSRF/RequestID mw: switch math/random usage to crypto/random
switch math/random usage to crypto/random

related to #2489

codecov[bot]: ## [Codecov](https://app.codecov.io/gh/labstack/echo/pull/2490?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=labstack) Report
Patch coverage: **`78.57`**% and project coverage change: **`-0.05`** :warning:
> Comparison is base [(`4598a4a`)](https://app.codecov.io/gh/labstack/echo/commit/4598a4a7458f69f2532ec825411967f6e82adfbd?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=labstack) 92.82% compared to head [(`cdd9635`)](https://app.codecov.io/gh/labstack/echo/pull/2490?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=labstack) 92.77%.

<details><summary>Additional details and impacted files</summary>


```diff
@@            Coverage Diff             @@
##           master    #2490      +/-   ##
==========================================
- Coverage   92.82%   92.77%   -0.05%     
==========================================
  Files          39       39              
  Lines        4624     4636      +12     
==========================================
+ Hits         4292     4301       +9     
- Misses        241      243       +2     
- Partials       91       92       +1     
```


| [Impacted Files](https://app.codecov.io/gh/labstack/echo/pull/2490?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=labstack) | Coverage Δ | |
|---|---|---|
| [middleware/util.go](https://app.codecov.io/gh/labstack/echo/pull/2490?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=labstack#diff-bWlkZGxld2FyZS91dGlsLmdv) | `87.03% <75.00%> (-3.44%)` | :arrow_down: |
| [middleware/csrf.go](https://app.codecov.io/gh/labstack/echo/pull/2490?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=labstack#diff-bWlkZGxld2FyZS9jc3JmLmdv) | `89.42% <100.00%> (ø)` | |
| [middleware/request\_id.go](https://app.codecov.io/gh/labstack/echo/pull/2490?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=labstack#diff-bWlkZGxld2FyZS9yZXF1ZXN0X2lkLmdv) | `83.87% <100.00%> (ø)` | |


</details>

[:umbrella: View full report in Codecov by Sentry](https://app.codecov.io/gh/labstack/echo/pull/2490?src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=labstack).   
:loudspeaker: Do you have feedback about the report comment? [Let us know in this issue](https://about.codecov.io/codecov-pr-comment-feedback/?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=labstack).


Patch:
diff --git a/middleware/body_limit.go b/middleware/body_limit.go
index b436bd5..99e3ac5 100644
--- a/middleware/body_limit.go
+++ b/middleware/body_limit.go
@@ -23,9 +23,8 @@ type (
 
 	limitedReader struct {
 		BodyLimitConfig
-		reader  io.ReadCloser
-		read    int64
-		context echo.Context
+		reader io.ReadCloser
+		read   int64
 	}
 )
 
@@ -80,7 +79,7 @@ func BodyLimitWithConfig(config BodyLimitConfig) echo.MiddlewareFunc {
 
 			// Based on content read
 			r := pool.Get().(*limitedReader)
-			r.Reset(req.Body, c)
+			r.Reset(req.Body)
 			defer pool.Put(r)
 			req.Body = r
 
@@ -102,9 +101,8 @@ func (r *limitedReader) Close() error {
 	return r.reader.Close()
 }
 
-func (r *limitedReader) Reset(reader io.ReadCloser, context echo.Context) {
+func (r *limitedReader) Reset(reader io.ReadCloser) {
 	r.reader = reader
-	r.context = context
 	r.read = 0
 }
 
diff --git a/middleware/csrf.go b/middleware/csrf.go
index 6899700..adf1221 100644
--- a/middleware/csrf.go
+++ b/middleware/csrf.go
@@ -6,7 +6,6 @@ import (
 	"time"
 
 	"github.com/labstack/echo/v4"
-	"github.com/labstack/gommon/random"
 )
 
 type (
@@ -103,6 +102,7 @@ func CSRFWithConfig(config CSRFConfig) echo.MiddlewareFunc {
 	if config.TokenLength == 0 {
 		config.TokenLength = DefaultCSRFConfig.TokenLength
 	}
+
 	if config.TokenLookup == "" {
 		config.TokenLookup = DefaultCSRFConfig.TokenLookup
 	}
@@ -132,7 +132,7 @@ func CSRFWithConfig(config CSRFConfig) echo.MiddlewareFunc {
 
 			token := ""
 			if k, err := c.Cookie(config.CookieName); err != nil {
-				token = random.String(config.TokenLength) // Generate token
+				token = randomString(config.TokenLength)
 			} else {
 				token = k.Value // Reuse token
 			}
diff --git a/middleware/request_id.go b/middleware/request_id.go
index 8c5ff66..e29c8f5 100644
--- a/middleware/request_id.go
+++ b/middleware/request_id.go
@@ -2,7 +2,6 @@ package middleware
 
 import (
 	"github.com/labstack/echo/v4"
-	"github.com/labstack/gommon/random"
 )
 
 type (
@@ -12,7 +11,7 @@ type (
 		Skipper Skipper
 
 		// Generator defines a function to generate an ID.
-		// Optional. Default value random.String(32).
+		// Optional. Defaults to generator for random string of length 32.
 		Generator func() string
 
 		// RequestIDHandler defines a function which is executed for a request id.
@@ -73,5 +72,5 @@ func RequestIDWithConfig(config RequestIDConfig) echo.MiddlewareFunc {
 }
 
 func generator() string {
-	return random.String(32)
+	return randomString(32)
 }
diff --git a/middleware/util.go b/middleware/util.go
index ab951a0..aa34d78 100644
--- a/middleware/util.go
+++ b/middleware/util.go
@@ -1,6 +1,8 @@
 package middleware
 
 import (
+	"crypto/rand"
+	"fmt"
 	"strings"
 )
 
@@ -52,3 +54,18 @@ func matchSubdomain(domain, pattern string) bool {
 	}
 	return false
 }
+
+func randomString(length uint8) string {
+	charset := "ABCDEFGHIJKLMNOPQRSTUVWXYZabcdefghijklmnopqrstuvwxyz"
+
+	bytes := make([]byte, length)
+	_, err := rand.Read(bytes)
+	if err != nil {
+		// we are out of random. let the request fail
+		panic(fmt.Errorf("echo randomString failed to read random bytes: %w", err))
+	}
+	for i, b := range bytes {
+		bytes[i] = charset[b%byte(len(charset))]
+	}
+	return string(bytes)
+}


Test patch:
diff --git a/middleware/body_limit_test.go b/middleware/body_limit_test.go
index 2bfce37..0fd66ee 100644
--- a/middleware/body_limit_test.go
+++ b/middleware/body_limit_test.go
@@ -56,9 +56,6 @@ func TestBodyLimit(t *testing.T) {
 
 func TestBodyLimitReader(t *testing.T) {
 	hw := []byte("Hello, World!")
-	e := echo.New()
-	req := httptest.NewRequest(http.MethodPost, "/", bytes.NewReader(hw))
-	rec := httptest.NewRecorder()
 
 	config := BodyLimitConfig{
 		Skipper: DefaultSkipper,
@@ -68,7 +65,6 @@ func TestBodyLimitReader(t *testing.T) {
 	reader := &limitedReader{
 		BodyLimitConfig: config,
 		reader:          io.NopCloser(bytes.NewReader(hw)),
-		context:         e.NewContext(req, rec),
 	}
 
 	// read all should return ErrStatusRequestEntityTooLarge
@@ -78,7 +74,7 @@ func TestBodyLimitReader(t *testing.T) {
 
 	// reset reader and read two bytes must succeed
 	bt := make([]byte, 2)
-	reader.Reset(io.NopCloser(bytes.NewReader(hw)), e.NewContext(req, rec))
+	reader.Reset(io.NopCloser(bytes.NewReader(hw)))
 	n, err := reader.Read(bt)
 	assert.Equal(t, 2, n)
 	assert.Equal(t, nil, err)
diff --git a/middleware/csrf_test.go b/middleware/csrf_test.go
index 6bccdbe..6b20297 100644
--- a/middleware/csrf_test.go
+++ b/middleware/csrf_test.go
@@ -8,7 +8,6 @@ import (
 	"testing"
 
 	"github.com/labstack/echo/v4"
-	"github.com/labstack/gommon/random"
 	"github.com/stretchr/testify/assert"
 )
 
@@ -233,7 +232,7 @@ func TestCSRF(t *testing.T) {
 	assert.Error(t, h(c))
 
 	// Valid CSRF token
-	token := random.String(32)
+	token := randomString(32)
 	req.Header.Set(echo.HeaderCookie, "_csrf="+token)
 	req.Header.Set(echo.HeaderXCSRFToken, token)
 	if assert.NoError(t, h(c)) {
diff --git a/middleware/rate_limiter_test.go b/middleware/rate_limiter_test.go
index 0f7c914..f66961f 100644
--- a/middleware/rate_limiter_test.go
+++ b/middleware/rate_limiter_test.go
@@ -10,7 +10,6 @@ import (
 	"time"
 
 	"github.com/labstack/echo/v4"
-	"github.com/labstack/gommon/random"
 	"github.com/stretchr/testify/assert"
 	"golang.org/x/time/rate"
 )
@@ -410,7 +409,7 @@ func TestNewRateLimiterMemoryStore(t *testing.T) {
 func generateAddressList(count int) []string {
 	addrs := make([]string, count)
 	for i := 0; i < count; i++ {
-		addrs[i] = random.String(15)
+		addrs[i] = randomString(15)
 	}
 	return addrs
 }
diff --git a/middleware/util_test.go b/middleware/util_test.go
index df1d262..7562d4a 100644
--- a/middleware/util_test.go
+++ b/middleware/util_test.go
@@ -93,3 +93,27 @@ func Test_matchSubdomain(t *testing.T) {
 		assert.Equal(t, v.expected, matchSubdomain(v.domain, v.pattern))
 	}
 }
+
+func TestRandomString(t *testing.T) {
+	var testCases = []struct {
+		name       string
+		whenLength uint8
+		expect     string
+	}{
+		{
+			name:       "ok, 16",
+			whenLength: 16,
+		},
+		{
+			name:       "ok, 32",
+			whenLength: 32,
+		},
+	}
+
+	for _, tc := range testCases {
+		t.Run(tc.name, func(t *testing.T) {
+			uid := randomString(tc.whenLength)
+			assert.Len(t, uid, int(tc.whenLength))
+		})
+	}
+}