[arvados] updated: 2.6.0-279-g1fb22ef77

[git repository hosting]

Summary of changes:
 lib/controller/localdb/container_gateway.go | 43 ++++++++++++++++++-----------
 1 file changed, 27 insertions(+), 16 deletions(-)

       via  1fb22ef7709eb9b07b05b863d1bdadc39e35c995 (commit)
      from  00c93619f7691c0828f5273bc457e2840dbdc084 (commit)

Those revisions listed above that are new to this repository have
not appeared on any other notification email; so we list those
revisions in full, below.


commit 1fb22ef7709eb9b07b05b863d1bdadc39e35c995
Author: Tom Clegg <tom at curii.com>
Date:   Mon Jun 19 15:29:58 2023 -0400

    20647: Improve comment on header deduplication.
    
    Arvados-DCO-1.1-Signed-off-by: Tom Clegg <tom at curii.com>

diff --git a/lib/controller/localdb/container_gateway.go b/lib/controller/localdb/container_gateway.go
index e42a44730..0b6a630fa 100644
--- a/lib/controller/localdb/container_gateway.go
+++ b/lib/controller/localdb/container_gateway.go
@@ -184,14 +184,7 @@ func (conn *Conn) ContainerRequestLog(ctx context.Context, opts arvados.Containe
 					return httpserver.ErrorWithStatus(errors.New("bad X-Arvados-Authorization-Response header"), http.StatusBadGateway)
 				}
 				resp.Header.Del("X-Arvados-Authorization-Response")
-				for hdr := range resp.Header {
-					// proxy.ServeHTTP adds each
-					// resp.Header to w.Header,
-					// which causes duplicate CORS
-					// and request-id headers,
-					// unless we do this.
-					w.Header().Del(hdr)
-				}
+				preemptivelyDeduplicateHeaders(w.Header(), resp.Header)
 				return nil
 			},
 			ErrorHandler: func(w http.ResponseWriter, r *http.Request, err error) {
@@ -283,14 +276,7 @@ func (conn *Conn) serveContainerRequestLogViaKeepWeb(opts arvados.ContainerLogOp
 			}
 		},
 		ModifyResponse: func(resp *http.Response) error {
-			for hdr := range resp.Header {
-				// proxy.ServeHTTP adds each
-				// resp.Header to w.Header, which
-				// causes duplicate CORS and
-				// request-id headers, unless we do
-				// this.
-				w.Header().Del(hdr)
-			}
+			preemptivelyDeduplicateHeaders(w.Header(), resp.Header)
 			return nil
 		},
 	}
@@ -304,6 +290,31 @@ func (conn *Conn) serveContainerRequestLogViaKeepWeb(opts arvados.ContainerLogOp
 	proxy.ServeHTTP(w, r)
 }
 
+// httputil.ReverseProxy uses (http.Header)Add() to copy headers from
+// the upstream Response to the downstream ResponseWriter. If headers
+// have already been set on the downstream ResponseWriter, Add() will
+// result in duplicate headers. For example, if we set CORS headers
+// and then use ReverseProxy with an upstream that also sets CORS
+// headers, our client will receive
+//
+//	Access-Control-Allow-Origin: *
+//	Access-Control-Allow-Origin: *
+//
+// ...which is incorrect.
+//
+// preemptivelyDeduplicateHeaders, when called from a ModifyResponse
+// hook, solves this by removing any conflicting headers from
+// ResponseWriter. This way, when ReverseProxy calls Add(), it will
+// assign the new values without causing duplicates.
+//
+// dst is the downstream ResponseWriter's Header(). src is the
+// upstream resp.Header.
+func preemptivelyDeduplicateHeaders(dst, src http.Header) {
+	for hdr := range src {
+		dst.Del(hdr)
+	}
+}
+
 // serveEmptyDir handles read-only webdav requests as if there was an
 // empty collection rooted at the given path. It's equivalent to
 // proxying to an empty collection in keep-web, but avoids the extra

-----------------------------------------------------------------------


hooks/post-receive
--