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

net/http: document that SetCookie can quote the value #37055

Closed
gazerro opened this issue Feb 5, 2020 · 4 comments
Closed

net/http: document that SetCookie can quote the value #37055

gazerro opened this issue Feb 5, 2020 · 4 comments
Labels
Documentation FrozenDueToAge NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one.
Milestone

Comments

@gazerro
Copy link
Contributor

gazerro commented Feb 5, 2020

The SetCookie function quotes the value if it contains spaces or commas but this behavior is not documented in SetCookie documentation:

SetCookie adds a Set-Cookie header to the provided ResponseWriter's headers. The provided cookie must have a valid Name. Invalid cookies may be silently dropped.

@odeke-em
Copy link
Member

odeke-em commented Feb 5, 2020

Thank you for the report @gazerro! Kindly cc-ing @vdobler for Cookies.

@cagedmantis cagedmantis added this to the Backlog milestone Feb 6, 2020
@cagedmantis cagedmantis added the NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. label Feb 6, 2020
@gopherbot
Copy link

Change https://golang.org/cl/221677 mentions this issue: net/http: document that SetCookie can quote the value

@vdobler
Copy link
Contributor

vdobler commented Mar 3, 2020

The quotes are allowed by RFC 6265. We could quote every value on Mondays and be compliant to the spec. The fact that the cookie value might be quoted is implicit in "a Set-Cookie header". The whole idea of providing dedicated cookie handling functions in net/http is to free the user from parsing Cookie and Set-Cookie headers himself. The value might be quoted or not but net/http, all relevant browsers and all cookie handling libraries in other languages transparently handle quoted and unquoted values. I do not see why the user needs to be informed about this particular detail of the generated Set-Cookie header. Other details are also not mentioned as they are regulated by the relevant spec (e.g. time zone of the expires field).

I do not think that this behaviour should be documented: Once documented it cannot be changed anymore.
The quoting was introduced to provide compatibility with other languages and common browsers to allow formally (according to RFC 6265) invalid cookie-values to be used because they are common in the wild.

The documentation states

Invalid cookies may be silently dropped.

but does not specify what exactly is considered "invalid", simply because this is a moving target: If every browser properly handles some character X and most other HTTP libraries allow X then net/http has tried to also allow X.

What values are considered valid/invalid and which one need quoting has changed several times. These changes have allowed users to use Go with existing browsers and existing legacy systems. If we fix what is quoted we loos the ability to accommodate for future changes.

I don't think this should be documented:

  • documenting it specifies it and makes it unchangable
  • the more interesting fact which values are invalid is undocumented for exactly this reason
  • the fact that quotes might or might not be used is part of the relevant RFC 6265 already

@gazerro
Copy link
Contributor Author

gazerro commented Jul 31, 2021

Now that the proposal #46370, to add a Cookie.Valid method, has been accepted, I think this issue is not that important anymore, so I close it.

@gazerro gazerro closed this as completed Jul 31, 2021
@golang golang locked and limited conversation to collaborators Jul 31, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Documentation FrozenDueToAge NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one.
Projects
None yet
Development

No branches or pull requests

5 participants