decodeURIComponent fails with 'URI malformed' error

[mikhailv]

In Google Chrome, if input data is incorrect, 'decodeURIComponent' method throws an exception with error string 'URIError: URI malformed'. This error causes cookie reading through 'Cookies' to stop working.

Actual behavior: code throws an error

Expected behavior: cookies with incorrect value not being returned. (possible solution: catch errors when '_getKeyValuePairFromCookieString' function is called or add error handling callback to Cookies API)

Environment: Chrome 33

To reproduce this error: invoke 'decodeURIComponent('%D0%EE%F1%F1%E8%FF')' in Chrome 33

[ScottHamper]

Hey Mikhail,

This error is occurring because the cookie value you supplied in your example is encoded as an "ISO 8859-1", or "ISO Latin-1", character, but Cookies.js expects cookie values to be encoded as UTF-8.

If you're setting this cookie on the server side, be sure to first URI encode the value in UTF-8.

In your example, the decoded value is:
"Ðîññèÿ"

The ISO 8859-1 encoding is:
"%D0%EE%F1%F1%E8%FF"

The UTF-8 encoding is:
"%C3%90%C3%AE%C3%B1%C3%B1%C3%A8%C3%BF"

[mikhailv]

Sorry, this is google translate (I am russian):

Hi, I understand the reason for this behavior , but in my case I do not control cookies on the site , the site is not mine. This site is loaded widget that uses this library , and it falls to the error as soon as trying to parse the cookie.

In my case I can either catch the exception, but then I can not get any cookies , because the error takes . It would be better if the library simply did not return such non-standard cookie values ​​. Because parsing specific cookie error is not caught falls parsing the entire list of cookies and a library in such a situation it becomes unusable.

I fixed the problem by modifying themselves library code, catch error and ignore such cookies, but would like to have the unmodified library code.

        for (var i = 0; i < cookiesArray.length; i++) {
            try {
                var cookieKvp = Cookies._getKeyValuePairFromCookieString(cookiesArray[i]);
                if (cookieObject[cookieKvp.key] === undefined) {
                    cookieObject[cookieKvp.key] = cookieKvp.value;
                }
            } catch (e) {
                // ignore parse exceptions
            }
        }

Certainly better to catch an error in a method _getKeyValuePairFromCookieString

[ScottHamper]

Hey Mikhail,

RFC6265 defines the set of valid characters allowed in a cookie value, and it's a rather limited list. To get around this limitation and allow people to set cookie values with any characters they want, the library will URI encode/decode values as UTF-8.

If the library didn't encode values as UTF-8, it would have to encode them in a different format instead, in order to still comply with RFC6265.

UTF-8 seems like the most logical encoding for the library to use, as its a Unicode encoding, and can support any character.

It's unfortunate that your situation does not allow for you to control what cookie values are being set on the server side. One thing you could try is to modify _getKeyValuePairFromCookieString to wrap the usage of decodeURIComponent with a try/catch, and in the catch, fall back to using the unescape method instead. unescape will decode the string as ISO instead of UTF-8. However, note that this method is now considered deprecated, and future browsers may not support it. As a result, I will not be making this change in the official Cookies.js project.

[rymai]

Hello @ScottHamper!

We're in the same situation as @mikhailv and I think that's a shame that cookies-js is not robust enough and cannot handle malencoded cookies. As @mikhailv proposed, the lib could simply ignore such cookies without any fallback to unescape and everything would be fine.
The end-result would be better than now: skip a malencoded cookie and not crash the page instead of trying to decode a malencoded cookie and crash the page.

Would you be interested in a pull-request toward that goal?

[ScottHamper]

Hey @rymai ,

I recognize the problem of being unable to read any cookies if there's only a single malformed cookie. This is definitely not ideal, and we can come up with a better solution. However, I consider silent failures to be poor practice, as they make debugging more difficult. The documentation for Cookies.js clearly lays out the expectations for encoding, and a malformed cookie value should be considered an error.

So it seems to me that the real issue is not that an error is thrown, but that an error is thrown regardless of whether or not the user is trying to access a malformed cookie. The reason this happens today is because Cookies.js attempts to parse/decode/cache the entire document.cookie string before returning the requested cookie value.

A more ideal implementation would be one which lazily decodes cookie values, so that the only time an error occurs is when the user attempts to read a malformed cookie. This approach could still have a complete failure if any cookie key is malformed, but would solve the issue for malformed values.

Solving the issue for malformed keys is tricker, as there's no way we can lazily decode any specific key. The best solution I can think of would be to use a try/catch like you guys have proposed for values, but then use console.error if it exists in the current browser in order to avoid silent failures (where possible). Ideally, there would be a way to let the error throw as normal, but continue execution afterwards. I can't think of a way to accomplish this, though, so console.error seems like the best compromise.

In any case, I can spend some time working on this later today. Let me know what your thoughts are, and if you have any other ideas for solutions that could be more ideal.