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

Feature request: Provide a way to read _all_ values of a cookie by name #813

Open
maxwellgerber opened this issue Mar 16, 2023 · 29 comments

Comments

@maxwellgerber
Copy link

maxwellgerber commented Mar 16, 2023

Is your feature request related to a problem? Please describe.
Whenever the same cookie name is set with differing domain or path semantics, multiple instances of that cookie are added to document.cookie. For example, try running the following code at https://example.com/somepath

image

The current API of js-cookie will only return the first value that matches the cookie name (Cookies.get('foo') === 'c').
As an application developer, this means it is impossible to access certain cookie values, or even realize that you've gotten yourself into this situation, without looking at document.cookie yourself directly.

I recognize that this is not the default case for most situations, but it is still a scenario that developers can find themselves in. For example, suppose an application migrates its cookies from being stored on a root domain, to being shared across all subdomains (or vice versa!) - being able to read two instances of the cookie name is a useful feature. Developers can use that information to attempt to unset cookies, console.log out debug info, etc.

Describe the solution you'd like
I'd love a new method - Cookies.list() - with similar semantics to Cookies.get(). Pass in the name of the cookie to retrieve and it will return all values stored for that name. Pass in nothing and it will return an object containing all lists of cookies.

namespace Cookies {
    list(name: string): string[]
    list(): Record<string, string[]>
}

Given a document.cookie of foo=a; foo=b; foo=c, I'd expect the following output:

Cookies.list('foo')
// => ['a', 'b', 'c']
Cookies.list()
// => { foo: ['a', 'b', 'c'] }
Cookies.list('bar')
// => []

a rough implementation might look very similar to the existing implementation for get:

  function list (name) {
    ...
    var cookies = document.cookie ? document.cookie.split('; ') : []
    var jar = {}
    for (var i = 0; i < cookies.length; i++) {
      var parts = cookies[i].split('=')
      var value = parts.slice(1).join('=')

      try {
        var found = decodeURIComponent(parts[0])
        jar[found] = jar[found] || [];
        jar[found].push(converter.read(value, found))
      } catch (e) {}
    }

    return name ? jar[name] || [] : jar
  }

Describe alternatives you've considered
Obviously, a developer can access document.cookie directly, but they'll also need to reimplement the logic in converter.read to ensure that they're getting the same values. At that point, they've reimplemented half of this library themselves.

Unlike issues like #800, it is impossible to build this functionality from the existing API.

@FagnerMartinsBrack
Copy link
Member

FagnerMartinsBrack commented Apr 21, 2023

We have an api called Cookies.get() without arguments that retrieves all cookies visible on the page, how's this feature request different than Cookies.get() or something like:

const cookies = Cookies.get();
Object.keys(cookies).filter(byname('cookieName')).map(key => ({ name: key, value: cookies[key] }));

About this:

Cookies.list('foo')
// => ['a', 'b', 'c']
Cookies.list()
// => { foo: ['a', 'b', 'c'] }
Cookies.list('bar')
// => []

It returns a different return type (Array Literal -> Object Literal -> Array Literal) based on the arguments, which is a bad API design. Return types should be predictable and stable in a method call regardless of the number of arguments.

(.get() is an exception to this rule due to historical reasons and byte-size optimizations by not having too many API tokens that have to be supported for a long time)

@carhartl
Copy link
Member

carhartl commented Apr 21, 2023

how's this feature request different

Cookies with the same name but from different scopes do all appear in document.cookie (easily observable in devtools) -- at least in some browsers --, and won't shadow each other, I learned the other day. They won't be distinguishable though, i.e. no way to tell from which scope one or the other is. Thus I'm not sure how useful it is to expose all, but ignoring all but the last, in get(), also seems to be incorrect. And is even inconsistent with get('foo'), which will return the first hit.

@FagnerMartinsBrack
Copy link
Member

FagnerMartinsBrack commented Apr 21, 2023

That makes sense, .get() can't be used. The semantics there is to list visible cookies as key/value pairs, I wonder if it's even useful to have .get() once we get smth like list().

