Skip to content

fix: fall back to JSON when JSONP callback sanitizes to empty#7161

Open
andrewstellman wants to merge 1 commit intoexpressjs:masterfrom
andrewstellman:fix/jsonp-empty-callback
Open

fix: fall back to JSON when JSONP callback sanitizes to empty#7161
andrewstellman wants to merge 1 commit intoexpressjs:masterfrom
andrewstellman:fix/jsonp-empty-callback

Conversation

@andrewstellman
Copy link
Copy Markdown

Summary

res.jsonp() sanitizes the callback parameter by stripping disallowed characters (/[^\[\]\w$.]/g), but the JSONP branch is entered before sanitization. A callback consisting entirely of disallowed characters (e.g. !!!) passes the callback.length !== 0 check, sets Content-Type: text/javascript, then sanitizes to an empty string, producing malformed JavaScript:

/**/ typeof  === 'function' &&  ({"count":1});

This patch moves the sanitization step before the JSONP branch check so an all-invalid callback is reduced to empty first and falls through to the plain JSON path, returning valid application/json.

Changes

  • lib/response.js — Move callback.replace(…) before the callback.length !== 0 guard.
  • test/res.jsonp.js — Add regression test: ?callback=!!! expects application/json with valid JSON body.

Test plan

  • New test fails before the fix (text/javascript with malformed JS)
  • New test passes after the fix (application/json with {"count":1})
  • All 24 res.jsonp tests pass
  • Full Express test suite passes (1250 tests)

Move callback charset sanitization before the JSONP branch check so
that a callback consisting entirely of disallowed characters (e.g.
"!!!") is reduced to an empty string before the length test. Previously
the response entered the JSONP path, set Content-Type to
text/javascript, then sanitized the callback to empty, producing
malformed JavaScript:
  /**/ typeof  === 'function' &&  ({"count":1});

Now the sanitized-empty callback falls through to the plain JSON path
and returns valid application/json.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
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.

1 participant