fix(middleware): only short-circuit CORS preflights

Treat OPTIONS requests as CORS preflights only when Origin and Access-Control-Request-Method are present, so non-preflight OPTIONS handlers can run.

Fixes #2534

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Author: deepakganesh78

bind_test.go

@@ -1577,7 +1577,7 @@ func TestTimeFormatBinding(t *testing.T) {
 		DateTimeLocal time.Time  `form:"datetime_local" format:"2006-01-02T15:04"`
 		Date          time.Time  `query:"date" format:"2006-01-02"`
 		CustomFormat  time.Time  `form:"custom" format:"01/02/2006 15:04:05"`
-		DefaultTime   time.Time  `form:"default_time"`                      // No format tag - should use default parsing
+		DefaultTime   time.Time  `form:"default_time"` // No format tag - should use default parsing
 		PtrTime       *time.Time `query:"ptr_time" format:"2006-01-02"`
 	}
 
@@ -1623,7 +1623,7 @@ func TestTimeFormatBinding(t *testing.T) {
 		{
 			name:        "nok, wrong format should fail",
 			contentType: MIMEApplicationForm,
-			data:        "datetime_local=2023-12-25",  // Missing time part
+			data:        "datetime_local=2023-12-25", // Missing time part
 			expectError: true,
 		},
 	}

middleware/cors.go

