-
-
Notifications
You must be signed in to change notification settings - Fork 31.9k
url: add fileURLToPathBuffer API #58700
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
Review requested:
|
This comment was marked as outdated.
This comment was marked as outdated.
249ab0d
to
8ccd48d
Compare
This comment was marked as outdated.
This comment was marked as outdated.
8ccd48d
to
803aa96
Compare
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #58700 +/- ##
==========================================
- Coverage 90.16% 90.12% -0.04%
==========================================
Files 637 637
Lines 188098 188122 +24
Branches 36905 36898 -7
==========================================
- Hits 169601 169548 -53
- Misses 11231 11319 +88
+ Partials 7266 7255 -11
🚀 New features to boost your workflow:
|
803aa96
to
cb63900
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
lgtm
what could we do with this buffer? How could a user manipulate this?
cb63900
to
755e985
Compare
It can be passed directly to |
This comment was marked as outdated.
This comment was marked as outdated.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I wont block on my one change recommendation, but please consider it in regards to consistency with path.win32
and path.posix
pattern.
The existing `fileURLToPath()` does not handle the case where the input URL contains percent-encoded characters that are not valid UTF-8 sequences. This can lead to issues, for instance, when the URL is constructed using file names in non-Unicode encodings (like Shift-JIS). This commit introduces a new API, `fileURLToPathBuffer()`, which returns a `Buffer` representing the path, allowing for accurate conversion of file URLs to paths without attempting to decode the percent-encoded bytes into characters.
755e985
to
e362a1e
Compare
This comment was marked as outdated.
This comment was marked as outdated.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Given that this is more robust, should internals that rely on fileURLToPath()
be eventually migrated to using this function?
Probably. |
Commit Queue failed- Loading data for nodejs/node/pull/58700 ✔ Done loading data for nodejs/node/pull/58700 ----------------------------------- PR info ------------------------------------ Title url: add fileURLToPathBuffer API (#58700) Author James M Snell <jasnell@gmail.com> (@jasnell) Branch jasnell:jasnell/add-fileurltopathbuffer -> nodejs:main Labels fs, url, semver-minor, whatwg-url, author ready, needs-ci Commits 1 - url: add fileURLToPathBuffer API Committers 1 - James M Snell <jasnell@gmail.com> PR-URL: https://github.com/nodejs/node/pull/58700 Reviewed-By: Matteo Collina <matteo.collina@gmail.com> Reviewed-By: Ethan Arrowood <ethan@arrowood.dev> Reviewed-By: LiviaMedeiros <livia@cirno.name> ------------------------------ Generated metadata ------------------------------ PR-URL: https://github.com/nodejs/node/pull/58700 Reviewed-By: Matteo Collina <matteo.collina@gmail.com> Reviewed-By: Ethan Arrowood <ethan@arrowood.dev> Reviewed-By: LiviaMedeiros <livia@cirno.name> -------------------------------------------------------------------------------- ℹ This PR was created on Thu, 12 Jun 2025 20:49:22 GMT ✔ Approvals: 3 ✔ - Matteo Collina (@mcollina) (TSC): https://github.com/nodejs/node/pull/58700#pullrequestreview-2927770869 ✔ - Ethan Arrowood (@Ethan-Arrowood): https://github.com/nodejs/node/pull/58700#pullrequestreview-2928406839 ✔ - LiviaMedeiros (@LiviaMedeiros): https://github.com/nodejs/node/pull/58700#pullrequestreview-2928714008 ✘ Last GitHub CI failed ℹ Last Full PR CI on 2025-06-14T19:33:13Z: https://ci.nodejs.org/job/node-test-pull-request/67444/ - Querying data for job/node-test-pull-request/67444/ ✔ Last Jenkins CI successful -------------------------------------------------------------------------------- ✔ Aborted `git node land` session in /home/runner/work/node/node/.ncuhttps://github.com/nodejs/node/actions/runs/15656016109 |
The existing `fileURLToPath()` does not handle the case where the input URL contains percent-encoded characters that are not valid UTF-8 sequences. This can lead to issues, for instance, when the URL is constructed using file names in non-Unicode encodings (like Shift-JIS). This commit introduces a new API, `fileURLToPathBuffer()`, which returns a `Buffer` representing the path, allowing for accurate conversion of file URLs to paths without attempting to decode the percent-encoded bytes into characters. PR-URL: #58700 Reviewed-By: Matteo Collina <matteo.collina@gmail.com> Reviewed-By: Ethan Arrowood <ethan@arrowood.dev> Reviewed-By: LiviaMedeiros <livia@cirno.name>
Landed in 3f6ad56 |
The existing `fileURLToPath()` does not handle the case where the input URL contains percent-encoded characters that are not valid UTF-8 sequences. This can lead to issues, for instance, when the URL is constructed using file names in non-Unicode encodings (like Shift-JIS). This commit introduces a new API, `fileURLToPathBuffer()`, which returns a `Buffer` representing the path, allowing for accurate conversion of file URLs to paths without attempting to decode the percent-encoded bytes into characters. PR-URL: #58700 Reviewed-By: Matteo Collina <matteo.collina@gmail.com> Reviewed-By: Ethan Arrowood <ethan@arrowood.dev> Reviewed-By: LiviaMedeiros <livia@cirno.name>
Notable changes: doc: * add islandryu to collaborators (Shima Ryuhei) #58714 fs: * (SEMVER-MINOR) allow correct handling of burst in fs-events with AsyncIterator (Philipp Dunkel) #58490 module: * (SEMVER-MINOR) remove experimental warning from type stripping (Marco Ippolito) #58643 test_runner: * (SEMVER-MINOR) support object property mocking (Idan Goshen) #58438 url: * (SEMVER-MINOR) add fileURLToPathBuffer API (James M Snell) #58700 PR-URL: #58727
The existing
fileURLToPath()
does not handle the case where the input URL contains percent-encoded characters that are not valid UTF-8 sequences. This can lead to issues, for instance, when the URL is constructed using file names in non-Unicode encodings (like Shift-JIS). This commit introduces a new API,fileURLToPathBuffer()
, which returns aBuffer
representing the path, allowing for accurate conversion of file URLs to paths without attempting to decode the percent-encoded bytes into characters.This is part of the fix for #58634 but there's more to do.