fasthttp
fasthttp copied to clipboard
`RequestHeader.Cookie`: Improve multiple cookies with same name support
From my understanding of the code, and testing though a third party server using fasthttp (Authelia), it seems like RequestHeader.Cookie(and thus Session.getSessionID) uses the first matching value it finds.
According to RFC 6265 4.2.2. Semantics:
Although cookies are serialized linearly in the Cookie header, servers SHOULD NOT rely upon the serialization order. In particular, if the Cookie header contains two cookies with the same name (e.g., that were set with different Path or Domain attributes), servers SHOULD NOT rely upon the order in which these cookies appear in the header.
While not mandatory (RFC uses "SHOULD", not "MUST"), it would be nice to implement this behavior.
The use case I have, and why I ended up here is I'm using the same service on two domains: auth.example.com and auth.sub.example.com. When my browser makes a request to svc.sub.example.com, it sends both cookies (Same-Site=lax is required for auth on subdomains that are not auth), thus fasthttp only finds the session if the auth.sub cookie was serialized first.
Current solution on my side is using different cookie names, but I thought I'd bring it up here since it seems like a valuable addition.
P.S. I only skimmed the code so this might not be possible, but maybe making RequestHeader.cookies a map instead of a slice would both make the code simpler and faster.
The problem is that all our methods assume there aren't duplicate cookies. RequestHeader.Cookie(key) for example returns a single value. Changing that to return multiple values would break backwards compatibility. Adding new methods for this is possible but adds even more methods to our already huge API surface.
Currently there is already a way to get the values of duplicate cookies: RequestHeader.VisitAllCookie()
I understand you don't want to break the API, or add something new, but I don't think that is necessary: changing RequestHeader.Cookie(key) to return the "best" cookie instead of the first could be done with only internal changes.
I'm open for a pull request that only changes which of the cookies is returned.