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

What is the meaning of 'path' in accessors? #10

Closed
jakearchibald opened this issue Mar 9, 2016 · 14 comments
Closed

What is the meaning of 'path' in accessors? #10

jakearchibald opened this issue Mar 9, 2016 · 14 comments

Comments

@jakearchibald
Copy link

has/get/getAll take a path but don't do anything with it at the moment.

@jakearchibald jakearchibald changed the title getAll takes a path but doesn't appear to do anything with it get/getAll take a path but don't do anything with it Mar 9, 2016
@jakearchibald jakearchibald changed the title get/getAll take a path but don't do anything with it has/get/getAll take a path but don't do anything with it Mar 9, 2016
@jakearchibald jakearchibald changed the title has/get/getAll take a path but don't do anything with it What is the meaning of 'path' in accessors? Mar 9, 2016
@jakearchibald
Copy link
Author

#4 is somewhat related.

When I was sketching a similar design (w3c/ServiceWorker#707 (comment)) I couldn't settle on "match" or "get". "get" doesn't map well.

WIth the singular get/match, "name" is required, it doesn't make sense otherwise. With getAll/matchAll the name is optional. It feels like the APIs should reflect that. Eg:

navigator.cookies.match(name, { path });
navigator.cookies.matchAll({ name, path });

In the singular match, I'm not sure what the default path should be, maybe it should be required too.

Additionally I can't decide if "path" should be treating the path as a key, or a scope. Eg:

navigator.cookies.matchAll({
  path: someRequest.url
});

Should the above return me all cookies with their path set to someRequest.url, or should it return me all the JS-visible cookies that would be sent along with a request to someRequest.url?

@jakearchibald
Copy link
Author

I think trying to fit this into a map-like shape is confusing things.

Maybe it should be simpler?

getAll - takes no arguments, returns all the cookies for the domain. This means you can get multiple entries with the same name but different paths. Filtering is left to the developer.

I think the defaults and path stuff are too confusing in the get method, so maybe we shouldn't have one.

forURL(url) - returns the JS-visible cookies that would be sent along with a request to url. url could default to the client url, making forURL() return the same set of cookies as document.cookie. "name" would be unique in the returned cookies, so it could be a map.

@jakearchibald
Copy link
Author

returns all the cookies for the domain

Hmm, I guess it should only be "cookies for this origin and parent origins". Exposing cookies targeted to other subdomains would be bad.

@bsittler
Copy link
Contributor

bsittler commented Mar 9, 2016

Exposing paths and/or domains is new information not currently visible on
the web. Cookie:/reading document.cookie does not tell you the path or
domain, only the name and value.

Allowing the path to be specified seems OK, but I think requiring it is
odd, at least in document contexts. I think it should default to the path
of the current page.
On Mar 9, 2016 04:45, "Jake Archibald" notifications@github.com wrote:

returns all the cookies for the domain

Hmm, I guess it should only be "cookies for this origin and parent
origins". Exposing cookies targeted to other subdomains would be bad.


Reply to this email directly or view it on GitHub
#10 (comment)
.

@bsittler
Copy link
Contributor

bsittler commented Mar 9, 2016

(Also, specifying a read path just means you get back all cookies in-scope
at that path, including those set at parent paths.)
On Mar 9, 2016 06:09, "Ben Wiley Sittler" bsittler@gmail.com wrote:

Exposing paths and/or domains is new information not currently visible on
the web. Cookie:/reading document.cookie does not tell you the path or
domain, only the name and value.

Allowing the path to be specified seems OK, but I think requiring it is
odd, at least in document contexts. I think it should default to the path
of the current page.
On Mar 9, 2016 04:45, "Jake Archibald" notifications@github.com wrote:

returns all the cookies for the domain

Hmm, I guess it should only be "cookies for this origin and parent
origins". Exposing cookies targeted to other subdomains would be bad.


Reply to this email directly or view it on GitHub
#10 (comment)
.

@jakearchibald
Copy link
Author

Exposing paths and/or domains is new information not currently visible on the web. Cookie:/reading document.cookie does not tell you the path or domain, only the name and value.

Right, but should we carry on that tradition? I can work out the path of cookies by loading an iframe and brute forcing the url & seeing how document.cookie changes.

Allowing the path to be specified seems OK, but I think requiring it is odd, at least in document contexts. I think it should default to the path of the current page.

If you can specify the path, you make the above hack even easier.

(Also, specifying a read path just means you get back all cookies in-scope
at that path, including those set at parent paths.)

I don't disagree with that behaviour, but it makes this very un-map-like. The meaning of "path" in get/set becomes quite different.

@bsittler
Copy link
Contributor

bsittler commented Mar 9, 2016

My only intent when including a path argument was to allow a ServiceWorker to see script-visible cookies which would be in-scope for a given Fetch interception.

@jakearchibald
Copy link
Author

Yep, it's a good use-case

@jakearchibald
Copy link
Author

"name" would be unique in the returned cookies, so it could be a map.

No it wouldn't. TIL multiple cookies of the same name are sent in this case.

@domenic
Copy link

domenic commented Mar 9, 2016

Right, but should we carry on that tradition?

I think we have to, otherwise this API is useless. The original point of this API was to allow people to manage cross-site logins in their service workers.

@jakearchibald
Copy link
Author

@domenic I was referring to exposing the path of a cookie. Discussion split off into #11

@bsittler
Copy link
Contributor

bsittler commented Jul 27, 2016

I've attempted to address this issue in the explainer (where path is called 'url' instead). Please take a look there and let me know what you think.

In particular, I think the IFRAME-based brute-forcing is preventable by a server rigorously sending X-FRAME-OPTIONS: DENY provided the user isn't tricked into allowing a pop-up. What do you think?

@bsittler
Copy link
Contributor

Fixed explainer link

@bsittler
Copy link
Contributor

Polyfill for document contexts and explainer are synced now on this issue. Re-open if you think the path restrictions are problematic (and explain why, please!)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants