ScottHamper / Cookies

JavaScript Client-Side Cookie Manipulation Library
The Unlicense
1.77k stars 169 forks source link

Attempting to get a properly encoded cookie fails if another cookie's key/value is malformed #47

Closed ScottHamper closed 9 years ago

ScottHamper commented 9 years ago

See #22 for background

Errors should only be thrown when the user attempts to get a malformed cookie.

rymai commented 9 years ago

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.

Thanks @ScottHamper for considering improving the status quo, it's much appreciated!

Your proposal is indeed more elegant than mine which only sweep the error under the carpet!

Do you have the time to implement it or would you like us to help?

ScottHamper commented 9 years ago

Hey @rymai ,

I've implemented a quick resolution. It is definitely not the prettiest implementation, but tbh I've been meaning to rewrite the entire library to have better organization for awhile now anyways. Maybe I will finally get around to that. In any case, I'll work on publishing a release with the fix.

Thanks for checking out the lib!

rymai commented 9 years ago

Awesome, this should work well for us, I'll keep you posted.

Thanks again for your reactivity and the quick fix!

jayfunk commented 9 years ago

@ScottHamper Just wanted to check if you did have time to get around to your solution to this issue? I ran into this issue recently using the library.

ScottHamper commented 9 years ago

Hey @jayfunk ,

Yep! It's in the latest 1.2.2 release.