fix(cookies): keep an Expires of 0 when serializing a cookie - #5808
Open
NgoQuocViet2001 wants to merge 1 commit into
Open
fix(cookies): keep an Expires of 0 when serializing a cookie#5808NgoQuocViet2001 wants to merge 1 commit into
NgoQuocViet2001 wants to merge 1 commit into
Conversation
The Expires branch tested cookie.expires for truthiness, so a numeric 0 -- the Unix epoch, and the canonical way to say "expire this cookie now" -- dropped the attribute entirely. expires is typed Date | number, and the Max-Age check directly above already tests the type rather than truthiness for the same reason.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
stringifytestsexpiresfor truthiness:expiresis typedDate | number, and a numeric0is the Unix epoch — the canonical way to say expire this cookie now. It is falsy, so the attribute is silently dropped.The
Max-Agebranch a few lines above already avoids exactly this:Effect
Only the numeric-zero case, and only for
expires. No error is raised — a caller asking for immediate expiry gets a session cookie instead, which is the unsafe direction for the usual purpose (clearing a cookie on logout).Fix
Treat
null/undefinedas absent instead of treating every falsy value that way. After the change:Test plan
test/cookie/cookies.jsbeside the existing numeric-expirestest:expires: 0serializes the epoch, andexpires: nullstill omits the attribute.node --test test/cookie/*.js→ 93 passing.util.jsfails the new test with'Space=Cat' == 'Space=Cat; Expires=Thu, 01 Jan 1970 00:00:00 GMT'.npx standardon both touched files → clean.