For list():

  1. Definitely we can't return { cookieA: 'A', CookieB: 'B' } because Object Literals don't allow multiple keys with the same name.
  2. Optionally [{ name: 'cookieA', value: 'A' }, { name: 'cookieB', value: 'B' }] for .get() would maintain 3 methods but a change in behaviour from existing API, even on a major bump it will create issues in the wild.
  3. Cookies.list('foo') // => ['a', 'b', 'c'] definitely less bytes than Cookies.list() // => [{ name: 'cookieA', value: 'A' }, { name: 'cookieB', value: 'B' }]. It's just that Cookies.list() have to return [] which is confusing (listing all but return nothing?). Also creates a new method without having to touch .get().

Certainly a v3 feature, just need to make sure it passes Browserstack as I'm afraid we may get issues with shadowing.

So in summary: This issue is to add the ability to return more than one cookie with the same name which is impossible today by .get() due to returning an Object Literal

@carhartl
Copy link
Member

carhartl commented Apr 23, 2023

I thinking of changing the api in a future version, with both get() and get('foo') consistently returning arrays.

get() // => [{foo: 'bar'}, {foo: 'baz'}]
get('foo') // => ['bar', 'baz']

The arrays might rather be a Set in order to avoid responses like [{foo: 'bar'}, {foo: 'bar'}]

  • There won't be another list() method, which seems more like a workaround to me
  • Fixes the aforementioned inconsistency of behavior that we at the moment observe with get([name])
  • list() will also not solve get()

@carhartl
Copy link
Member

carhartl commented Apr 23, 2023

Alternatively, in a future version, there could be the list() or all() method, we document that get(name) returns the first cookie that is found, and get rid of the overloaded get() function altogether. get(name) should then probably be called findFirst(name) 🙈

@FagnerMartinsBrack
Copy link
Member

FagnerMartinsBrack commented Apr 23, 2023

I thinking of changing the api in a future version, with both get() and get('foo') consistently returning arrays.

That solves the Array<any> return that is consistent for both get(String) and get(). However, it's still inconsistent with Array<String> vs Array<Object<string:string>>.

For example, processCookies(cookies) would break due to the different type in the Array item (just a pseudo code below):

- const cookies = Cookie.get(); // Code gets all cookies, first implementation
+ const cookies = Cookie.get('authId'); // Author changes the code to get only auth-related cookies in the future without changing the processing. The assumption here is that the type is consistent.
processAuthCookies(cookies);

[...]

const processAuthCookies = (cookies) => {
  // This breaks simply from a change of a parameter of a code the user does not control.
  // The cause/effect is so far from it that it's almost impossible to debug and takes hours depending on the complexity of the app
  console.log(cookies.map(cookie => processAuthId(cookie.authId)));
};

Of course, we can justify any API decision with "you should read the docs". It's just that the Principle of Least Surprise is violated in that case.

@FagnerMartinsBrack
Copy link
Member

get(name) should then probably be called findFirst(name) 🙈

I see the why hands in the eyes :D
Less methods = better = less bytes = less complexity
.findFirst() could be just .someMethod()[0] in the userland, not much value to add to the lib for like 3 chars in userland.

The only concern I guess is that if we start changing return types based on input it will create a lot of confusion for the reasons above.

@carhartl
Copy link
Member

carhartl commented Apr 23, 2023

The only concern I guess is that if we start changing return types based on input

I'm confused.. could you explain which return type we plan to change based on input? I understand that we are currently changing the return type in get([name]) based in argument provided or not.

@carhartl
Copy link
Member

carhartl commented Apr 23, 2023

Elsewhere I try to convince everyone that these days, especially with some IE peculiarities out of the way, you don't need a library any longer to work with cookies, haha:

const s = 'foo=bar; foo=baz'
Array.from(s.matchAll(/(?![ ;])([^=]+)=([^;]+)/g)) // all the cookies

(sure would need to map the iterator/array to cleanup the result)

:)

@FagnerMartinsBrack
Copy link
Member

FagnerMartinsBrack commented Apr 23, 2023

I'm confused.. could you explain which return type we plan to change based on input? I understand that we are currently changing the return type in get([name]) based in argument provided or not.

