Repository: labstack/echo
Pull request #2136 (2022-03-15 19:02:21)

fix CSRF middleware not being able to extract token from `multipart/form-data` form
Fix for #2135 CSRF middleware not being able to extract token from `multipart/form-data` form

This can be tested by hand:
```go
func main() {
	e := echo.New()
	e.Use(middleware.Logger())
	e.Use(middleware.Recover())

	e.Use(middleware.CSRFWithConfig(middleware.CSRFConfig{
		TokenLookup: "form:csrf",
	}))

	e.POST("/form", func(c echo.Context) error {
		csrf, ok := c.Get("csrf").(string)
		if !ok {
			return echo.NewHTTPError(http.StatusBadRequest, "missing CSRF value")
		}
		return c.String(http.StatusCreated, csrf)
	})

	if err := e.Start(":8080"); err != http.ErrServerClosed {
		log.Fatal(err)
	}
}
```


```bash
echo "<div>hi</div>" > test.html

curl --trace-ascii /dev/stdout \
    --cookie "_csrf=test" \
    -F csrf=test \
    -F upload=@test.html \
    http://localhost:8080/form
```

output of curl after fix
```bash
x@x:~/code/echo/main$ curl --trace-ascii /dev/stdout --cookie "_csrf=test" -F csrf=test -F upload=@test.html http://localhost:8080/form
== Info:   Trying 127.0.0.1:8080...
== Info: Connected to localhost (127.0.0.1) port 8080 (#0)
=> Send header, 210 bytes (0xd2)
0000: POST /form HTTP/1.1
0015: Host: localhost:8080
002b: User-Agent: curl/7.74.0
0044: Accept: */*
0051: Cookie: _csrf=test
0065: Content-Length: 299
007a: Content-Type: multipart/form-data; boundary=--------------------
00ba: ----a297ba70c335a0d7
00d0: 
=> Send data, 299 bytes (0x12b)
0000: --------------------------a297ba70c335a0d7
002c: Content-Disposition: form-data; name="csrf"
0059: 
005b: test
0061: --------------------------a297ba70c335a0d7
008d: Content-Disposition: form-data; name="upload"; filename="test.ht
00cd: ml"
00d2: Content-Type: text/html
00eb: 
00ed: <div>hi</div>.
00fd: --------------------------a297ba70c335a0d7--
== Info: We are completely uploaded and fine
== Info: Mark bundle as not supporting multiuse
<= Recv header, 22 bytes (0x16)
0000: HTTP/1.1 201 Created
<= Recv header, 41 bytes (0x29)
0000: Content-Type: text/plain; charset=UTF-8
<= Recv header, 63 bytes (0x3f)
0000: Set-Cookie: _csrf=test; Expires=Wed, 16 Mar 2022 19:00:31 GMT
<= Recv header, 14 bytes (0xe)
0000: Vary: Cookie
<= Recv header, 37 bytes (0x25)
0000: Date: Tue, 15 Mar 2022 19:00:31 GMT
<= Recv header, 19 bytes (0x13)
0000: Content-Length: 4
<= Recv header, 2 bytes (0x2)
0000: 
<= Recv data, 4 bytes (0x4)
0000: test
== Info: Connection #0 to host localhost left intact
test
```

