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

Jump to bottom

net/http: parsing of cookie attribute values is too lax #7751

Closed
vdobler opened this issue Apr 10, 2014 · 6 comments
Closed

net/http: parsing of cookie attribute values is too lax #7751

vdobler opened this issue Apr 10, 2014 · 6 comments
Labels
FrozenDueToAge
Milestone
Go1.4

Comments

@vdobler
Copy link
Contributor

vdobler commented Apr 10, 2014 

Currently the parsing of the values for the cookie attributes
Domain, Path, Secure, Max-Age, Expires and HttpOnly is too lax:
1. Quoted values are unquoted (which is okay for the cookie-value
itself, but not for the attribute values according to RFC 6265)
2. All values are parsed with basically the full set of allowed
characters.

Point 2 might be overkill, but there is no need to unquote
as browsers do not handle quoted attribute values.

As the infrastructure for restricted set of characters is
already in place with func parseCookieValueUsing(...) it
might be worth to allow only '0' to '9' while parsing Max-Age
and fail early (now malformed values fail in strconv.Atoi)
@ianlancetaylor
Copy link
Contributor

Comment 1:

Labels changed: added repo-main, release-go1.4.

@rsc
Copy link
Contributor

rsc commented Sep 16, 2014

Comment 2:

Nigel, worth doing anything for 1.4?

@nigeltao
Copy link
Contributor

Comment 3:

It's been a while since I remember the details of HTTP cookies... can you give examples
of "Cookie: etc" lines that we're not parsing correctly?
Also, this isn't a regression, and if we're not doing it correctly, we've always not
done it correctly, right?

Status changed to WaitingForReply.

@vdobler
Copy link
Contributor Author

vdobler commented Sep 17, 2014

Comment 4:

Cookie: lines are okay, the issue is with Set-Cookie: headers, e.g.
Set-Cookie: name=value; Max-Age="45"
should be rejected according to http://tools.ietf.org/html/rfc6265#section-5.2.2
as the value of Max-Age starts with a quote and only digits and minus is
allowed. (Only the cookie-value should be unquoted before processing, the
cookie-avs should not be unquoted.)
See http://play.golang.org/p/PpzVRGeg0S

@gopherbot
Copy link

Comment 5:

CL https://golang.org/cl/148890043 mentions this issue.

@nigeltao
Copy link
Contributor

Comment 6:

This issue was closed by revision e59ad69.

Status changed to Fixed.

@rsc rsc added this to the Go1.4 milestone Apr 14, 2015
@rsc rsc removed the release-go1.4 label Apr 14, 2015
@golang golang locked and limited conversation to collaborators Jun 25, 2016
wheatman pushed a commit to wheatman/go-akaros that referenced this issue Jun 25, 2018
@nigeltao @wheatman
net/http: allow double-quotes only on cookie values, not cookie
69f82e0 
attribute values, a la RFC 6265 section 4.1.1 "Syntax".

Fixes golang#7751.

LGTM=dr.volker.dobler
R=dr.volker.dobler
CC=bradfitz, golang-codereviews
https://golang.org/cl/148890043
wheatman pushed a commit to wheatman/go-akaros that referenced this issue Jun 26, 2018
@nigeltao @wheatman
net/http: allow double-quotes only on cookie values, not cookie
de5be32 
attribute values, a la RFC 6265 section 4.1.1 "Syntax".

Fixes golang#7751.

LGTM=dr.volker.dobler
R=dr.volker.dobler
CC=bradfitz, golang-codereviews
https://golang.org/cl/148890043
wheatman pushed a commit to wheatman/go-akaros that referenced this issue Jul 9, 2018
@nigeltao @wheatman
net/http: allow double-quotes only on cookie values, not cookie
ef65709 
attribute values, a la RFC 6265 section 4.1.1 "Syntax".

Fixes golang#7751.

LGTM=dr.volker.dobler
R=dr.volker.dobler
CC=bradfitz, golang-codereviews
https://golang.org/cl/148890043
wheatman pushed a commit to wheatman/go-akaros that referenced this issue Jul 30, 2018
@nigeltao @wheatman
net/http: allow double-quotes only on cookie values, not cookie
631c8af 
attribute values, a la RFC 6265 section 4.1.1 "Syntax".

Fixes golang#7751.

LGTM=dr.volker.dobler
R=dr.volker.dobler
CC=bradfitz, golang-codereviews
https://golang.org/cl/148890043
This issue was closed.
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees
No one assigned
Labels
FrozenDueToAge
Projects
None yet
Milestone
Go1.4
Development

No branches or pull requests
5 participants
@rsc @vdobler @ianlancetaylor @nigeltao @gopherbot