fix(router): Handle repeated headers in response #1537
Conversation
|
Hi @cmtm, thanks for the PR. Please add an integration test to verify the correct behaviour. Example: https://github.com/wundergraph/cosmo/blob/main/router-tests/header_set_test.go |
cmtm
force-pushed
the
forward-multiple-headers
branch
from
0f48af5 to
32268e5
Compare
|
I've added the integration tests. |
cmtm
force-pushed
the
forward-multiple-headers
branch
from
32268e5 to
32c5307
Compare
| return testenv.SubgraphsConfig{ | ||
| Employees: testenv.SubgraphConfig{ | ||
| Middleware: func(handler http.Handler) http.Handler { | ||
| return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { | ||
| w.Header().Set(header, valA) | ||
| w.Header()[header] = valA |
There was a problem hiding this comment.
Hi @cmtm
Manually modifying the underlying map is not fully equal to header.Set, because the wrapper always uses canonical form of the header name
The simplest fix will be
w.Header()[textproto.CanonicalMIMEHeaderKey(header)] = valA
There was a problem hiding this comment.
We should write a test that verify it.
There was a problem hiding this comment.
Done. I wrote a test for it, but I'm not sure how the http.Header type handles canonicalization internally. If it canonicalizes when deserializing an http request, we might not be able to properly test the canonicalization with it. We'd need to use a lower level construct.
There was a problem hiding this comment.
Note that I didn't change the behavior of this handler, since it's okay if it doesn't do canonicalization.
When multiple headers with the same name were sent in the response from a subgraph, only one would be included in the response to the client when forwarding headers. Fix it so that they're all forwarded.
cmtm
force-pushed
the
forward-multiple-headers
branch
from
7f78495 to
3807826
Compare
When multiple headers with the same name were sent in the response from
a subgraph, only one would be included in the response to the client
when forwarding headers. Fix it so that they're all forwarded.