Skip to content

fix: parse JSON cookies with falsy values - #172

Open
spokodev wants to merge 2 commits into
expressjs:masterfrom
spokodev:fix/json-cookies-falsy-values
Open

fix: parse JSON cookies with falsy values#172
spokodev wants to merge 2 commits into
expressjs:masterfrom
spokodev:fix/json-cookies-falsy-values

Conversation

@spokodev

@spokodev spokodev commented Sep 2, 2026

Copy link
Copy Markdown

JSONCookies() only writes the parsed value back when it is truthy (if (val)), so a cookie whose JSON payload is false, 0, null or "" is left as the raw j:-prefixed string instead of being inflated. The README states such values "will be exposed as the result of JSON.parse".

JSONCookie() already returns undefined only on parse failure, so guarding on val !== undefined preserves the "keep the original value on invalid JSON" behaviour while correctly parsing falsy values.

Refs #168.

Comment thread index.js Outdated
val = JSONCookie(obj[key])

if (val) {
if (val !== undefined) {

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[P1] Preserve the invalid-signature sentinel for signed JSON false. The middleware runs JSONCookies() over req.signedCookies after verification (lines 64–65), while signedCookie() and the README use boolean false to signal a failed signature. With this condition, a correctly signed j:false payload is also converted to boolean false. I reproduced both a valid cookie created with cookie-signature.sign('j:false', secret) and the same cookie with a corrupted signature; req.signedCookies.flag is false in both cases, so callers can no longer distinguish authentic data from signature failure—the exact ambiguity raised in issue #168. Please resolve that representation/API conflict before parsing false here, and add a middleware-level test covering valid j:false versus a tampered signed cookie.

Parsing "j:false" into boolean false made an authentically signed value
indistinguishable from a cookie whose signature did not verify, since
signedCookies() marks those with false (expressjs#168). The other falsy JSON values have
no such conflict and are still parsed.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@spokodev

spokodev commented Sep 7, 2026

Copy link
Copy Markdown
Author

Reproduced, and you were right — the PR as it stood introduced that ambiguity rather than inheriting it.

On master, an authentic j:false and a tampered cookie are distinguishable, because JSONCookies skipped every falsy parse:

authentic j:false  -> "j:false"  (string)
tampered signature -> false      (boolean)

With if (val !== undefined) both became boolean false, so req.signedCookies.flag could no longer tell the two apart. That is exactly #168, and turning an open representation question into a silent one in the verification path is not a trade worth making for this fix.

So the parse is now scoped to the falsy values that have no conflict — j:0, j:"" and j:null — and boolean false is left reserved for the signature marker, with j:false staying as its raw string until #168 settles what the marker should be. The condition carries a comment saying why, so it does not read as an oversight.

Added the middleware-level tests you asked for: an authentic j:false returns {"flag":"j:false"} and a tampered one returns {"flag":false}. The first fails against the previous commit. 37 passing, eslint clean.

If the maintainers would rather resolve #168 first — say, by marking failures with undefined or a symbol — then j:false can be parsed here too, and I will extend this.

@UlisesGascon UlisesGascon self-assigned this Sep 8, 2026
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.

3 participants