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
net/http: AddCookie fails to add a malformed cookie to a request #38437
Comments
the cookie delimiter (defined in RFC6265 Section 5.4.4.2) is "; " with a space, AddCookie works when the delimiter is properly used |
Yes, However I would still expect AddCookie to work in this case by either
|
the code for AddCookie is very simplistic https://golang.org/src/net/http/request.go?s=15364:15402#L423 the same RFC also says there must only be 1 cookie header |
Why does request.Cookie() parse all |
Kindly /ccing @vdobler, in addition to what @seankhliao recommends. |
The issue title is misleading, the problem is not in net/http.Request.AddCookie not adding the cookie to the request. AddCookie works properly. The problem stems from mixing manual (via Request.Header) and automatic (via Request.AddCookie) cookie management using malformed manual cookies. Unfortunately there is not much which can be done here. AddCookie can be used to properly add an arbitrary number of cookies but nothing prevents the user from manually adding extra Cookie entries or adding or changing existing entries. If this manual action is done properly net/http.Request.Cookies will properly parse the Cookie header. This behaviour is analog to how net/http.Request.Cookies works for server requests: If the client sends a malformed Cookie header it won't be able to parse that header. mariusgrigaitis seems to understand that the problem is manual messing up the header and suggests a few things to do (different order):
So all "solutions" require adding (complicated) documentation which explains the various quirks if you mix malformed manual-cookies and wellformed AddCookie-cookies. I think the underlying problem is the assumption you could safely mix malformed manual-cookies ("MMC") and wellformed AddCookie-cookies ("WAC"): You cannot. This should be documented. But it doesn't require any code change. The problem is not that you cannot mix MMC and WAC, the problem is the expectation that you could. You can mix wellformed manual cookies and AddCookie cookies but that is almost a no-brianer. There are valid use cases for manual fiddling with the Cookie header and setting strange values and there are valid uses for AddCookie. Both ways of setting cookies are provided and you are encouraged to use AddCookie only. But if your back is to the wall you can manually set Cookie headers in which case you must take great care: You can mix manual setting Cookies and AddCookies but results depend on what you do manually. |
net/http tries to be strict what it sends but halfway liberal on what it accepts as input. RFC 6265 forbids sending multiple Cookie headers but doesn't disallow parsing several Cookie headers. |
Thank you @vdobler for the analysis, and thank you too @seankhliao and @mariusgrigaitis! |
Change https://golang.org/cl/235141 mentions this issue: |
What version of Go are you using (
go version
)?Does this issue reproduce with the latest release?
Yes
What operating system and processor architecture are you using (
go env
)?go env
OutputWhat did you do?
https://play.golang.org/p/yagiRaYwKRk
https://play.golang.org/p/UVW1Hh9O8Gc
What did you expect to see?
I would expect
request.AddCookie("name", "value")
followed byrequest.Cookie("name")
to always return Cookie withname=value
or at least Cookie with same key but previous value.What did you see instead?
request.Cookie() does not see the Cookie.
Other considerations
Different programming languages / libraries behave differently in cookie parsing. If golang application is acting as "proxy" and adds cookies to the request, backend systems could parse cookies differently.
The text was updated successfully, but these errors were encountered: