feat: take client request headers into account when deciding whether to reuse WS conns #647
Conversation
…to reuse WS conns Otherwise it's not possible for the client to route headers through the engine because a connection with different headers might be reused instead of creating a new one. Instead, now we only reuse connections if the headers except Sec-Websocket-Key and Content-Length match.
…g-headers-via-websocket
…g-headers-via-websocket
| // However, to improve the changes of connection reuse we do skip | ||
| // some headers that are very likely to be different but very unlikely | ||
| // to cause different responses, otherwise no connections would be reused. | ||
| for key, values := range ctx.Request.Header { |
There was a problem hiding this comment.
I think we should have an allow list of headers that we want to add to the hash.
E.g. in most cases I think we want to limit this to just "Authorization".
This allow list could be configurable at the resolver level, and then we expose it through the cosmo router yaml.
WDYT?
I can also imagine use cases where we want the user to be authenticated, but the origin connection should not use the Authorization header. So the behavior should be configurable and potentially opt-out.
cc @StarpTech
There was a problem hiding this comment.
I agree, it should be configurable and I like the proposal to maintain a waitlist rather than taking all headers into consideration. I'd definitely add Authorization by default to prevent data exposure and let the rest be use case specific.
There was a problem hiding this comment.
I've refactored it so the caller can pass a list of headers. This allows us to make it automatic in the router side, by passing the headers declared in the forwarding rules. PTAL
…d-support-for-forwarding-headers-via-websocket
Introduce a new field in SubscriptionConfiguration that allows selecting which headers get hashed when decided when to reuse a connection.
…-forwarding-headers-via-websocket' into alberto/eng-4378-add-support-for-forwarding-headers-via-websocket
…d-support-for-forwarding-headers-via-websocket
…d-support-for-forwarding-headers-via-websocket
fiam
deleted the
alberto/eng-4378-add-support-for-forwarding-headers-via-websocket
branch
…to reuse WS conns (wundergraph#647) Allow callers to indicate a list of headers (by name / regular expression) that must be taken into account when deciding to reuse connections.
Allow callers to indicate a list of headers (by name / regular expression) that must be taken into account when deciding to reuse connections.