diff --git a/middleware/cors.go b/middleware/cors.go index 96ed16985..d7e8d5446 100644 --- a/middleware/cors.go +++ b/middleware/cors.go @@ -193,7 +193,25 @@ func (config CORSConfig) ToMiddleware() (echo.MiddlewareFunc, error) { res := c.Response() origin := req.Header.Get(echo.HeaderOrigin) - res.Header().Add(echo.HeaderVary, echo.HeaderOrigin) + // Check if CORS headers already exist (e.g., from a proxied upstream service) + // to avoid duplication in proxy chains + if res.Header().Get(echo.HeaderAccessControlAllowOrigin) != "" { + // CORS headers already present, likely from upstream - skip processing + if preflight := req.Method == http.MethodOptions; preflight { + return c.NoContent(http.StatusNoContent) + } + return next(c) + } + + // Add Origin to Vary header only if not already present + varyHeader := res.Header().Get(echo.HeaderVary) + if !strings.Contains(varyHeader, echo.HeaderOrigin) { + if varyHeader == "" { + res.Header().Set(echo.HeaderVary, echo.HeaderOrigin) + } else { + res.Header().Set(echo.HeaderVary, varyHeader+", "+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 @@ -261,8 +279,20 @@ func (config CORSConfig) ToMiddleware() (echo.MiddlewareFunc, error) { // Preflight will end with c.NoContent(http.StatusNoContent) as we do not know if // at the end of handler chain is actual OPTIONS route or 404/405 route which // response code will confuse browsers - res.Header().Add(echo.HeaderVary, echo.HeaderAccessControlRequestMethod) - res.Header().Add(echo.HeaderVary, echo.HeaderAccessControlRequestHeaders) + + // Add to Vary header only if not already present + varyHeader = res.Header().Get(echo.HeaderVary) + varyValues := []string{echo.HeaderAccessControlRequestMethod, echo.HeaderAccessControlRequestHeaders} + for _, varyValue := range varyValues { + if !strings.Contains(varyHeader, varyValue) { + if varyHeader == "" { + varyHeader = varyValue + } else { + varyHeader = varyHeader + ", " + varyValue + } + } + } + res.Header().Set(echo.HeaderVary, varyHeader) if !hasCustomAllowMethods && routerAllowMethods != "" { res.Header().Set(echo.HeaderAccessControlAllowMethods, routerAllowMethods) diff --git a/middleware/cors_test.go b/middleware/cors_test.go index 5de4ca063..91034afa4 100644 --- a/middleware/cors_test.go +++ b/middleware/cors_test.go @@ -626,3 +626,83 @@ func Test_allowOriginFunc(t *testing.T) { } } } + +// TestCORSProxyChain tests that CORS headers are not duplicated when +// multiple CORS middlewares are chained in a proxy scenario +func TestCORSProxyChain(t *testing.T) { + e := echo.New() + req := httptest.NewRequest(http.MethodGet, "/", nil) + req.Header.Set(echo.HeaderOrigin, "http://example.com") + rec := httptest.NewRecorder() + c := e.NewContext(req, rec) + + // Simulate Service B (upstream) that already set CORS headers + upstreamHandler := func(c *echo.Context) error { + c.Response().Header().Set(echo.HeaderAccessControlAllowOrigin, "http://example.com") + c.Response().Header().Set(echo.HeaderVary, "Origin") + return c.String(http.StatusOK, "test") + } + + // Apply CORS middleware on Service A (proxy layer) + mw := CORS("*") + handler := mw(upstreamHandler) + + err := handler(c) + assert.NoError(t, err) + assert.Equal(t, http.StatusOK, rec.Code) + + // Verify headers are not duplicated + assert.Equal(t, "http://example.com", rec.Header().Get(echo.HeaderAccessControlAllowOrigin)) + + // Check that Vary header contains "Origin" only once + varyHeader := rec.Header().Get(echo.HeaderVary) + assert.Contains(t, varyHeader, "Origin") + + // Count occurrences of "Origin" in Vary header - should be exactly 1 + originCount := strings.Count(varyHeader, "Origin") + assert.Equal(t, 1, originCount, "Vary header should contain 'Origin' only once, got: %s", varyHeader) + + // Verify there's no duplicate Access-Control-Allow-Origin header + allowOriginHeaders := rec.Header().Values(echo.HeaderAccessControlAllowOrigin) + assert.Equal(t, 1, len(allowOriginHeaders), "Should have exactly one Access-Control-Allow-Origin header") +} + +// TestCORSProxyChainPreflight tests preflight requests in proxy chains +func TestCORSProxyChainPreflight(t *testing.T) { + e := echo.New() + req := httptest.NewRequest(http.MethodOptions, "/", nil) + req.Header.Set(echo.HeaderOrigin, "http://example.com") + req.Header.Set(echo.HeaderAccessControlRequestMethod, "POST") + rec := httptest.NewRecorder() + c := e.NewContext(req, rec) + + // Simulate Service B (upstream) that already set CORS headers + upstreamHandler := func(c *echo.Context) error { + c.Response().Header().Set(echo.HeaderAccessControlAllowOrigin, "*") + c.Response().Header().Set(echo.HeaderVary, "Origin, Access-Control-Request-Method, Access-Control-Request-Headers") + c.Response().Header().Set(echo.HeaderAccessControlAllowMethods, "GET,POST,PUT") + return c.NoContent(http.StatusNoContent) + } + + // Apply CORS middleware on Service A (proxy layer) + mw := CORS("*") + handler := mw(upstreamHandler) + + err := handler(c) + assert.NoError(t, err) + assert.Equal(t, http.StatusNoContent, rec.Code) + + // Verify headers are not duplicated + assert.Equal(t, "*", rec.Header().Get(echo.HeaderAccessControlAllowOrigin)) + + // Check that Vary header contains each value only once + varyHeader := rec.Header().Get(echo.HeaderVary) + assert.Contains(t, varyHeader, "Origin") + + originCount := strings.Count(varyHeader, "Origin") + assert.Equal(t, 1, originCount, "Vary header should contain 'Origin' only once, got: %s", varyHeader) + + // Verify there's no duplicate Access-Control-Allow-Origin header + allowOriginHeaders := rec.Header().Values(echo.HeaderAccessControlAllowOrigin) + assert.Equal(t, 1, len(allowOriginHeaders), "Should have exactly one Access-Control-Allow-Origin header") +}