Skip to content

fix: accept a comma-delimited list in req.acceptsCharsets - #7451

Closed
marceli1404 wants to merge 1 commit into
expressjs:masterfrom
marceli1404:fix/accepts-charsets-comma-list
Closed

fix: accept a comma-delimited list in req.acceptsCharsets#7451
marceli1404 wants to merge 1 commit into
expressjs:masterfrom
marceli1404:fix/accepts-charsets-comma-list

Conversation

@marceli1404

Copy link
Copy Markdown

Bug

req.acceptsCharsets() silently ignores the documented comma-delimited form. The jsdoc (added by merged PR #6088) documents:

req.acceptsCharsets('utf-8, utf-16');
// => utf-8

...but the implementation passes the arguments straight through to accepts().charsets(...), which never splits commas. So a client following the documented API gets a hard false:

// with request header `Accept-Charset: iso-8859-1, utf-8`
req.acceptsCharsets('iso-8859-1, utf-8');   // => false   (bug; iso-8859-1 IS acceptable)
req.acceptsCharsets('iso-8859-1', 'utf-8'); // => iso-8859-1  (multi-arg works)

Fix

Expand comma-delimited args before passing them along, so req.acceptsCharsets('iso-8859-1, utf-8') behaves like the documented multi-arg form:

req.acceptsCharsets = function(...charsets) {
  const accept = accepts(this);
  return accept.charsets(...charsets.flatMap((charset) =>
    Array.isArray(charset) ? charset : String(charset).split(',').map((s) => s.trim())
  ));
};

Verification

  • New regression tests fail on the old code and pass on this change.
  • Existing behavior unchanged: no-arg (returns the acceptable list), single-arg, multi-arg, and array inputs are byte-identical.
  • npm test (mocha, full suite): 1262 passing, 0 failing.
  • eslint clean on the changed files.

@krzysdz

krzysdz commented Sep 2, 2026

Copy link
Copy Markdown
Contributor

As far as I can tell, the docs are wrong. Comma delimited string has not been supported since cec0c06, according to #6936 (comment). You'll find more details and an attempt at fixing the docs in #7452.

@krzysdz krzysdz closed this Sep 2, 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.

2 participants