That's what I mean:
Change return type based on input === changing the return type in get([name]) based in argument provided or not

We're talking about the same thing. Input = argument. My mental model is return = fn(input) and yours seems to be return = fn(argument) just different names (where fn in this case is get(...))

Changing the return type of the function/method by not providing an argument is as much a surprise. I believe that to be one of the worst decisions we've ever made in regards to js-cookie API design, not worse than the old .json flag :D
At least our decisions are versioned. document.cookie is forever.

--

Elsewhere I try to convince everyone that these days, especially with some IE peculiarities out of the way, you don't need a library any longer to work with cookies, haha:

Same here, tough you still need js-cookie to encode characters disallowed by the spec, just because browsers allow certain chars doesn't mean servers always will. That's IMHO the only value proposition of this project in modern days, and that's why this has been relevant for decades -> we design once, works forever.

Not many projects have had this mindset, and that's why you have what you have in the frontend tooling/community.

@max-stytch
Copy link

max-stytch commented May 6, 2023

It returns a different return type (Array Literal -> Object Literal -> Array Literal) based on the arguments, which is a bad API design. Return types should be predictable and stable in a method call regardless of the number of arguments.

Big ➕ to this - was just trying to match existing behavior of .get() 😄

It sounds like there are three options:

  1. Change nothing (and close this ticket). Add a note to the README warning about shadowing and explaining the current library behavior (returning first cookie sometimes, last cookie sometimes).
  2. Add a net-new method, and leave get() the way it is for historic reasons. This increases API surface & bundle size, but allows us to support shadowing without surprising existing users.
  3. Fundamentally change get() behavior with a major version release.

If we're OK defining a new pattern for a new function, my vote would go to

all(): Array<{name: string, value: string}> 

and then letting filtering by name occur in userland.

const fooCookies = Cookie.all().filter(c => c.name === 'foo')
  function all () {
    var cookies = document.cookie ? document.cookie.split('; ') : []
    var jar = []
    for (var i = 0; i < cookies.length; i++) {
      var parts = cookies[i].split('=')
      var value = parts.slice(1).join('=')

      try {
        var name = decodeURIComponent(parts[0])
        jar.push({ name, value: converter.read(value, name) })
      } catch (e) {}
    }

    return jar
  }

@carhartl
Copy link
Member

carhartl commented May 6, 2023

That would mean we‘d leave get('foo') fundamentally flawed as it is.

I think first of all I would want to figure out whether all relevant browsers behave the same.

@carhartl
Copy link
Member

carhartl commented May 6, 2023

Apart from that, I‘m also in favor of all(). We could be deprecating get() in a new mayor release, and subsequently remove it.

@carhartl
Copy link
Member

carhartl commented May 6, 2023

That would mean we‘d leave get('foo') fundamentally flawed as it is.

Though I‘m asking myself, — thinking out loud here — what kind of use-case this really is. You‘d have two (or more) cookies with the same name and there was no way to distinguish between them (by age or some other attribute). Returning the first seems as good/useful as returning a list.. (and you could always resort to all() to at least detect if there are two cookies with the same name visible).

@FagnerMartinsBrack
Copy link
Member

FagnerMartinsBrack commented May 7, 2023

what kind of use-case this really is

That's a great question. If you're reading this, please comment with a real-life scenario of returning multiple cookies with the same name. We can always abide by technical implementations but js-cookie is known by its simple API and going an extra mile to develop 20% of the work that gives 95% - not 80% - of the value.

Also, we optimize for byte size. Unless the use case is real and relevant and there's no other way of doing it in userland, we historically always favour bytes over features.

One of the features we don't wanna break if possible is "< 800 bytes gzipped!"

@max-stytch
Copy link

If you're reading this, please comment with a real-life scenario of returning multiple cookies with the same name.

What prompted myself opening this ticket was a migration we ran to move a cookie from being shared with subdomains (foo=a; domain=.example.com) to only being shared with the root domain (foo=b;). We wanted to determine if multiple cookies with the same name were set from JS userland. The only way we could do this was by copying most of js-cookie into our app (admittedly quite simple to do!) and changing the get(name: string) method to return an array, since we still needed the read & write converters.

