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

Investigate if the check for '/' in step 7 for Cookie::new_wrapped is correct #5944

Closed
jdm opened this issue May 5, 2015 · 6 comments
Closed
Assignees
Labels

Comments

@jdm
Copy link
Member

@jdm jdm commented May 5, 2015

    If the cookie-attribute-list contains an attribute with an
    attribute-name of "Path", set the cookie's path to attribute-
    value of the last attribute in the cookie-attribute-list with an
    attribute-name of "Path". Otherwise, set the cookie's path to
    the default-path of the request-uri.

This warrants further investigation.

@jdm jdm added the A-network label May 5, 2015
@jdm jdm mentioned this issue May 5, 2015
@frewsxcv
Copy link
Member

@frewsxcv frewsxcv commented Aug 2, 2015

@nox
Copy link
Member

@nox nox commented Apr 8, 2017

The check is correct, but maybe misplaced, or maybe the attribute is ignored altogether in the cookie crate itself.

@nox
Copy link
Member

@nox nox commented Apr 8, 2017

AFAICT the cookie crate doesn't do it itself. Should the parser ignore Path attributes that don't start with a /, @alexcrichton?

@nox nox self-assigned this Apr 8, 2017
@alexcrichton
Copy link
Contributor

@alexcrichton alexcrichton commented Apr 8, 2017

@SergioBenitez
Copy link

@SergioBenitez SergioBenitez commented Apr 9, 2017

@nox The cookie parser should not ignore Path values that do not begin with '/'. The grammar for path-value does not require a '/' as the first character (RFC6265 4.1.1). The cookie crate cannot default the cookie attribute value to default-path since it obviously lacks this context. As such, I believe the best thing to do is to forward the information upward so that the dependent library (here, servo) can make an informed decision.

@nox
Copy link
Member

@nox nox commented Apr 10, 2017

Ok, I see, so that step is correct, and the cookie crate shouldn't do anything different.

@nox nox closed this Apr 10, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
5 participants
You can’t perform that action at this time.