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

WARequestCookie>>path: sets empty path for '/' #917

Closed
theseion opened this issue May 27, 2017 · 2 comments
Closed

WARequestCookie>>path: sets empty path for '/' #917

theseion opened this issue May 27, 2017 · 2 comments
Assignees
Labels
Milestone

Comments

@theseion
Copy link
Member

(relates to issues #915 and #916, as setting the path on the session cookie can fix it to be used across the entire domain)

WACurrentRequestContext
    use: (WARequestContext
        request: WARequest new
        response: WAResponse new)
    during: [
	WACookie new
	    key: 'foo';
	    value: 'bar';
	    path: '/';
	    rfc6265String ] 

Output: 'foo=bar; path='
Expected: 'foo=bar; path=/

A possible workaround is to use #pathEncoded:codec: but I don't think that is the solution

@theseion
Copy link
Member Author

I've added a couple of tests for edge cases. That has lead me to reading the specs (https://tools.ietf.org/html/rfc6265#section-5.1.4) and I've modified the path behaviour so that the path attribute will always be sent. The spec says that user-agents must use an equivalent of the following algorithm:

  1. Let uri-path be the path portion of the request-uri if such a
    portion exists (and empty otherwise). For example, if the
    request-uri contains just a path (and optional query string),
    then the uri-path is that path (without the %x3F ("?") character
    or query string), and if the request-uri contains a full
    absoluteURI, the uri-path is the path component of that URI.

  2. If the uri-path is empty or if the first character of the uri-
    path is not a %x2F ("/") character, output %x2F ("/") and skip
    the remaining steps.

  3. If the uri-path contains no more than one %x2F ("/") character,
    output %x2F ("/") and skip the remaining step.

  4. Output the characters of the uri-path from the first character up
    to, but not including, the right-most %x2F ("/").

That means:

  • missing path attribute -> '/'
  • empty path attribute -> '/'
  • path without leading slash -> '/'

I think that setting the path attribute explicitly a) makes debugging easier and b) clarifies intent and c) prevents problems when browsers do funny things with path matching on implicit paths.

What do you think?

theseion added a commit to theseion/Seaside that referenced this issue May 27, 2017
* initialize the cookie path attribute to '/' as that is what browsers are supposed to use as default / fallback
* the cookie path attribute will now always be set
* WARequestCookie>>pathEncoded now correctly answers '/'  instead of the empty string when '/' was set as path 
* cookie paths are now forced to use a leading slash. Browsers will replace paths without leading slash with '/'
* cookie paths are now forced to omit a trailing slash. Browsers will remove the trailing slash (with '/' as the exception)
* nil, the empty string, and '/' are now treated as equal when setting the path of a cookie

* added tests for all of the above
* updated other cookie tests that did not expect the path attribute in the output
theseion added a commit to theseion/Seaside that referenced this issue May 28, 2017
* fixed initialization of path attribute (could fail when no request context was yet available, e.g. when parsing cookies in the server adaptor)
@marschall
Copy link
Contributor

I think that's a bug in WARequestCookie >> #pathEncoded:codec: even though RFC 6265 specifies that empty and / should be treated equally.

marschall added a commit to marschall/Seaside that referenced this issue Sep 10, 2018
@marschall marschall mentioned this issue Sep 10, 2018
marschall added a commit that referenced this issue Sep 11, 2018
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