zigzap / zap

blazingly fast backends in zig
MIT License
1.99k stars 71 forks source link

cookie Max-Age should be set to -1 when invalidating the cookie #19

Closed edyu closed 1 year ago

edyu commented 1 year ago

Browser will delete the cookie automatically when the max-age is set to -1.

renerocksai commented 1 year ago

According to the Mozilla doc:

Max-Age=<number> (Optional) Indicates the number of seconds until the cookie expires. A zero or negative number will expire the cookie immediately.

Based on that information, I see no benefit in changing 0 to -1.

According to this springio doc here:

A positive value indicates when the cookie should expire relative to the current time. A value of 0 means the cookie should expire immediately. A negative value results in no "Max-Age" attribute in which case the cookie is removed when the browser is closed.

An expired cookie (max-age: 0) will be removed when the browser closes, too. This is why we use max-age: 0 to implement session cookies.

Which is also reflected / recommended in the facil.io docs docs:

Max Age (how long should the cookie persist), in seconds (0 == session).

So, basically, 0 and -1 have the same effects, according to the above docs.

renerocksai commented 1 year ago

Thanks for the work you put into it. It's just that I don't see a reason to change one arbitrary constant to another if they're supposed to lead to identical consequences.

If, however, you can show me evidence that there seems to be deviating behavior by popular browsers, it might be a good idea to cater to their needs so users of Zap don't have to.

edyu commented 1 year ago

Setting to 0 will set it to session. It won't expire.

edyu commented 1 year ago

Btw session usually means when it will be cleaned up when browser closes so it's a slight annoyance. Because the value is already set to invalid so it doesn't leak much information except that the cookie is in the browser cache. I personally prefer the cookie to be removed by the browser which is what happens when it's set to -1. In other words it doesn't matter so much if it were set to 0. But the existing code actually sets it to what the max-age was set in init so you could have the token "invalid" in the browser for awhile.

edyu commented 1 year ago

Btw all this is observed in chrome.

edyu commented 1 year ago

Screenshot_20230520-161603.png

As you can see it's not a change from 0 to -1. It's from whatever the user sets (which could be arbitrarily long) to -1.

renerocksai commented 1 year ago

So, to be clear, for the record, let's state it how it supposedly is:

So this statement from MDN is wrong:

A zero or negative number will expire the cookie immediately.

And this statement from spring.io is wrong:

A value of 0 means the cookie should expire immediately. A negative value results in no "Max-Age" attribute in which case the cookie is removed when the browser is closed.

Because according to you, on a negative value the browser won't wait until it is closed to remove the cookie.

Man, the web is a mess. I will trust you on this one and merge.