Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

RequestHeader.Cookie: Improve multiple cookies with same name support #1584

Open
ThinkChaos opened this issue Jun 25, 2023 · 3 comments
Open
Labels

Comments

@ThinkChaos
Copy link

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.

@erikdubbelboer
Copy link
Collaborator

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()

@ThinkChaos
Copy link
Author

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.

@erikdubbelboer
Copy link
Collaborator

I'm open for a pull request that only changes which of the cookies is returned.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants