Skip to content

fs: normalize encoding before utf8 fast path check#63695

Open
goingforstudying-ctrl wants to merge 1 commit into
nodejs:mainfrom
goingforstudying-ctrl:fix-utf8-fast-paths
Open

fs: normalize encoding before utf8 fast path check#63695
goingforstudying-ctrl wants to merge 1 commit into
nodejs:mainfrom
goingforstudying-ctrl:fix-utf8-fast-paths

Conversation

@goingforstudying-ctrl
Copy link
Copy Markdown

The fs.readFileSync and fs.writeFileSync fast paths for utf8 encoding only checked for 'utf8' and 'utf-8', but normalizeEncoding also accepts 'UTF8' and 'UTF-8'. This meant that using those encodings would bypass the fast path and go through the slower generic path.

Fix by normalizing the encoding before checking if it's utf8.

Fixes: #49888

The fs.readFileSync and fs.writeFileSync fast paths for utf8 encoding
only checked for 'utf8' and 'utf-8', but normalizeEncoding also accepts
'UTF8' and 'UTF-8'. This meant that using those encodings would bypass
the fast path and go through the slower generic path.

Fix by normalizing the encoding before checking if it's utf8.

Fixes: nodejs#49888
@nodejs-github-bot nodejs-github-bot added fs Issues and PRs related to the fs subsystem / file system. needs-ci PRs that need a full CI run. labels Jun 1, 2026
Comment thread lib/fs.js
Comment on lines +493 to +494
const encoding = normalizeEncoding(options.encoding) || options.encoding;
if (encoding === 'utf8') {
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.

Suggested change
const encoding = normalizeEncoding(options.encoding) || options.encoding;
if (encoding === 'utf8') {
if (options.encoding && normalizeEncoding(options.encoding) === 'utf8') {

user sended a real utf8 encoding only use fast path, otherwise we return string instead of a buffer, thats add a null condition better

Comment thread lib/fs.js
Comment on lines +2850 to +2851
const encoding = normalizeEncoding(options.encoding) || options.encoding;
if (typeof data === 'string' && encoding === 'utf8') {
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.

ditto

@lpinca
Copy link
Copy Markdown
Member

lpinca commented Jun 1, 2026

Duplicate of #60539 and #62304.

@goingforstudying-ctrl
Copy link
Copy Markdown
Author

Thanks for pointing that out, lpinca. I was not aware of the earlier PRs. I will close this one in favor of the existing efforts.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

fs Issues and PRs related to the fs subsystem / file system. needs-ci PRs that need a full CI run.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

fs: utf8 fast paths don't accept all valid utf8 values

4 participants