Skip to content

[v20.x] backport libuv wtf-8 decoding fix - #51976

Closed
richardlau wants to merge 2 commits into
nodejs:v20.x-stagingfrom
richardlau:surrogate
Closed

[v20.x] backport libuv wtf-8 decoding fix#51976
richardlau wants to merge 2 commits into
nodejs:v20.x-stagingfrom
richardlau:surrogate

Conversation

@richardlau

Copy link
Copy Markdown
Member

This backports libuv/libuv@d09441c to Node.js 20 to fix a long standing regression in the way filenames are handled on Windows. On Node.js 21 this was fixed by the update to libuv 1.47.0, but that version cannot be backported to Node.js 20 because libuv have dropped support for macOS <11 and we target macOS 10.15 for Node.js 20.

Refs: #48673

richardlauand others added 2 commits March 5, 2024 14:02
Original commit message:
fs: fix WTF-8 decoding issue (nodejs#4092)
We forgot to mask off the high bits from the first byte, so we ended up
always failing the subsequent range check.
Refs: libuv/libuv#2970Fixes: nodejs#48673
PR-URL: nodejs#51800Fixes: nodejs#51789
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
Reviewed-By: Yagiz Nizipli <yagiz.nizipli@sentry.io>
Reviewed-By: Marco Ippolito <marcoippolito54@gmail.com>
@nodejs-github-bot

Copy link
Copy Markdown
Collaborator

Review requested:

  • @nodejs/security-wg

@nodejs-github-botnodejs-github-bot added libuv Issues and PRs related to the libuv dependency or the uv binding. needs-ci PRs that need a full CI run. v20.x labels Mar 5, 2024
@richardlaurichardlau added the request-ci Add this label to start a Jenkins CI on a PR. label Mar 5, 2024
@github-actionsgithub-actionsBot removed the request-ci Add this label to start a Jenkins CI on a PR. label Mar 5, 2024
@nodejs-github-bot

Copy link
Copy Markdown
Collaborator

@nodejs-github-bot

Copy link
Copy Markdown
Collaborator

@UlisesGasconUlisesGascon 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!

@mertcanaltinmertcanaltin 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 🚀

richardlau added a commit that referenced this pull request Mar 15, 2024
Original commit message:
fs: fix WTF-8 decoding issue (#4092)
We forgot to mask off the high bits from the first byte, so we ended up
always failing the subsequent range check.
Refs: libuv/libuv#2970Fixes: #48673
PR-URL: #51976
Refs: #48673
Reviewed-By: Rafael Gonzaga <rafael.nunu@hotmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Marco Ippolito <marcoippolito54@gmail.com>
Reviewed-By: Santiago Gimeno <santiago.gimeno@gmail.com>
Reviewed-By: Ulises Gascón <ulisesgascongonzalez@gmail.com>
richardlau pushed a commit that referenced this pull request Mar 15, 2024
PR-URL: #51800
Backport-PR-URL: #51976Fixes: #51789
Refs: #48673
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
Reviewed-By: Yagiz Nizipli <yagiz.nizipli@sentry.io>
Reviewed-By: Marco Ippolito <marcoippolito54@gmail.com>
@richardlau

Copy link
Copy Markdown
MemberAuthor

Landed in ea50540...e8f5735.

@richardlau
richardlau deleted the surrogate branch March 15, 2024 17:23
@richardlau

Copy link
Copy Markdown
MemberAuthor

FYI I'm aiming to do a Node.js 20 release around 26 March 2024 which will include this.

@richardlaurichardlau mentioned this pull request Mar 25, 2024
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

libuvIssues and PRs related to the libuv dependency or the uv binding.needs-ciPRs that need a full CI run.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

9 participants

@richardlau@nodejs-github-bot@santigimeno@lpinca@UlisesGascon@StefanStojanovic@RafaelGSS@marco-ippolito@mertcanaltin