Fix CORS header duplication in chained proxies

[BiosSystem]

Fixes #2853.

When Echo CORS middleware is run in a chained proxy setup (or in front of any handler copying upstream headers using Add), headers like Access-Control-Allow-Origin and Vary get duplicated in the response.

Changes:

• Run simple request CORS header writing inside a Before hook on the response. This allows the proxy's CORS config to cleanly Set the headers, overriding duplicated upstream headers from the proxy or downstream response copy.
• Implement addVaryHeader helper to merge and deduplicate Vary values case-insensitively.
• Add unit test simulating ReverseProxy behavior to verify headers are not duplicated.

[aldas]

I do not think this is a good idea. I do not think this is sustainable for middlewares to start adding extra logic so they work potentially with some other middleware. and res.Before(func() { is such dark magic of Echo and really hard to debug that I really do not want to accept it for maintainability reasons.

Are we lacking mechnisms (callbacks/handler funcs etc) at the moment in middlewares to allow for you to plug-in these work-a-rounds at your app/repo?

[Copilot]

Pull request overview

This PR fixes duplicated CORS-related response headers (notably Access-Control-Allow-Origin and Vary) when Echo’s CORS middleware is used in chained proxy scenarios where upstream headers are copied with Header.Add.

Changes:

• Moves simple-request CORS header writing into Response.Before(...) hooks so the middleware can Set/dedupe headers after an upstream/proxy handler has Added them.
• Adds an addVaryHeader helper to merge and deduplicate Vary values case-insensitively.
• Adds a unit test that simulates reverse-proxy-style header copying to ensure CORS headers are not duplicated.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.

File	Description
middleware/cors.go	Defers simple-request CORS header writes via Response.Before and introduces addVaryHeader for deduping Vary.
middleware/cors_test.go	Adds a regression test simulating proxy header copying to verify no duplication of CORS/Vary headers.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[aldas]

As I said I am not in favor of this PR, especially because of Response.Before usage.

just for information - there is similar PR agains v4 #2983

[aldas]

@vishr , could yuo look at this PR. I do not think CORS middleware which is meant to guard application should, by default, have workarounds when it works as "man-in-the-middle" or less dramatic way of saying - proxy.

And could someone explain me - is this duplicate situation

Vary: Origin
Vary: Origin

problem that every application with this CORS middleware needs to spend cycles on - most of the time you DO NOT deal with proxied responses

why this could not be the solution: we wrap CORS middleware and conditionally work with the response headers after handlerchain (proxy) has returned with the answer. but this could be done before if needed.

package main

import (
	"log/slog"
	"net/http"
	"strings"

	"github.com/labstack/echo/v5"
	"github.com/labstack/echo/v5/middleware"
)

func main() {
	e := echo.New()

	e.Use(func(next echo.HandlerFunc) echo.HandlerFunc {
		cors := middleware.CORS("http://localhost")(next)
		return func(c *echo.Context) error {
			err := cors(c)
			// after handler/proxy has returned, add Vary header
			// or deduplicate etc
			addVaryHeader(c.Response().Header(), echo.HeaderOrigin)
			return err
		}
	})

	e.GET("/", func(c *echo.Context) error {
		c.Response().Header().Set(echo.HeaderVary, echo.HeaderOrigin) // emulate proxied application
		return c.String(http.StatusOK, "Hello, World!")
	})

	if err := e.Start(":8080"); err != nil {
		slog.Error("Failed to start server", "error", err)
	}
}

func addVaryHeader(h http.Header, value string) {
	if h.Get(echo.HeaderVary) == "" {
		h.Set(echo.HeaderVary, value)
		return
	}
	for _, v := range h.Values(echo.HeaderVary) {
		for _, part := range strings.Split(v, ",") {
			if strings.EqualFold(strings.TrimSpace(part), value) {
				return
			}
		}
	}
	h.Add(echo.HeaderVary, value)
}

[aldas]

@BiosSystem would this middleware wrapper this be workable workaround for you

	e.Use(func(next echo.HandlerFunc) echo.HandlerFunc {
		cors := middleware.CORS("http://localhost")(next)
		return func(c *echo.Context) error {
			err := cors(c)
			// after handler/proxy has returned, add Vary header or deduplicate etc
			//addVaryHeader(c.Response().Header(), echo.HeaderOrigin)
			return err
		}
	})

[vishr]

Thanks for digging into this and iterating on the PR, @BiosSystem — the analysis in #2853 is solid and the problem is real.

We've decided not to solve it inside the CORS middleware, though. The duplication comes from running CORS on more than one layer of a proxy chain, where a reverse proxy copies the upstream's CORS headers on top of the ones already set. Compensating for that requires wrapping the ResponseWriter and deduping in middleware whose job is to guard the application — adding permanent API surface and per-request work for everyone to undo a configuration choice. We'd rather keep the default lean and predictable. rs/cors reached the same conclusion for the identical request (rs/cors#45).

The fix belongs at the boundary:

• Own CORS at the edge only — internal upstreams behind a gateway generally shouldn't run CORS.
• Strip the upstream's CORS headers when proxying via httputil.ReverseProxy.ModifyResponse, if an upstream must keep CORS for standalone use.

Both are now documented under the CORS middleware page's "Behind a reverse proxy" section (labstack/echox#421), with a pointer added to the CORS godoc (#3027).

Closing this in favor of that guidance — thank you again for the report and the work here, it directly drove the docs. 🙏