Skip to content

Fix charset decoding for text bodies - #754

Open
deepakganesh78 wants to merge 1 commit into
expressjs:masterfrom
deepakganesh78:fix/issue680-textdecoder-charsets
Open

Fix charset decoding for text bodies#754
deepakganesh78 wants to merge 1 commit into
expressjs:masterfrom
deepakganesh78:fix/issue680-textdecoder-charsets

Conversation

@deepakganesh78

Copy link
Copy Markdown

Fixes #680

Reproduction

A text/plain; charset=windows-1252 request containing bytes 80 81 82 83 8d 9e 9f decoded to €�‚ƒ�žŸ, replacing C1 control mappings at 0x81 and 0x8d. A text/plain; charset=utf-16le request containing two unpaired high surrogates decoded to a non-well-formed JavaScript string instead of replacement characters.

Root cause

raw-body/iconv-lite decoding is used for request character sets. For encodings covered by the WHATWG Encoding Standard, that can differ from the platform TextDecoder mappings and UTF-16 replacement behavior available in supported Node.js versions.

Fix

Read parser bodies as bytes, then decode with Node's WHATWG TextDecoder when it supports the charset. Keep iconv-lite as the fallback for legacy charsets that TextDecoder does not support, and keep generic utf-16 on the existing iconv-lite path to preserve current BOM-based behavior.

Compatibility notes

This intentionally changes decoded text for WHATWG-supported charset labels to match TextDecoder; unsupported labels and existing utf-16/utf-32 behavior continue through iconv-lite.

Validation

  • Regression tests fail without the fix: npx mocha --reporter spec --check-leaks test\text.js --grep "windows-1252|utf-16" -> 0 passing, 2 failing.
  • npm test -> 271 passing.
  • npm run lint -> passed.

Use Node's WHATWG TextDecoder for charsets it supports so text bodies follow Encoding Standard mappings, while keeping iconv-lite as the fallback for legacy encodings such as generic utf-16 and utf-32.

Add regression coverage for windows-1252 C1 controls and invalid UTF-16 surrogate pairs.

Fixes expressjs#680

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

@bjohansebas bjohansebas left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I actually recommend upgrading to the latest version of raw-body instead of using iconv-lite. It also lets you override the TextDecoder, as raw-body now does, in case anyone needs support for UTF-7 or other custom encodings.

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.

Almost all non-utf8 encodings are incorrect

2 participants