Skip to content

Commit d3283c8

Browse files
committed
Fix CORS header duplication in proxy chains
When multiple Echo services with CORS middleware are chained (e.g., Service A proxies to Service B, both with middleware.CORS enabled), CORS headers were being duplicated in the response. This fix adds a check to detect existing Access-Control-Allow-Origin headers in the response (indicating the request was proxied from an upstream service that already applied CORS), and skips re-applying CORS headers in that case. Also updates Vary header handling to prevent duplication by checking if values already exist before adding them. Fixes #2853
1 parent fa5311b commit d3283c8

File tree

2 files changed

+113
-3
lines changed

2 files changed

+113
-3
lines changed

middleware/cors.go

Lines changed: 33 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -193,7 +193,25 @@ func (config CORSConfig) ToMiddleware() (echo.MiddlewareFunc, error) {
193193
res := c.Response()
194194
origin := req.Header.Get(echo.HeaderOrigin)
195195

196-
res.Header().Add(echo.HeaderVary, echo.HeaderOrigin)
196+
// Check if CORS headers already exist (e.g., from a proxied upstream service)
197+
// to avoid duplication in proxy chains
198+
if res.Header().Get(echo.HeaderAccessControlAllowOrigin) != "" {
199+
// CORS headers already present, likely from upstream - skip processing
200+
if preflight := req.Method == http.MethodOptions; preflight {
201+
return c.NoContent(http.StatusNoContent)
202+
}
203+
return next(c)
204+
}
205+
206+
// Add Origin to Vary header only if not already present
207+
varyHeader := res.Header().Get(echo.HeaderVary)
208+
if !strings.Contains(varyHeader, echo.HeaderOrigin) {
209+
if varyHeader == "" {
210+
res.Header().Set(echo.HeaderVary, echo.HeaderOrigin)
211+
} else {
212+
res.Header().Set(echo.HeaderVary, varyHeader+", "+echo.HeaderOrigin)
213+
}
214+
}
197215

198216
// Preflight request is an OPTIONS request, using three HTTP request headers: Access-Control-Request-Method,
199217
// 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) {
261279
// Preflight will end with c.NoContent(http.StatusNoContent) as we do not know if
262280
// at the end of handler chain is actual OPTIONS route or 404/405 route which
263281
// response code will confuse browsers
264-
res.Header().Add(echo.HeaderVary, echo.HeaderAccessControlRequestMethod)
265-
res.Header().Add(echo.HeaderVary, echo.HeaderAccessControlRequestHeaders)
282+
283+
// Add to Vary header only if not already present
284+
varyHeader = res.Header().Get(echo.HeaderVary)
285+
varyValues := []string{echo.HeaderAccessControlRequestMethod, echo.HeaderAccessControlRequestHeaders}
286+
for _, varyValue := range varyValues {
287+
if !strings.Contains(varyHeader, varyValue) {
288+
if varyHeader == "" {
289+
varyHeader = varyValue
290+
} else {
291+
varyHeader = varyHeader + ", " + varyValue
292+
}
293+
}
294+
}
295+
res.Header().Set(echo.HeaderVary, varyHeader)
266296

267297
if !hasCustomAllowMethods && routerAllowMethods != "" {
268298
res.Header().Set(echo.HeaderAccessControlAllowMethods, routerAllowMethods)

middleware/cors_test.go

Lines changed: 80 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -626,3 +626,83 @@ func Test_allowOriginFunc(t *testing.T) {
626626
}
627627
}
628628
}
629+
630+
// TestCORSProxyChain tests that CORS headers are not duplicated when
631+
// multiple CORS middlewares are chained in a proxy scenario
632+
func TestCORSProxyChain(t *testing.T) {
633+
e := echo.New()
634+
req := httptest.NewRequest(http.MethodGet, "/", nil)
635+
req.Header.Set(echo.HeaderOrigin, "http://example.com")
636+
rec := httptest.NewRecorder()
637+
c := e.NewContext(req, rec)
638+
639+
// Simulate Service B (upstream) that already set CORS headers
640+
upstreamHandler := func(c *echo.Context) error {
641+
c.Response().Header().Set(echo.HeaderAccessControlAllowOrigin, "http://example.com")
642+
c.Response().Header().Set(echo.HeaderVary, "Origin")
643+
return c.String(http.StatusOK, "test")
644+
}
645+
646+
// Apply CORS middleware on Service A (proxy layer)
647+
mw := CORS("*")
648+
handler := mw(upstreamHandler)
649+
650+
err := handler(c)
651+
assert.NoError(t, err)
652+
assert.Equal(t, http.StatusOK, rec.Code)
653+
654+
// Verify headers are not duplicated
655+
assert.Equal(t, "http://example.com", rec.Header().Get(echo.HeaderAccessControlAllowOrigin))
656+
657+
// Check that Vary header contains "Origin" only once
658+
varyHeader := rec.Header().Get(echo.HeaderVary)
659+
assert.Contains(t, varyHeader, "Origin")
660+
661+
// Count occurrences of "Origin" in Vary header - should be exactly 1
662+
originCount := strings.Count(varyHeader, "Origin")
663+
assert.Equal(t, 1, originCount, "Vary header should contain 'Origin' only once, got: %s", varyHeader)
664+
665+
// Verify there's no duplicate Access-Control-Allow-Origin header
666+
allowOriginHeaders := rec.Header().Values(echo.HeaderAccessControlAllowOrigin)
667+
assert.Equal(t, 1, len(allowOriginHeaders), "Should have exactly one Access-Control-Allow-Origin header")
668+
}
669+
670+
// TestCORSProxyChainPreflight tests preflight requests in proxy chains
671+
func TestCORSProxyChainPreflight(t *testing.T) {
672+
e := echo.New()
673+
req := httptest.NewRequest(http.MethodOptions, "/", nil)
674+
req.Header.Set(echo.HeaderOrigin, "http://example.com")
675+
req.Header.Set(echo.HeaderAccessControlRequestMethod, "POST")
676+
rec := httptest.NewRecorder()
677+
c := e.NewContext(req, rec)
678+
679+
// Simulate Service B (upstream) that already set CORS headers
680+
upstreamHandler := func(c *echo.Context) error {
681+
c.Response().Header().Set(echo.HeaderAccessControlAllowOrigin, "*")
682+
c.Response().Header().Set(echo.HeaderVary, "Origin, Access-Control-Request-Method, Access-Control-Request-Headers")
683+
c.Response().Header().Set(echo.HeaderAccessControlAllowMethods, "GET,POST,PUT")
684+
return c.NoContent(http.StatusNoContent)
685+
}
686+
687+
// Apply CORS middleware on Service A (proxy layer)
688+
mw := CORS("*")
689+
handler := mw(upstreamHandler)
690+
691+
err := handler(c)
692+
assert.NoError(t, err)
693+
assert.Equal(t, http.StatusNoContent, rec.Code)
694+
695+
// Verify headers are not duplicated
696+
assert.Equal(t, "*", rec.Header().Get(echo.HeaderAccessControlAllowOrigin))
697+
698+
// Check that Vary header contains each value only once
699+
varyHeader := rec.Header().Get(echo.HeaderVary)
700+
assert.Contains(t, varyHeader, "Origin")
701+
702+
originCount := strings.Count(varyHeader, "Origin")
703+
assert.Equal(t, 1, originCount, "Vary header should contain 'Origin' only once, got: %s", varyHeader)
704+
705+
// Verify there's no duplicate Access-Control-Allow-Origin header
706+
allowOriginHeaders := rec.Header().Values(echo.HeaderAccessControlAllowOrigin)
707+
assert.Equal(t, 1, len(allowOriginHeaders), "Should have exactly one Access-Control-Allow-Origin header")
708+
}

0 commit comments

Comments
 (0)