Skip to content

fs: accept all valid utf8 values in fast paths - #62304

Closed
wdskuki wants to merge 2 commits into
nodejs:mainfrom
wdskuki:fix-utf8-fast-paths
Closed

fs: accept all valid utf8 values in fast paths#62304
wdskuki wants to merge 2 commits into
nodejs:mainfrom
wdskuki:fix-utf8-fast-paths

Conversation

@wdskuki

@wdskuki wdskuki commented Mar 18, 2026

Copy link
Copy Markdown

This PR fixes issue #49888 by accepting all valid UTF8 encoding variants (utf8, utf-8, UTF8, UTF-8) in fs.readFileSync and fs.writeFileSync fast paths.

Changes

  • Modified lib/fs.js to accept UTF8 and UTF-8 (uppercase) in addition to utf8 and utf-8
  • Added test case to verify the fix

Testing

  • Added test/parallel/test-fs-utf8-fast-path-casing.js

Fixes: #49888

Accept UTF8 and UTF-8 (uppercase) in fs.readFileSync and
fs.writeFileSync fast paths, in addition to utf8 and utf-8.

Fixes: nodejs#49888
@nodejs-github-bot nodejs-github-bot added fs Issues and PRs related to file-system APIs and the fs module. needs-ci PRs that need a full CI run. labels Mar 18, 2026

@mcollina mcollina 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.

lgtm

@jasnell jasnell 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.

LGTM modulo the linting issues in the test

@codecov

codecov Bot commented Mar 19, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 89.68%. Comparing base (9fc6b64) to head (7f98286).
⚠️ Report is 10 commits behind head on main.

Additional details and impacted files
@@           Coverage Diff           @@
##             main   #62304   +/-   ##
=======================================
  Coverage   89.68%   89.68%           
=======================================
  Files         676      676           
  Lines      206575   206577    +2     
  Branches    39549    39553    +4     
=======================================
+ Hits       185262   185274   +12     
+ Misses      13446    13445    -1     
+ Partials     7867     7858    -9     
Files with missing lines Coverage Δ
lib/fs.js 98.19% <100.00%> (+<0.01%) ⬆️

... and 34 files with indirect coverage changes

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

The fs.readFileSync and fs.writeFileSync fast paths for UTF-8 encoding
only accepted exact matches: 'utf8', 'utf-8', 'UTF8', 'UTF-8'. However,
Node.js considers all case-insensitive variations as valid UTF-8 (e.g.,
'Utf8', 'Utf-8', 'uTf8', etc.).

This commit adds an isUtf8Encoding() helper function that accepts all
valid UTF-8 encoding names, matching the behavior of normalizeEncoding()
in lib/internal/util.js.

Fixes nodejs#49888

@jazelly jazelly 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.

lgtm

@jazelly jazelly added the request-ci Add this label to start a Jenkins CI on a PR. Only starts once the PR has an approving review. label Mar 27, 2026
@github-actions github-actions Bot removed the request-ci Add this label to start a Jenkins CI on a PR. Only starts once the PR has an approving review. label Mar 27, 2026
@nodejs-github-bot

Copy link
Copy Markdown
Collaborator

@github-actions

Copy link
Copy Markdown
Contributor

This pull request has been marked as stale due to 90 days of inactivity.
It will be automatically closed in 30 days if no further activity occurs. If this is still relevant, please leave a comment or update it to keep it open.

@github-actions github-actions Bot added the stale Issues and PRs marked stale due to inactivity and scheduled for automatic closure. label Jul 28, 2026
@github-actions

Copy link
Copy Markdown
Contributor

This pull request has been automatically closed after 30 days of inactivity following its stale status (no activity for a total of 120 days).
If this is still relevant, feel free to reopen it or leave a comment with additional details so we can continue the discussion.

@github-actions github-actions Bot closed this Aug 27, 2026
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 file-system APIs and the fs module. needs-ci PRs that need a full CI run. stale Issues and PRs marked stale due to inactivity and scheduled for automatic closure.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

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

5 participants