Skip to content

fix: ignore Expires attributes that parse as Invalid Date - #91

Merged
nfriedly merged 1 commit into
nfriedly:masterfrom
00200200:codex/ignore-invalid-cookie-expires
Oct 16, 2026
Merged

nfriedly merged 1 commit into
nfriedly:masterfrom
00200200:codex/ignore-invalid-cookie-expires

Conversation

@00200200

Copy link
Copy Markdown

An unparseable Expires attribute currently adds an Invalid Date to the returned cookie. A malformed duplicate also overwrites an earlier valid expiry, for example Expires=Wed, 21 Oct 2026 07:28:00 GMT; Expires=not-a-date. RFC 6265 section 5.2.1 requires ignoring an Expires attribute whose date cannot be parsed.

Only assign expires when the parsed Date has a valid timestamp. Tests cover empty and unparseable values, preserving an earlier valid expiry, retaining other attributes, and using the last valid expiry including the Unix epoch.

Validation on Node 20.19.5: two new regression tests fail on the original source. With the fix, npm test passes: CommonJS build, complete ESLint check, 67 Mocha tests, and tsd type checks. git diff --check also passes.

This retains native Date parsing; it does not implement the full cookie-date grammar or resolve the engine-specific valid-date issue in #35.

@nfriedly

nfriedly commented Oct 13, 2026 •

Copy link
Copy Markdown
Owner

Thanks for this! I have a branch where I have a custom date parser, but I think it will compliment this.

Do you have any real-world examples of where this went wrong, or is it just a theoretical fix?

@00200200

Copy link
Copy Markdown
Author

Thanks! I found this proactively while checking invalid Expires handling; I don't have a confirmed production incident. The regression is synthetic: an invalid duplicate Expires currently overwrites an earlier valid date with Invalid Date. This is a standards-conformance fix: ignore the invalid attribute and retain the valid expiry.

@nfriedly
nfriedly merged commit aab8962 into nfriedly:master Oct 16, 2026
3 checks passed
@nfriedly

Copy link
Copy Markdown
Owner

Ok, this will be published shortly in v3.1.4

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