fix(res): handle null maxAge gracefully in res.cookie() and updated the dependency "cookie" to the 1.0.2 latest version #6875
Conversation
This pull request fixes an edge case in res.cookie() where specifying maxAge: null caused the function to produce incorrect cookie headers or throw an internal error.
Previously, maxAge: null was not normalized and led to unintended conversion to 0, causing an undesired Expires header (Max-Age=0).
This behavior conflicted with Express’s intended semantics, where null should mean “no expiration — session cookie”.
✅ Updated Behavior
When maxAge is explicitly null, it is now normalized to undefined.
The cookie is serialized without Expires or Max-Age fields (consistent with session cookies).
Existing logic for finite and numeric maxAge values remains unchanged.
All test cases, including res.cookie(name, string, options) maxAge should not throw on null, now pass successfully.
⚙️ Implementation Summary
In lib/response.js, the logic under res.cookie() was updated:
if (opts.maxAge === null) {
opts.maxAge = undefined;
}
This ensures that null values are excluded from expiration logic.
✅ Test Results
All 1,238 tests pass locally:
1238 passing (11s)
0 failing
🧩 Motivation
This change improves compatibility with earlier Express 4.x behavior, aligns with the HTTP cookie specification for session cookies, and maintains backward compatibility with existing user code.
|
Review the following changes in direct dependencies. Learn more about Socket for GitHub.
|
|
Thanks for the PR! This PR feels a bit off, as far as I can see |
yes, you are right i am trying to update the cookie to 1.0.2 the bigger question now is why we need separate discussion for updating cookie if it is handled here |
|
@efekrskl |
|
Hi @SaisakthiM, I'd say I've made my points already about the PR. Let's wait for others to chime in. Let's not tag them directly, though, because the team is really busy. Keep in mind this doesn't mean that your PR is bad or anything. We just need more opinions before making a final decision. |
sorry for that |
|
wll it take more time for review |
|
any progreess done |
There was a problem hiding this comment.
I see that there are two changes here: one is updating cookie and the other is handling a null case (or are they related?). The cookie update cannot be done until express 6, as it introduces breaking changes such as jshttp/cookie#180 (see https://github.com/jshttp/cookie/releases/tag/v1.0.0).
That’s why I’m adding the labels, so we can revisit this in the future when we start working on Express 6.
3 participants
This pull request fixes an edge case in res.cookie() where specifying maxAge: null caused the function to produce incorrect cookie headers or throw an internal error when updated to 1.0.2
Previously, maxAge: null was not normalized and led to unintended conversion to 0, causing an undesired Expires header (Max-Age=0). This behavior conflicted with Express’s intended semantics, where null should mean “no expiration — session cookie”.
Updated Behavior
When maxAge is explicitly null, it is now normalized to undefined.
The cookie is serialized without Expires or Max-Age fields (consistent with session cookies).
Existing logic for finite and numeric maxAge values remains unchanged.
All test cases, including res.cookie(name, string, options) maxAge should not throw on null, now pass successfully.
Implementation Summary
In lib/response.js, the logic under res.cookie() was updated:
This ensures that null values are excluded from expiration logic.
Test Results
Test:
Version :
Test:
Version:
Motivation
This change improves compatibility with earlier Express 4.x behavior, aligns with the HTTP cookie specification for session cookies, and maintains backward compatibility with existing user code.