jshttp / cookie

HTTP server cookie parsing and serialization
MIT License
1.32k stars 147 forks source link

Pull request in 0.1.1 broke existing functionality #23

Closed MaxMotovilov closed 10 years ago

MaxMotovilov commented 10 years ago

My library galette relied on treatment of maxAge: null as implemented by 0.1.0 and before, i.e. not serializing it. Changes in https://github.com/defunctzombie/node-cookie/pull/20 broke this by inserting Max-Age: 0 where it wasn't intended: instead of a non-persistent cookie I get one that is immediately dropped. I can adapt to new behavior but would like a clarification whether it will now be standardized upon.

The reason why I was using null to begin with was that I wanted to mask a non-null maxAge in the prototype of the dictionary being serialized.

defunctzombie commented 10 years ago

The fixes introduced were to support maxAge: 0 and should have no affect on maxAge: null or being undefined. Can you provide a test case showing this library inserting a max age when you did not specify one?

MaxMotovilov commented 10 years ago

Will do once I'm done firefighting :) For the time being I got clear break when switching from 0.1.0 to 0.1.1; just need to distill into a simple snippet.

MaxMotovilov commented 10 years ago

...never mind :) Thorough digging uncovered a 0 where I was positive a null should be, so this change simply brought a latent bug in my code into focus.

defunctzombie commented 10 years ago

:)