codecov[bot]: # [Codecov](https://codecov.io/gh/labstack/echo/pull/2136?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=labstack) Report
> Merging [#2136](https://codecov.io/gh/labstack/echo/pull/2136?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=labstack) (283d1ac) into [master](https://codecov.io/gh/labstack/echo/commit/b445958c3ce4cf34997a67ef73a30cd870170945?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=labstack) (b445958) will **not change** coverage.
> The diff coverage is `100.00%`.

```diff
@@           Coverage Diff           @@
##           master    #2136   +/-   ##
=======================================
  Coverage   92.10%   92.10%           
=======================================
  Files          37       37           
  Lines        3028     3028           
=======================================
  Hits         2789     2789           
  Misses        150      150           
  Partials       89       89           
```


| [Impacted Files](https://codecov.io/gh/labstack/echo/pull/2136?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=labstack) | Coverage Δ | |
|---|---|---|
| [middleware/extractor.go](https://codecov.io/gh/labstack/echo/pull/2136/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=labstack#diff-bWlkZGxld2FyZS9leHRyYWN0b3IuZ28=) | `98.83% <100.00%> (ø)` | |

------

[Continue to review full report at Codecov](https://codecov.io/gh/labstack/echo/pull/2136?src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=labstack).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=labstack)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/labstack/echo/pull/2136?src=pr&el=footer&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=labstack). Last update [b445958...283d1ac](https://codecov.io/gh/labstack/echo/pull/2136?src=pr&el=lastupdated&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=labstack). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=labstack).

aldas: @lammel please review

Patch:
diff --git a/echo.go b/echo.go
index 5b30872..c2f23c1 100644
--- a/echo.go
+++ b/echo.go
@@ -732,7 +732,7 @@ func (e *Echo) StartServer(s *http.Server) (err error) {
 	return s.Serve(e.Listener)
 }
 
-func (e *Echo) configureServer(s *http.Server) (err error) {
+func (e *Echo) configureServer(s *http.Server) error {
 	// Setup
 	e.colorer.SetOutput(e.Logger.Output())
 	s.ErrorLog = e.StdLogger
@@ -747,10 +747,11 @@ func (e *Echo) configureServer(s *http.Server) (err error) {
 
 	if s.TLSConfig == nil {
 		if e.Listener == nil {
-			e.Listener, err = newListener(s.Addr, e.ListenerNetwork)
+			l, err := newListener(s.Addr, e.ListenerNetwork)
 			if err != nil {
 				return err
 			}
+			e.Listener = l
 		}
 		if !e.HidePort {
 			e.colorer.Printf("⇨ http server started on %s\n", e.colorer.Green(e.Listener.Addr()))
@@ -791,7 +792,7 @@ func (e *Echo) TLSListenerAddr() net.Addr {
 }
 
 // StartH2CServer starts a custom http/2 server with h2c (HTTP/2 Cleartext).
-func (e *Echo) StartH2CServer(address string, h2s *http2.Server) (err error) {
+func (e *Echo) StartH2CServer(address string, h2s *http2.Server) error {
 	e.startupMutex.Lock()
 	// Setup
 	s := e.Server
@@ -808,11 +809,12 @@ func (e *Echo) StartH2CServer(address string, h2s *http2.Server) (err error) {
 	}
 
 	if e.Listener == nil {
-		e.Listener, err = newListener(s.Addr, e.ListenerNetwork)
+		l, err := newListener(s.Addr, e.ListenerNetwork)
 		if err != nil {
 			e.startupMutex.Unlock()
 			return err
 		}
+		e.Listener = l
 	}
 	if !e.HidePort {
 		e.colorer.Printf("⇨ http server started on %s\n", e.colorer.Green(e.Listener.Addr()))
diff --git a/middleware/extractor.go b/middleware/extractor.go
index a57ed4e..afdfd81 100644
--- a/middleware/extractor.go
+++ b/middleware/extractor.go
@@ -168,8 +168,8 @@ func valuesFromCookie(name string) ValuesExtractor {
 // valuesFromForm returns a function that extracts values from the form field.
 func valuesFromForm(name string) ValuesExtractor {
 	return func(c echo.Context) ([]string, error) {
-		if parseErr := c.Request().ParseForm(); parseErr != nil {
-			return nil, fmt.Errorf("valuesFromForm parse form failed: %w", parseErr)
+		if c.Request().Form == nil {
+			_ = c.Request().ParseMultipartForm(32 << 20) // same what `c.Request().FormValue(name)` does
 		}
 		values := c.Request().Form[name]
 		if len(values) == 0 {
diff --git a/middleware/recover.go b/middleware/recover.go
index a621a9e..7b61285 100644
--- a/middleware/recover.go
+++ b/middleware/recover.go
@@ -2,6 +2,7 @@ package middleware
 
 import (
 	"fmt"
+	"net/http"
 	"runtime"
 
 	"github.com/labstack/echo/v4"
@@ -77,6 +78,9 @@ func RecoverWithConfig(config RecoverConfig) echo.MiddlewareFunc {
 
 			defer func() {
 				if r := recover(); r != nil {
+					if r == http.ErrAbortHandler {
+						panic(r)
+					}
 					err, ok := r.(error)
 					if !ok {
 						err = fmt.Errorf("%v", r)


Test patch:
diff --git a/middleware/extractor_test.go b/middleware/extractor_test.go
index ae4b30a..2e898f5 100644
--- a/middleware/extractor_test.go
+++ b/middleware/extractor_test.go
@@ -1,9 +1,11 @@
 package middleware
 
 import (
+	"bytes"
 	"fmt"
 	"github.com/labstack/echo/v4"
 	"github.com/stretchr/testify/assert"
+	"mime/multipart"
 	"net/http"
 	"net/http/httptest"
 	"net/url"
@@ -499,6 +501,25 @@ func TestValuesFromForm(t *testing.T) {
 		return req
 	}
 
+	exampleMultiPartFormRequest := func(mod func(w *multipart.Writer)) *http.Request {
+		var b bytes.Buffer
+		w := multipart.NewWriter(&b)
+		w.WriteField("name", "Jon Snow")
+		w.WriteField("emails[]", "jon@labstack.com")
+		if mod != nil {
+			mod(w)
+		}
+
+		fw, _ := w.CreateFormFile("upload", "my.file")
+		fw.Write([]byte(`<div>hi</div>`))
+		w.Close()
+
+		req := httptest.NewRequest(http.MethodPost, "/", strings.NewReader(b.String()))
+		req.Header.Add(echo.HeaderContentType, w.FormDataContentType())
+
+		return req
+	}
+
 	var testCases = []struct {
 		name         string
 		givenRequest *http.Request
@@ -520,6 +541,14 @@ func TestValuesFromForm(t *testing.T) {
 			whenName:     "emails[]",
 			expectValues: []string{"jon@labstack.com", "snow@labstack.com"},
 		},
+		{
+			name: "ok, POST multipart/form, multiple value",
+			givenRequest: exampleMultiPartFormRequest(func(w *multipart.Writer) {
+				w.WriteField("emails[]", "snow@labstack.com")
+			}),
+			whenName:     "emails[]",
+			expectValues: []string{"jon@labstack.com", "snow@labstack.com"},
+		},
 		{
 			name:         "ok, GET form, single value",
 			givenRequest: exampleGetFormRequest(nil),
@@ -540,16 +569,6 @@ func TestValuesFromForm(t *testing.T) {
 			whenName:     "nope",
 			expectError:  errFormExtractorValueMissing.Error(),
 		},
-		{
-			name: "nok, POST form, form parsing error",
-			givenRequest: func() *http.Request {
-				req := httptest.NewRequest(http.MethodPost, "/", nil)
-				req.Body = nil
-				return req
-			}(),
-			whenName:    "name",
-			expectError: "valuesFromForm parse form failed: missing form body",
-		},
 		{
 			name: "ok, cut values over extractorLimit",
 			givenRequest: examplePostFormRequest(func(v *url.Values) {
diff --git a/middleware/recover_test.go b/middleware/recover_test.go
index 9ac4fee..b27f3b4 100644
--- a/middleware/recover_test.go
+++ b/middleware/recover_test.go
@@ -28,6 +28,35 @@ func TestRecover(t *testing.T) {
 	assert.Contains(t, buf.String(), "PANIC RECOVER")
 }
 
+func TestRecoverErrAbortHandler(t *testing.T) {
+	e := echo.New()
+	buf := new(bytes.Buffer)
+	e.Logger.SetOutput(buf)
+	req := httptest.NewRequest(http.MethodGet, "/", nil)
+	rec := httptest.NewRecorder()
+	c := e.NewContext(req, rec)
+	h := Recover()(echo.HandlerFunc(func(c echo.Context) error {
+		panic(http.ErrAbortHandler)
+	}))
+	defer func() {
+		r := recover()
+		if r == nil {
+			assert.Fail(t, "expecting `http.ErrAbortHandler`, got `nil`")
+		} else {
+			if err, ok := r.(error); ok {
+				assert.ErrorIs(t, err, http.ErrAbortHandler)
+			} else {
+				assert.Fail(t, "not of error type")
+			}
+		}
+	}()
+
+	h(c)
+
+	assert.Equal(t, http.StatusInternalServerError, rec.Code)
+	assert.NotContains(t, buf.String(), "PANIC RECOVER")
+}
+
 func TestRecoverWithConfig_LogLevel(t *testing.T) {
 	tests := []struct {
 		logLevel  log.Lvl