[Lazarus revival] fix(res): handle null maxAge gracefully in res.cookie() and updated the dependency "cookie" to the 1.0.2 latest version - #1
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.
|
thanks, i left the PR it was idle for a long time thanks for the fix |
|
Hey @SaisakthiM — thank you. This PR is exactly why I built the tool. It's called Lazarus, a project for OpenAI Build Week: it finds open-source pull requests that went stale (merge conflicts, the base branch moving on, maintainers getting busy) and uses an OpenAI Codex agent to rebase them onto the current code, resolve the conflicts, and get the tests green again — then opens a fresh PR crediting the original author. Your null-maxAge fix was one of the ones it revived: it rebased onto today's master, handled the cookie 1.0.2 dependency jump, and the suite went from conflicted to 1260 passing / 0 failing. You did the real work months ago; the tool just made sure it didn't get lost. Honestly, your "it was idle for a long time, thanks for the fix" comment means more than any test result — that's the whole reason the project exists. 🙏 |
2 participants
Maintainer Briefing: fix(res): handle null maxAge gracefully in res.cookie() and updated the dependency "cookie" to the 1.0.2 latest version
What this PR was trying to do
Normalize
res.cookie()options so an explicitmaxAge: nullproduces a session cookie withoutExpiresorMax-Age, while retaining existing handling for numeric values. The PR also proposes upgrading the directcookiedependency to 1.0.2.What changed in the repo since it died
The repository has advanced substantially, including several response-related changes, dependency upgrades, and additional tests. No listed intervening commit directly addresses null
maxAgehandling. The baseline did not merge cleanly and had 1,237 passing tests with one unrelated acceptance-test failure.What Lazarus changed to revive it
The revived branch merges cleanly and
npm testreports 1,260 passing tests. A follow-up commit,284b82a9, changesres.sendFile()to pass absolute paths as a basename plus parentroot, avoiding dotfile-parent rejection bysend@1.2.1; targetedres.sendFile()tests and lint were also reported as passing. The supplied log does not show the revived cookie normalization, dependency-version diff, or associated test changes, so their exact final implementation is unavailable.Test evidence
Risk assessment
The intended cookie behavior is narrowly scoped, but the dependency upgrade may alter serialization or validation beyond
maxAge: null; evidence for those exact changes is unavailable. The additionalres.sendFile()change is unrelated to the PR’s cookie purpose and affects absolute-path handling, so it warrants separate maintainer review despite the clean full test run.Credits
Originally contributed by @SaisakthiM.
Original PR: expressjs#6875
Original author: @SaisakthiM