A comparable issue could be the treatment of headers in serverside Node.js - in the original release headers were treated as a Map<string, string> but this had to change to accomadate multiple headers of the same name: nodejs/node-v0.x-archive#2750 (comment)


One of the features we don't wanna break if possible is "< 800 bytes gzipped!"

I bet we can make that happen ;)

@carhartl
Copy link
Member

I think first of all I would want to figure out whether all relevant browsers behave the same.

I did a relatively superficial experiment, three mayor browsers behave the same: https://github.com/js-cookie/js-cookie/wiki/Cookies-with-duplicated-name

@carhartl
Copy link
Member

What's interesting to note as well is that the nested cookie consistently comes first.. probably a side-effect of adhering to

Cookies with longer paths are listed before cookies with shorter paths.

https://www.rfc-editor.org/rfc/rfc6265#section-5.4

carhartl added a commit that referenced this issue May 30, 2023
In the case of two (or more) cookies with the same name visible to the
current scope, so far we've been retrieving the first one we find for
`get('name')` and the last one for `get()` (because in the latter case
we continued the loop and eventually overwrote a duplicate cookie name
in the returned result/object).

This change makes the behavior at least consistent, so there aren't
different cookies in the result for both calls.

We should be returning all visible cookies though, but the API change
for this is still being discussed.. see:
#813

Also see: https://github.com/js-cookie/js-cookie/wiki/Cookies-with-duplicated-name
carhartl added a commit to carhartl/typescript-cookie that referenced this issue Jun 24, 2023
In the case of two (or more) cookies with the same name visible to the
current scope, so far we've been retrieving the first one we find for
`getCookie()`, but the last one for `getCookies()` (because in the
latter case we continued the loop and eventually overwrote a duplicate
cookie name in the returned result/object).

This change makes the behavior at least consistent, so there aren't
different cookies in the result for both calls.

Changing the api to include both cookies in the result seems reasonable.

For context: js-cookie#813
@mh-trimble
Copy link

mh-trimble commented Jun 29, 2023

what happened for me was in testing for the issue in #828, that the Cookies.get returned the last modified cookie. and if you look at the documents.cookie string you can see that changing the cookie value changes the position of the entry. (for the same path but different domain)

@guest271314
Copy link

Any reason get() doesn't return GitHub cookies logged_in and dotcom_user?

@carhartl
Copy link
Member

httpOnly maybe?

@guest271314
Copy link

@carhartl Not sure what you mean? I can read the cookie in Application panel of Chrome DevTools, not with document.cookie. I'm trying to programmatically detect being logged in to GitHub so I can redirect from github.com to github.com/dashboard-feed.

@guest271314
Copy link

@carhartl A visual of what I'm talking about
Screenshot_2023-09-23_10-00-59

@carhartl
Copy link
Member

If you can‘t read it with document.cookie I‘m sure it‘s a httpOnly cookie. There should be a column for that in the Chrome dev tools as well indicating this attribute.

@guest271314
Copy link

Thanks. Never really experimented with cookies before. Am just trying to figure out a way to programmatically determine when I am logged in to GitHub.

@chmerk
Copy link

chmerk commented Apr 23, 2024

Hi I wanted to ask for the state of this, add our real world problem and report some browser specific behavior that I found:

  1. Hi guys. Great you are still doing this! :) What is the state of this request?

  2. Our real world scenario: We develop a consent dialog ("cookie banner") as a Stencil.js-built web component for a big corporation (i.e. hundreds of websites) and use js-cookie for cookie access (e.g. reading and storing users decisions).
    We support "sharing" consent across subdomains by using a common storage cookie for the "main domain". (So integrators can configure, whether they want to store the cookie for sub.domain.com or domain.com).

Scenario A: We have two major versions so far and also versioned the cookies IN the cookie (default cookie name is the same). When user comes to sub.domain.com, with an old cookie from domain.com consent banner shows up, as it's not a "v2 cookie". Component stores users decision in new cookie (v2) for sub.domain.com.
Upon navigation (or reload), banner shows up again, as js-cookie (in some browsers) always returns the "main" cookie.
This is a UX issue for us (but can be solved by either just using the main-domain for everything (if that is wanted in that use case) or - as a workaround - configuring a different cookie name).

