Skip to content

Commit e433dde

Browse files
committed
middleware/cors: fix header duplication in chained proxies using response writer wrapper
1 parent ad69ec5 commit e433dde

1 file changed

Lines changed: 107 additions & 31 deletions

File tree

middleware/cors.go

Lines changed: 107 additions & 31 deletions
Original file line numberDiff line numberDiff line change
@@ -189,10 +189,15 @@ func (config CORSConfig) ToMiddleware() (echo.MiddlewareFunc, error) {
189189
return next(c)
190190
}
191191

192+
// Add Vary: Origin unconditionally to all requests
193+
addVaryHeader(c.Response().Header(), echo.HeaderOrigin)
194+
192195
req := c.Request()
193-
res := c.Response()
194196
origin := req.Header.Get(echo.HeaderOrigin)
195197

198+
rw := &corsResponseWriter{ResponseWriter: c.Response()}
199+
c.SetResponse(rw)
200+
196201
// Preflight request is an OPTIONS request, using three HTTP request headers: Access-Control-Request-Method,
197202
// Access-Control-Request-Headers, and the Origin header. See: https://developer.mozilla.org/en-US/docs/Glossary/Preflight_request
198203
// For simplicity we just consider method type and later `Origin` header.
@@ -215,12 +220,8 @@ func (config CORSConfig) ToMiddleware() (echo.MiddlewareFunc, error) {
215220
// No Origin provided. This is (probably) not request from actual browser - proceed executing middleware chain
216221
if origin == "" {
217222
if preflight { // req.Method=OPTIONS
218-
addVaryHeader(res.Header(), echo.HeaderOrigin)
219223
return c.NoContent(http.StatusNoContent)
220224
}
221-
res.Before(func() {
222-
addVaryHeader(res.Header(), echo.HeaderOrigin)
223-
})
224225
return next(c) // let non-browser calls through
225226
}
226227

@@ -241,61 +242,54 @@ func (config CORSConfig) ToMiddleware() (echo.MiddlewareFunc, error) {
241242
// no CORS middleware should block non-preflight requests;
242243
// such requests should be let through. One reason is that not all requests that
243244
// carry an Origin header participate in the CORS protocol.
244-
res.Before(func() {
245-
addVaryHeader(res.Header(), echo.HeaderOrigin)
246-
})
247245
return next(c)
248246
}
249247

250248
// Origin existed and was allowed
251249

252250
// Simple request will be let though
253251
if !preflight {
254-
res.Before(func() {
255-
addVaryHeader(res.Header(), echo.HeaderOrigin)
256-
res.Header().Set(echo.HeaderAccessControlAllowOrigin, allowedOrigin)
257-
if config.AllowCredentials {
258-
res.Header().Set(echo.HeaderAccessControlAllowCredentials, "true")
259-
} else {
260-
res.Header().Del(echo.HeaderAccessControlAllowCredentials)
261-
}
262-
if exposeHeaders != "" {
263-
res.Header().Set(echo.HeaderAccessControlExposeHeaders, exposeHeaders)
264-
}
265-
})
252+
c.Response().Header().Set(echo.HeaderAccessControlAllowOrigin, allowedOrigin)
253+
if config.AllowCredentials {
254+
c.Response().Header().Set(echo.HeaderAccessControlAllowCredentials, "true")
255+
} else {
256+
c.Response().Header().Del(echo.HeaderAccessControlAllowCredentials)
257+
}
258+
if exposeHeaders != "" {
259+
c.Response().Header().Set(echo.HeaderAccessControlExposeHeaders, exposeHeaders)
260+
}
266261
return next(c)
267262
}
268263
// Below code is for Preflight (OPTIONS) request
269264
//
270265
// Preflight will end with c.NoContent(http.StatusNoContent) as we do not know if
271266
// at the end of handler chain is actual OPTIONS route or 404/405 route which
272267
// response code will confuse browsers
273-
addVaryHeader(res.Header(), echo.HeaderOrigin)
274-
res.Header().Set(echo.HeaderAccessControlAllowOrigin, allowedOrigin)
268+
c.Response().Header().Set(echo.HeaderAccessControlAllowOrigin, allowedOrigin)
275269
if config.AllowCredentials {
276-
res.Header().Set(echo.HeaderAccessControlAllowCredentials, "true")
270+
c.Response().Header().Set(echo.HeaderAccessControlAllowCredentials, "true")
277271
} else {
278-
res.Header().Del(echo.HeaderAccessControlAllowCredentials)
272+
c.Response().Header().Del(echo.HeaderAccessControlAllowCredentials)
279273
}
280-
addVaryHeader(res.Header(), echo.HeaderAccessControlRequestMethod)
281-
addVaryHeader(res.Header(), echo.HeaderAccessControlRequestHeaders)
274+
addVaryHeader(c.Response().Header(), echo.HeaderAccessControlRequestMethod)
275+
addVaryHeader(c.Response().Header(), echo.HeaderAccessControlRequestHeaders)
282276

283277
if !hasCustomAllowMethods && routerAllowMethods != "" {
284-
res.Header().Set(echo.HeaderAccessControlAllowMethods, routerAllowMethods)
278+
c.Response().Header().Set(echo.HeaderAccessControlAllowMethods, routerAllowMethods)
285279
} else {
286-
res.Header().Set(echo.HeaderAccessControlAllowMethods, allowMethods)
280+
c.Response().Header().Set(echo.HeaderAccessControlAllowMethods, allowMethods)
287281
}
288282

289283
if allowHeaders != "" {
290-
res.Header().Set(echo.HeaderAccessControlAllowHeaders, allowHeaders)
284+
c.Response().Header().Set(echo.HeaderAccessControlAllowHeaders, allowHeaders)
291285
} else {
292286
h := req.Header.Get(echo.HeaderAccessControlRequestHeaders)
293287
if h != "" {
294-
res.Header().Set(echo.HeaderAccessControlAllowHeaders, h)
288+
c.Response().Header().Set(echo.HeaderAccessControlAllowHeaders, h)
295289
}
296290
}
297291
if config.MaxAge != 0 {
298-
res.Header().Set(echo.HeaderAccessControlMaxAge, maxAge)
292+
c.Response().Header().Set(echo.HeaderAccessControlMaxAge, maxAge)
299293
}
300294
return c.NoContent(http.StatusNoContent)
301295
}
@@ -329,3 +323,85 @@ func addVaryHeader(h http.Header, value string) {
329323
}
330324
h.Add(echo.HeaderVary, value)
331325
}
326+
327+
type corsResponseWriter struct {
328+
http.ResponseWriter
329+
deduplicated bool
330+
}
331+
332+
func (w *corsResponseWriter) WriteHeader(statusCode int) {
333+
w.deduplicate()
334+
w.ResponseWriter.WriteHeader(statusCode)
335+
}
336+
337+
func (w *corsResponseWriter) Write(b []byte) (int, error) {
338+
w.deduplicate()
339+
return w.ResponseWriter.Write(b)
340+
}
341+
342+
func (w *corsResponseWriter) Unwrap() http.ResponseWriter {
343+
return w.ResponseWriter
344+
}
345+
346+
func (w *corsResponseWriter) deduplicate() {
347+
if w.deduplicated {
348+
return
349+
}
350+
w.deduplicated = true
351+
352+
h := w.ResponseWriter.Header()
353+
deduplicateHeader(h, echo.HeaderAccessControlAllowOrigin)
354+
deduplicateHeader(h, echo.HeaderAccessControlAllowCredentials)
355+
deduplicateHeader(h, echo.HeaderAccessControlExposeHeaders)
356+
deduplicateHeader(h, echo.HeaderAccessControlAllowHeaders)
357+
deduplicateHeader(h, echo.HeaderAccessControlAllowMethods)
358+
deduplicateHeader(h, echo.HeaderAccessControlMaxAge)
359+
deduplicateVary(h)
360+
}
361+
362+
func deduplicateHeader(h http.Header, key string) {
363+
values := h[key]
364+
if len(values) <= 1 {
365+
return
366+
}
367+
seen := make(map[string]bool)
368+
var result []string
369+
for _, v := range values {
370+
trimmed := strings.TrimSpace(v)
371+
if !seen[trimmed] {
372+
seen[trimmed] = true
373+
result = append(result, v)
374+
}
375+
}
376+
h[key] = result
377+
}
378+
379+
func deduplicateVary(h http.Header) {
380+
values := h[echo.HeaderVary]
381+
if len(values) == 0 {
382+
return
383+
}
384+
seen := make(map[string]bool)
385+
var varyParts []string
386+
for _, v := range values {
387+
for _, part := range strings.Split(v, ",") {
388+
trimmed := strings.TrimSpace(part)
389+
if trimmed == "" {
390+
continue
391+
}
392+
lower := strings.ToLower(trimmed)
393+
if !seen[lower] {
394+
seen[lower] = true
395+
varyParts = append(varyParts, trimmed)
396+
}
397+
}
398+
}
399+
if len(varyParts) > 0 {
400+
h.Del(echo.HeaderVary)
401+
for _, part := range varyParts {
402+
h.Add(echo.HeaderVary, part)
403+
}
404+
} else {
405+
h.Del(echo.HeaderVary)
406+
}
407+
}

0 commit comments

Comments
 (0)