@@ -190,18 +190,12 @@ func CORSWithConfig(config CORSConfig) echo.MiddlewareFunc {
 
 			res.Header().Add(echo.HeaderVary, echo.HeaderOrigin)
 
-			// Preflight request is an OPTIONS request, using three HTTP request headers: Access-Control-Request-Method,
-			// Access-Control-Request-Headers, and the Origin header. See: https://developer.mozilla.org/en-US/docs/Glossary/Preflight_request
-			// For simplicity we just consider method type and later `Origin` header.
-			preflight := req.Method == http.MethodOptions
-
-			// Although router adds special handler in case of OPTIONS method we avoid calling next for OPTIONS in this middleware
-			// as CORS requests do not have cookies / authentication headers by default, so we could get stuck in auth
-			// middlewares by calling next(c).
-			// But we still want to send `Allow` header as response in case of Non-CORS OPTIONS request as router default
-			// handler does.
+			preflight := isCORSPreflight(req)
+
+			// Echo's router adds an Allow header for OPTIONS requests. Copy it before true
+			// CORS preflight requests short-circuit the handler chain.
 			routerAllowMethods := ""
-			if preflight {
+			if req.Method == http.MethodOptions {
 				tmpAllowMethods, ok := c.Get(echo.ContextKeyHeaderAllow).(string)
 				if ok && tmpAllowMethods != "" {
 					routerAllowMethods = tmpAllowMethods
@@ -211,10 +205,7 @@ func CORSWithConfig(config CORSConfig) echo.MiddlewareFunc {
 
 			// No Origin provided. This is (probably) not request from actual browser - proceed executing middleware chain
 			if origin == "" {
-				if !preflight {
-					return next(c)
-				}
-				return c.NoContent(http.StatusNoContent)
+				return next(c)
 			}
 
 			if config.AllowOriginFunc != nil {
@@ -305,3 +296,9 @@ func CORSWithConfig(config CORSConfig) echo.MiddlewareFunc {
 		}
 	}
 }
+
+func isCORSPreflight(r *http.Request) bool {
+	return r.Method == http.MethodOptions &&
+		r.Header.Get(echo.HeaderOrigin) != "" &&
+		r.Header.Get(echo.HeaderAccessControlRequestMethod) != ""
+}

middleware/cors_test.go

@@ -246,6 +246,10 @@ func TestCORS(t *testing.T) {
 			for k, v := range tc.whenHeaders {
 				req.Header.Set(k, v)
 			}
+			if method == http.MethodOptions && req.Header.Get(echo.HeaderOrigin) != "" &&
+				req.Header.Get(echo.HeaderAccessControlRequestMethod) == "" {
+				req.Header.Set(echo.HeaderAccessControlRequestMethod, http.MethodGet)
+			}
 
 			err := h(c)
 
@@ -265,6 +269,70 @@ func TestCORS(t *testing.T) {
 	}
 }
 
+func TestCORS_NonPreflightOPTIONSPassThrough(t *testing.T) {
+	e := echo.New()
+	cors := CORSWithConfig(CORSConfig{
+		AllowOrigins: []string{"*"},
+		AllowMethods: []string{http.MethodPut},
+	})
+
+	for _, tc := range []struct {
+		name    string
+		headers map[string]string
+	}{
+		{
+			name: "OPTIONS without Origin",
+		},
+		{
+			name: "OPTIONS with Origin but without Access-Control-Request-Method",
+			headers: map[string]string{
+				echo.HeaderOrigin: "https://example.com",
+			},
+		},
+	} {
+		t.Run(tc.name, func(t *testing.T) {
+			req := httptest.NewRequest(http.MethodOptions, "/hello", nil)
+			for k, v := range tc.headers {
+				req.Header.Set(k, v)
+			}
+			rec := httptest.NewRecorder()
+			c := e.NewContext(req, rec)
+			called := false
+
+			h := cors(func(c echo.Context) error {
+				called = true
+				c.Response().Header().Set(echo.HeaderAllow, "GET, OPTIONS")
+				return c.NoContent(http.StatusNoContent)
+			})
+
+			assert.NoError(t, h(c))
+			assert.True(t, called)
+			assert.Equal(t, "GET, OPTIONS", rec.Header().Get(echo.HeaderAllow))
+			assert.Empty(t, rec.Header().Get(echo.HeaderAccessControlAllowMethods))
+		})
+	}
+
+	t.Run("true preflight short-circuits next", func(t *testing.T) {
+		req := httptest.NewRequest(http.MethodOptions, "/hello", nil)
+		req.Header.Set(echo.HeaderOrigin, "https://example.com")
+		req.Header.Set(echo.HeaderAccessControlRequestMethod, http.MethodPut)
+		rec := httptest.NewRecorder()
+		c := e.NewContext(req, rec)
+		called := false
+
+		h := cors(func(c echo.Context) error {
+			called = true
+			return c.NoContent(http.StatusOK)
+		})
+
+		assert.NoError(t, h(c))
+		assert.False(t, called)
+		assert.Equal(t, http.StatusNoContent, rec.Code)
+		assert.Equal(t, "*", rec.Header().Get(echo.HeaderAccessControlAllowOrigin))
+		assert.Equal(t, "PUT", rec.Header().Get(echo.HeaderAccessControlAllowMethods))
+	})
+}
+
 func Test_allowOriginScheme(t *testing.T) {
 	tests := []struct {
 		domain, pattern string
@@ -298,6 +366,7 @@ func Test_allowOriginScheme(t *testing.T) {
 		rec := httptest.NewRecorder()
 		c := e.NewContext(req, rec)
 		req.Header.Set(echo.HeaderOrigin, tt.domain)
+		req.Header.Set(echo.HeaderAccessControlRequestMethod, http.MethodGet)
 		cors := CORSWithConfig(CORSConfig{
 			AllowOrigins: []string{tt.pattern},
 		})
@@ -389,6 +458,7 @@ func Test_allowOriginSubdomain(t *testing.T) {
 		rec := httptest.NewRecorder()
 		c := e.NewContext(req, rec)
 		req.Header.Set(echo.HeaderOrigin, tt.domain)
+		req.Header.Set(echo.HeaderAccessControlRequestMethod, http.MethodGet)
 		cors := CORSWithConfig(CORSConfig{
 			AllowOrigins: []string{tt.pattern},
 		})
@@ -472,6 +542,9 @@ func TestCORSWithConfig_AllowMethods(t *testing.T) {
 			c := e.NewContext(req, rec)
 
 			req.Header.Set(echo.HeaderOrigin, tc.whenOrigin)
+			if tc.whenOrigin != "" {
+				req.Header.Set(echo.HeaderAccessControlRequestMethod, http.MethodGet)
+			}
 			if tc.allowContextKey != "" {
 				c.Set(echo.ContextKeyHeaderAllow, tc.allowContextKey)
 			}
@@ -605,6 +678,9 @@ func TestCorsHeaders(t *testing.T) {
 			if tc.originDomain != "" {
 				req.Header.Set(echo.HeaderOrigin, tc.originDomain)
 			}
+			if tc.method == http.MethodOptions && tc.originDomain != "" {
+				req.Header.Set(echo.HeaderAccessControlRequestMethod, http.MethodGet)
+			}
 
 			// we run through whole Echo handler chain to see how CORS works with Router OPTIONS handler
 			e.ServeHTTP(rec, req)
@@ -663,6 +739,7 @@ func Test_allowOriginFunc(t *testing.T) {
 		rec := httptest.NewRecorder()
 		c := e.NewContext(req, rec)
 		req.Header.Set(echo.HeaderOrigin, origin)
+		req.Header.Set(echo.HeaderAccessControlRequestMethod, http.MethodGet)
 		cors := CORSWithConfig(CORSConfig{
 			AllowOriginFunc: allowOriginFunc,
 		})