Skip to content

Conversation

@SaisakthiM
Copy link

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:

if (opts.maxAge === null) {
  opts.maxAge = undefined;
}

This ensures that null values are excluded from expiration logic.

Test Results

  1. In cookie 1.0.2
    Test:
  1238 passing (11s)
  0 failing

Version :

npm list cookie
express@5.1.0 C:\Coding Project\Git\express
├─┬ cookie-parser@1.4.7
│ └── cookie@0.7.2
├── cookie@1.0.2
└─┬ express-session@1.18.2
  └── cookie@0.7.2
  1. In cookie 0.7.2
    Test:
  1238 passing (11s)

Version:

npm list cookie
express@5.1.0 C:\Coding Project\Git\express
├─┬ cookie-parser@1.4.7
│ └── cookie@0.7.2 deduped
├── cookie@0.7.2
└─┬ express-session@1.18.2
  └── cookie@0.7.2 deduped

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.

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.
@socket-security
Copy link

Review the following changes in direct dependencies. Learn more about Socket for GitHub.

Diff Package Supply Chain
Security
Vulnerability Quality Maintenance License
Addedcookie@​1.0.210010010082100

View full report

@efekrskl
Copy link

efekrskl commented Nov 3, 2025

Thanks for the PR!

This PR feels a bit off, as far as I can see maxAge: null already works fine. I think you are self introducing a regression by updating cookie to 1.0.2. If updating cookie is the goal, IMO that should be a separate PR with its own discussion and tests.

@SaisakthiM
Copy link
Author

SaisakthiM commented Nov 4, 2025

Thanks for the PR!

This PR feels a bit off, as far as I can see maxAge: null already works fine. I think you are self introducing a regression by updating cookie to 1.0.2. If updating cookie is the goal, IMO that should be a separate PR with its own discussion and tests.

yes, you are right i am trying to update the cookie to 1.0.2
but in the new cookie module, null values are not ignored and it throws a error if normally updated
so i also updated the test to convert the null to undefined so now it ignores and this approach is backward compatible
so that's the reason why i made this PR
it updates and handles tests and modules

the bigger question now is why we need separate discussion for updating cookie if it is handled here
is there any kind of security threats in cookie module or do you feel it is a unnecessary update

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants