-
Notifications
You must be signed in to change notification settings - Fork 2.1k
Cookie name with brackets #595
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
Comments
Could you explain why you need brackets in the cookie‘s key? |
I think this feature request is kinda related to this one: #560 |
@seahindeniz Do you have control over the |
@carhartl could you elaborate more? What do you mean by control over? If you mean can I delete, update or create a new one? - Yes I can do that. |
Sorry, I wasn't clear. My question was, would you be in control of the backend writing the cookie in question, and thus change its name to something that is complying with the standard? But you seem to have answered that already. For the time being you will have to resort to |
It's always possible to have an option to avoid complying with standards. However, that standard was built based on the way browsers interpret cookies so that if you deviate from it your cookie won't be written or read correctly. When I built the encoding, I tested every single character in the latest Safari, Chrome, Firefox, Opera, and IE. Many of the ones that didn't comply with the standard couldn't be read or written using In this case, it makes sense to use the library and discover that you can't use a square brackets in the name than deploy the app in production using Again, it's possible to add an option to override it if you still want to have control over the @carhartl I guess we didn't release 3.0 yet so there might still be time for an API change like this? |
Thank you @FagnerMartinsBrack I'm actually Ok complying with the standards but sadly I can't do anything about making them to comply and that's the first problem I couldn't solve 🙂 I currently solved the problem on my end by following the document.cookie suggestion. But IMHO would be good to have a new option for to override, till all browsers strictly follows the naming standards. |
I'm still a bit torn here. There's no value in strictly adhering to a standard when the real world looks different. On the other hand we have to apply some standard for the encoding, be it an official standard or a de-facto one. I wouldn't know though what the latter one would be... I've also been thinking about what a reasonably simple, possibly backwards-compatible api could look like. I will definitely want to refrain from overloading the return type of the Here's my idea: writing, reading and the name are three different things, therefore the solution I have in mind - and I believe, given the real-world, we'll have to add supporting bypassing any encoding -, is adding a third option to the converter api: Cookies.withConverter({
write: ...,
read: ...,
name: ...
}
}) This provides the simplicity + flexibility I desire (and even compatibility, making it easy for users to migrate from v2 -> v3).
Not really, mostly I didn't find the time, but of course now we still have the opportunity do bake a solution into v3. |
I'm going to give it a try later today, and see how this plays out... |
(Also, I believe a naming converter doesn‘t just apply to writing a cookie, it‘s also involved when getting one.) |
Some browsers won't work correctly with cookies if you deviate from the standard. It's not like the real world is different, only Chrome is more lenient than other browsers with the brackets in the name, and Chrome is not the world (yet).
Yeah you would require read/write converters for the name too, that's why I was thinking on something like this: const NameConverter = Cookies.NameConverter;
const ValueConverter = Cookies.ValueConverter;
Cookies.withConverter(NameConverter({
read: () => {},
write: () => {}
}));
Cookies.withConverter(ValueConverter({
read: () => {},
write: () => {}
})); This one keeps cohesion of the interface over composing the kind of converter the dev wanna use. This design also don't allow having both read and write for name and value. or Cookies.withConverter({
readName: () => {},
readValue: () => {},
writeName: () => {},
writeValue: () => {},
}); Same as what you proposed, only that "write" and "read" are not default to "value". That's an assumption that has to be documented. If we change read/write to readValue/writeValue then it's a self-documented API at the cost of an API change for a major version. |
Cookies.withValueConverter({
read: () => {},
write: () => {}
}).withNameConverter({
read; () => {},
write: () => {}
});
/// ----
const usingAlgorithmA = () => {};
const usingAlgorithmB = () => {};
Cookies
.withValueConverter(usingAlgorithmA)
.withNameConverter(usingAlgorithmB); Con: API Breaking Change or Cookies({
processName: {
read: () => {}
write: () => {}
},
processValue: {
read: () => {}
write: () => {}
}
}) Just throwing some random ideas at this point haha Con: Law of Demeter |
I checked in Chrome, Firefox, Safari (all latest) and IE 11 and all of them accepted |
Anyway, I want to absolutely avoid adding the complexity that's originating from adding converters for both key and value. There was once the idea to make use of the given converters for both name and value, should there come the need for it. Maybe that time is now. I'll investigate whether that's a feasible avenue. Then, if you have to have "non-standards-mode" like requested here, you could basically implement "noop" converters: Cookies.withConverter({
read: value => value,
write: value => value
}) |
I was misunderstood. I didn't mean to state that all the major browsers today do not accept brackets. I didn't even run that test recently. It's tempting to dump standards as long as all server-side and client-side implementations we test are working fine. We can even argue that Chromium is going to be everywhere in 5 years' time so why bother supporting different platforms? However, we don't have proof that all server-side and client-side implementations out there and, most importantly, the ones who are created in the future will accept brackets. They will all base themselves on RFC 6265 as we did here too. Maybe they will be more lenient, maybe not, impossible to know without making assumptions. The standard is not perfect. For example, some PHP standard functions escape and unescape the plus sign by default while the RFC 6265 allows it to be used on Although the standard is not perfect, we don't have proof that how the world looks like today will be the same in the future. The reason the web worked so well and a website from 1995 still renders is because it was built on top of standards, even when those standards didn't make any sense.
Every time you escape a character it adds a few more bytes to it. It increases the size of the cookie. The cookie header cannot be compressed by GZip. According to RFC 6265, the cookie value can have dozens of characters that are not allowed and the cookie name only a few. By coupling the converter to both name and value we are forcing the developer to increase the payload size of an individual cookie unnecessarily, which helps it reach the limit. Although designing an API for both name and value adds some complexity to js-cookie internals, it doesn't add more complexity or unintended side-effects to the developer using the library. I believe we should add a converter for each |
I'm also in favour of keeping name and value converters distinct. If you merge them you'll lose the ability to make complex value converters like |
I‘m not arguing for dropping adherence to a standard. But: we might be adhering to the wrong one. The DOM specification for document.cookie refers to RFC 2965: https://www.w3.org/TR/DOM-Level-2-HTML/html.html#ID-8747038 We have proof for this in at least a couple of mayor browsers. I’m pretty sure this is not going to change in the next 10 years, if ever, simply because it would “break the web” in myriads of ways. Lesson to be learned here from the HTML 5 spec, which in many ways was modeled after the real world, and not vice-versa. Furthermore, the status of RFC 6265 is still “Proposed Standard” (emphasis on proposed). https://www.rfc-editor.org/info/rfc6265 Also quoting MDN:
https://developer.mozilla.org/en-US/docs/Web/API/Document/cookie This is the status quo that developers expect to work with. RFC 6265 isn’t. We’re sitting in an ivory tower by adhering to a theoretical standard. |
And because of that we’re discussing adding more converters, which is to me the wrong solution to the (self-made) problem.. If we choose to stick to the current encoding implementation (I wouldn‘t) we need to enable developers to work with legacy cookies (let‘s call them that maybe), by way of opting out of default conversion for the entire cookie — because real-world. And not force them to set up ever more converters. In many years of maintaining a cookie library I have never seen the need for encoding a cookie name and value in different ways, which is what the proposed converter setup is providing us. |
That changes everything. Unfortunately, I don't have answers for that. Based on a quick search, there are around 393k websites using js-cookie - the search might be skewed due to the use of bundlers. From those, 103k are calling That could mean 30% of folks using js-cookie are also using converters, but that's just a rough unprecise estimation. |
The only thing I can do at this point is to summon @abarth (author of the spec), @bagder (author of curl), @montulli (who co-created RFC 2965 and coined the term "cookie") with the question: Based on the content of this thread, is there a practical reason why we would continue using RFC 6265 as a reference implementation to understand which characters are allowed and which ones are not in a cookie name and cookie value? Which one should we be using to understand how cookie name and cookie value should be encoded? According to RFC 6265, cookie-name is a token, defined in RFC2616, Section 2.2:
RFC 2965 specified the syntax for the header, where "attr" is "token" but doesn't link to any reference (what's "token" there then?):
Different systems (server, clients, and userland applications) are accepting or rejecting the name/value of cookies in different ways and developers are asking for ways to encode/decode a cookie name differently to bypass RFC 6265 guidelines. We can't encode every From what it seems, RFC 2965 seems to be the most "real world" specification out there which doesn't specify anything and leaves to the developer to research in a given point in time which characters are valid and which ones are not. Are we missing anything here? |
I took another look at RFC 6265.. if I understand it correctly the rules we are adhering to are aimed at server implementations. For user-agents the spec
Section 5.2 describes the algorithm to be used when reading the set-cookie header. https://tools.ietf.org/html/rfc6265#section-5.2 On the other hand, I‘m imagining that this applies to browser vendors, not necessarily us. |
If you're parsing the Set-Cookie header, you should use the algorithm in Section 5.2. Otherwise, you'll often encounter Set-Cookie headers that you cannot parse correctly. |
@abarth We're using The problem is there are userland implementations that are accepting plain characters that are NOT allowed in a cookie-name (or Should we be using RFC 6265 as a guideline for |
That's not really how cookies work. There isn't any such thing as an invalid character from the point of view of cookie consumers. The point of Section 5.2 is to explain how to process every octet sequence.
What do you mean by "not supported in the spec?" Section 5.2 doesn't treat brackets any differently than any other characters. The only character that are treated differently from other characters are
|
We're talking about the I'm looking at the This is the
Notice the:
Note also the "except separators". Separators are specified as:
The brackets are there, so "any CHAR except CTLs or separators" is the same as "any CHAR except CTLs or square brackets" |
Our RFC 6265 based cookie name + value encoding was based on the requirements for server implementers, and as such too strict. We need to look at RFC 6265 section 5.2, requirements for user agents. But even the algorithm described in section 5.2 isn't that relevant for us, in that we had to follow the steps when writing a cookie with this library. This is left to the browser vendors who have to implement the setter functionality behind `document.cookie`. All we have to do is avoid surprises when said algorithm is being followed while `document.cookie = ...` is being executed with the cookie string we're constructing. It means we have to encode ";" and "=" in the cookie name, and ";" in the cookie value. Follow-up to discussion in #595.
I just thought that we could still face interoperability issues given that RFC 6265 doesn't require a particular encoding at all... we can't have a ";" in the cookie name, but the spec doesn't say how it should be encoded (percent-encoding seems to be common though?). Another possibility for how to support distinct converters for name + value: always return two values (which would at least nicely mirror what we're already passing to the converter): Cookies.withConverter({
write: function (value, name) {
return [value.toUpperCase(), name.toUpperCase()]
}
}) I just hope we don't need it and accept percent-encoding in the cookie name for ";" and "=" as a given. |
So far we haven't had reports with That means, of course, we have to test for cookies that have a percent char in the value and in the name (such as |
With the current implementation we don't have to bother, it's using a regex without any danger of misinterpreting "%". Unless someone would insist on using |
Our RFC 6265 based cookie name + value encoding was based on the requirements for server implementers, and as such too strict. We need to look at RFC 6265 section 5.2, requirements for user agents. But even the algorithm described in section 5.2 isn't that relevant for us, in that we had to follow the steps when writing a cookie with this library. This is left to the browser vendors who have to implement the setter functionality behind `document.cookie`. All we have to do is avoid surprises when said algorithm is being followed while `document.cookie = ...` is being executed with the cookie string we're constructing. It means we have to encode ";" and "=" in the cookie name, and ";" in the cookie value. Follow-up to discussion in #595.
Closed by #608 |
I've bumped the v3 beta containing the new implementation. https://github.com/js-cookie/js-cookie/releases/tag/v3.0.0-beta.4 |
Introduced: #623 |
Back to the drawing board.. |
To remind myself here, reading such a cookie already works, it's all about being able to write it. |
Let me double check this! While Safari doesn't support deviating from RFC 6265 for the cookie value (#623), it seems to allow it for the key. Then, if all browsers we support would support this, we may relax the enocding for the name while writing and be done. There'd be a test to know if one of these browsers would start to change the behavior. I would be ok doing this for the time being as a compromise. We wouldn't be adding new code, might even reduce it slightly. |
It's a mess. While Safari seems to adhere to RFC 6265 more strictly for the value (#623 (comment)), for the name it is deviating from the spec: |
Yes, it's a mess. That's why I strongly suggest going with the spec. The mess of today may not be the mess of tomorrow, so compromises may break later. I believe we can create a more stable library if we follow a common/standard guideline. Creating our own compromises IMHO is a recipe for disaster. |
I agree.. had a weak moment. |
So here's a summary:
I suggest to either a) create a "Converter API" that allows devs to overwrite the standard converter of cookie-name Ping @carhartl |
Likely going to settle with: const Customized = Cookies.withReadConverter({
key: (key) => { ... },
value: (value, key) => { ... }
})
.withWriteConverter({
key: (key) => { ... },
value: (value, key) => { ... }
}) |
That's a good one @carhartl. Here are my assumptions:
Are those assumptions correct so far? I can't see any issues with this approach. |
Yes to all three @FagnerMartinsBrack |
Formerly known as converter... It's required mainly for being able to write 3rd-party cookies that aren't RFC 6265 compliant and where we therefore need to bypass the default encoding, similar to what is already possible for the value. See js-cookie#595
This comment was marked as spam.
This comment was marked as spam.
It's been a while since the last comment on this issue so I'll reiterate the current state since 2021: This issue is essentially pending the implementation of the code that @carhartl suggested in this comment. There won't be a need for a major version bump (new functions added). If you're reading this and willing to open a PR with proper tests implemented for that functionality, we'll review and potentially accept. Otherwise, wait for the maintainers here to implement it, but given the current interest in doing so I don't see this landing soon unless there's some community push with PRs. |
Is your feature request related to a problem? Please describe.
Because of escaping characters on the key, I can't able to add brackets in key without having escaped characters.
js-cookie/src/api.mjs
Lines 21 to 23 in b1d83a0
Describe the solution you'd like
I'm suggesting to have a additional way to override the key replacers same as with the value.
js-cookie/README.md
Lines 308 to 312 in b1d83a0
to have something like this definition
and in additional, this definition to have an overloaded return type to keep both definition exist.
Describe alternatives you've considered
Passing additional optional parameter to disable escapers.
Additional context
I know this solution can expect this this answer #590 (comment) . I'm only suggesting to have an optional way to avoid complying to standards.
Such as this
js-cookie/src/rfc6265.mjs
Lines 5 to 10 in b1d83a0
The text was updated successfully, but these errors were encountered: