You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
{{ message }}
Repository navigation
Replace deprecated url.parse in uriToFilePath with WHATWG URL - #1926
The reason will be displayed to describe this comment to others. Learn more.
🟡 Changes recommended
The URI extraction regresses Windows file URI handling by skipping preprocessing, causing literal backslashes and surrounding whitespace to produce undefined results or incorrect filenames.
The reason will be displayed to describe this comment to others. Learn more.
🔵 Needs a closer look
Authority-only file URIs incorrectly return undefined instead of preserving their implicit root path, causing a regression for valid inputs such as file://localhost.
No open issues 🎉
1 previously missed issue
In code that hasn't changed since last review
Preserve implicit root path for authority-only file URIs
server/src/node/files.ts:28
Authority-only file URIs lose their implicit root path here. For example, file://localhost and file://localhost#fragment previously returned / on POSIX, but this regex captures an empty path and the following guard returns undefined, even though new URL() accepts both with pathname /. Preserve the implicit / for a nonempty authority while continuing to use the raw path when one is present.
Authority-only file URIs incorrectly return undefined instead of preserving their implicit root path, causing a regression for valid inputs such as file://localhost.
Use `new URL(uri).pathname` instead of `url.parse(uri).path`. Input
that URL can't parse still returns undefined. Query strings are no
longer included in the returned file path.
URL.pathname resolves '..' segments before percent-decoding, so
file:///tmp/one%2Ftwo/../three.ts mapped to /tmp/three.ts instead of
/tmp/one/three.ts. Take the raw path from the URI string so segments
are decoded before path.normalize, as url.parse did.
Taking the raw path from the URI string skipped preprocessing that
url.parse did: file://localhost\C:\work\a.ts returned undefined,
leading whitespace returned undefined, and trailing whitespace stayed
in the path. Trim the input and convert backslashes to forward slashes
before extracting the path.
When a file URI has an authority but no path, such as file://localhost
or file://localhost#frag, return '/' as url.parse did. Previously the
raw path regex captured an empty string and returned undefined. URIs
with an empty authority (file://, file:) still return undefined.
String.prototype.trim() strips Unicode whitespace such as U+2003 that
url.parse kept as part of the filename, and leaves C0 control
characters that url.parse removed. Trim only C0 controls, space,
U+00A0 and U+FEFF so the resolved paths match the old behavior.
The reason will be displayed to describe this comment to others. Learn more.
🟡 Changes recommended
The URL constructor runs before legacy-compatible trimming, causing file URLs with leading U+00A0 or U+FEFF to return undefined instead of their previously resolved paths.
WHATWG URL does not trim leading U+00A0 or U+FEFF, so inputs such as
"\u00A0file:///tmp/a.ts" threw and returned undefined, while url.parse
returned /tmp/a.ts. Trim and convert backslashes before constructing
the URL, and use the same normalized string for the raw path.
The reason will be displayed to describe this comment to others. Learn more.
🔵 Needs a closer look
Human assessment is needed to weigh unpublished findings involving changed file-URI path semantics and quadratic whitespace normalization that can block the server’s event loop.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
url.parseis deprecated in Node.js. This PR replaces it inFiles.uriToFilePathwith the WHATWGURLAPI.new URL(uri).pathnameinstead ofurl.parse(uri).path. The rest of the decoding and Windows drive-letter handling stays the same.URLcan't parse the input, the function returnsundefined, as it did before.file:///a/b.ts?q=1now gives/a/b.ts.There are no existing tests for
uriToFilePath, and none were added since the function is deprecated.