Scenario B: User previously visited domain.com and accepted (gave cookie consent). Then user comes to sub.domain.com (consent banner does not show up, as it's a valid "v2 cookie"). User goes to settings and changes consent (declines).
Upon navigation (or reload), component will use "main cookie" again (the one with the wrong user decision), as js-cookie (in some browsers) always returns the "main" cookie.
This is even a legal problem for us. But as a workaround can be solved by configuring a different cookie name.

However, in both cases I would expect js-cookie/the browser to return the more specific cookie (which would be sub.domain.com). But I realize this is not possible for you due to technical limitations.

  1. I observed this behavior in Firefox (125) and Chromium (123) but not in Safari (17.4). The latter seems to use the "correct" cookie.

I'm happy to provide additional info or support otherwise if needed!
Thanks a lot! :)

@FagnerMartinsBrack
Copy link
Member

FagnerMartinsBrack commented Apr 24, 2024

@chmerk Hi, thanks for your comment.

As stated in the contributing guidelines, in order to take any further action we need a Short, Self Contained, Correct (Compilable), Example that shows a problem with js-cookie.

As I see this seems to be an issue with browsers and your business domain model so it should be reported to them, js-cookie is only a Façade on browser cookies and we don't implement any subdomain-specific logic or browser-compatible workarounds as there's only so much we can do with JavaScript.

For support, you can create a question in StackOverflow under the tag js-cookie. The reason why we don't answer support questions on Github is that StackOverflow has a higher relevance in Google results and therefore a solution posted there is more likely to help other people with the same problem 😃 .

Don't let this stop you from keep contributing to js-cookie 👍

Thank you!

@chmerk
Copy link

chmerk commented Apr 24, 2024

Hi @FagnerMartinsBrack,

thanks for the quick reaction. This was not meant as a support question but to provide additional real-life examples in support of the OP.

For clarity, I'll phrase the problem to be more js-cookie-specific:
Afaik, there is currently no way with js-cookie to see multiple cookies with the same name (a.k.a shadowed cookies). So we're running into issues, where cookies with the same name exist for parent and subdomains. Especially as in some cases (browsers) js-cookie returns the "more general" cookie (parent domain) on .get(cookiename), instead of the more specific one.
In cases where we observed this (FF, Chrome), this happens even after js-cookie wrote a new (specific) cookie for the subdomain, but returns the other one upon next .get(cookiename).

So, I would be happy to see something like .list() or .all() (or an adaption of get()) in js-cookie to deal with this sort of situations.

Just interested: Why didn't this land in v3?

Again: Thanks a lot! :)

@FagnerMartinsBrack
Copy link
Member

FagnerMartinsBrack commented Apr 25, 2024

This thread is mostly about returning an array of cookies whenever you do Cookies.get(name) so that we can account for cookies with the same name, then it morphed into which browsers support showing many cookies and the attributes limitations of it. The use case here is a cookie that has the exact same attributes, only set in a different subdomain or domain where it's still accessible in document.cookie.

@carhartl made some research before but AFAIK it hasn't been extensive. I don't think we do get them for all browsers, and we may hook into an implementation detail and bring more questions/support issues our way, but happy to be proven wrong. The work required here is an extensive research on the duplicated cookies in different subdomains so that browser-specific limitations don't make it impossible for us to implement that on js-cookie side.

@chmerk That's probably why we haven't done this in V3 since there hasn't been enough cross-browser research in this area at the time.

The questions the research need to answer:

  1. Are all browsers consistent in their behaviours with cookies with the same name for every attribute? We need a list of behavior with each browser version that encompasses 99% of browsers out there.
  2. Is the behaviour in the spec (or are we willing to send a review to RFC 6265 and send a correction to add to the spec?)
  3. If change is required, is the impact of changing Cookies.get(name) to return arrays worth the impact for 99% of common usage, even as a major version? Or do we document that uses like this may require a userland-specific code on top of document.cookie?

If anyone wanna help with "1" I can help with "2" and "3" after that.

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

No branches or pull requests

7 participants