Skip to content

Commit bd0ec90

Browse files
aduh95injunchoi98
authored andcommitted
url: handle "unsafe" characters properly in pathToFileURL
Co-authored-by: EarlyRiser42 <tkfydtls464@gmail.com> PR-URL: #54545 Fixes: #54515 Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
1 parent d9fd632 commit bd0ec90

File tree

3 files changed

+67
-42
lines changed

3 files changed

+67
-42
lines changed

lib/internal/process/execution.js

+1-1
Original file line numberDiff line numberDiff line change
@@ -49,7 +49,7 @@ function tryGetCwd() {
4949

5050
let evalIndex = 0;
5151
function getEvalModuleUrl() {
52-
return pathToFileURL(`${process.cwd()}/[eval${++evalIndex}]`).href;
52+
return `${pathToFileURL(process.cwd())}/[eval${++evalIndex}]`;
5353
}
5454

5555
/**

lib/internal/url.js

+57-37
Original file line numberDiff line numberDiff line change
@@ -1490,44 +1490,75 @@ function fileURLToPath(path, options = kEmptyObject) {
14901490
return (windows ?? isWindows) ? getPathFromURLWin32(path) : getPathFromURLPosix(path);
14911491
}
14921492

1493-
// The following characters are percent-encoded when converting from file path
1494-
// to URL:
1495-
// - %: The percent character is the only character not encoded by the
1496-
// `pathname` setter.
1497-
// - \: Backslash is encoded on non-windows platforms since it's a valid
1498-
// character but the `pathname` setters replaces it by a forward slash.
1499-
// - LF: The newline character is stripped out by the `pathname` setter.
1500-
// (See whatwg/url#419)
1501-
// - CR: The carriage return character is also stripped out by the `pathname`
1502-
// setter.
1503-
// - TAB: The tab character is also stripped out by the `pathname` setter.
1493+
// RFC1738 defines the following chars as "unsafe" for URLs
1494+
// @see https://www.ietf.org/rfc/rfc1738.txt 2.2. URL Character Encoding Issues
15041495
const percentRegEx = /%/g;
1505-
const backslashRegEx = /\\/g;
15061496
const newlineRegEx = /\n/g;
15071497
const carriageReturnRegEx = /\r/g;
15081498
const tabRegEx = /\t/g;
1509-
const questionRegex = /\?/g;
1499+
const quoteRegEx = /"/g;
15101500
const hashRegex = /#/g;
1501+
const spaceRegEx = / /g;
1502+
const questionMarkRegex = /\?/g;
1503+
const openSquareBracketRegEx = /\[/g;
1504+
const backslashRegEx = /\\/g;
1505+
const closeSquareBracketRegEx = /]/g;
1506+
const caretRegEx = /\^/g;
1507+
const verticalBarRegEx = /\|/g;
1508+
const tildeRegEx = /~/g;
15111509

15121510
function encodePathChars(filepath, options = kEmptyObject) {
1513-
const windows = options?.windows;
1514-
if (StringPrototypeIndexOf(filepath, '%') !== -1)
1511+
if (StringPrototypeIncludes(filepath, '%')) {
15151512
filepath = RegExpPrototypeSymbolReplace(percentRegEx, filepath, '%25');
1516-
// In posix, backslash is a valid character in paths:
1517-
if (!(windows ?? isWindows) && StringPrototypeIndexOf(filepath, '\\') !== -1)
1518-
filepath = RegExpPrototypeSymbolReplace(backslashRegEx, filepath, '%5C');
1519-
if (StringPrototypeIndexOf(filepath, '\n') !== -1)
1513+
}
1514+
1515+
if (StringPrototypeIncludes(filepath, '\t')) {
1516+
filepath = RegExpPrototypeSymbolReplace(tabRegEx, filepath, '%09');
1517+
}
1518+
if (StringPrototypeIncludes(filepath, '\n')) {
15201519
filepath = RegExpPrototypeSymbolReplace(newlineRegEx, filepath, '%0A');
1521-
if (StringPrototypeIndexOf(filepath, '\r') !== -1)
1520+
}
1521+
if (StringPrototypeIncludes(filepath, '\r')) {
15221522
filepath = RegExpPrototypeSymbolReplace(carriageReturnRegEx, filepath, '%0D');
1523-
if (StringPrototypeIndexOf(filepath, '\t') !== -1)
1524-
filepath = RegExpPrototypeSymbolReplace(tabRegEx, filepath, '%09');
1523+
}
1524+
if (StringPrototypeIncludes(filepath, ' ')) {
1525+
filepath = RegExpPrototypeSymbolReplace(spaceRegEx, filepath, '%20');
1526+
}
1527+
if (StringPrototypeIncludes(filepath, '"')) {
1528+
filepath = RegExpPrototypeSymbolReplace(quoteRegEx, filepath, '%22');
1529+
}
1530+
if (StringPrototypeIncludes(filepath, '#')) {
1531+
filepath = RegExpPrototypeSymbolReplace(hashRegex, filepath, '%23');
1532+
}
1533+
if (StringPrototypeIncludes(filepath, '?')) {
1534+
filepath = RegExpPrototypeSymbolReplace(questionMarkRegex, filepath, '%3F');
1535+
}
1536+
if (StringPrototypeIncludes(filepath, '[')) {
1537+
filepath = RegExpPrototypeSymbolReplace(openSquareBracketRegEx, filepath, '%5B');
1538+
}
1539+
// Back-slashes must be special-cased on Windows, where they are treated as path separator.
1540+
if (!options.windows && StringPrototypeIncludes(filepath, '\\')) {
1541+
filepath = RegExpPrototypeSymbolReplace(backslashRegEx, filepath, '%5C');
1542+
}
1543+
if (StringPrototypeIncludes(filepath, ']')) {
1544+
filepath = RegExpPrototypeSymbolReplace(closeSquareBracketRegEx, filepath, '%5D');
1545+
}
1546+
if (StringPrototypeIncludes(filepath, '^')) {
1547+
filepath = RegExpPrototypeSymbolReplace(caretRegEx, filepath, '%5E');
1548+
}
1549+
if (StringPrototypeIncludes(filepath, '|')) {
1550+
filepath = RegExpPrototypeSymbolReplace(verticalBarRegEx, filepath, '%7C');
1551+
}
1552+
if (StringPrototypeIncludes(filepath, '~')) {
1553+
filepath = RegExpPrototypeSymbolReplace(tildeRegEx, filepath, '%7E');
1554+
}
1555+
15251556
return filepath;
15261557
}
15271558

15281559
function pathToFileURL(filepath, options = kEmptyObject) {
1529-
const windows = options?.windows;
1530-
if ((windows ?? isWindows) && StringPrototypeStartsWith(filepath, '\\\\')) {
1560+
const windows = options?.windows ?? isWindows;
1561+
if (windows && StringPrototypeStartsWith(filepath, '\\\\')) {
15311562
const outURL = new URL('file://');
15321563
// UNC path format: \\server\share\resource
15331564
// Handle extended UNC path and standard UNC path
@@ -1558,7 +1589,7 @@ function pathToFileURL(filepath, options = kEmptyObject) {
15581589
);
15591590
return outURL;
15601591
}
1561-
let resolved = (windows ?? isWindows) ? path.win32.resolve(filepath) : path.posix.resolve(filepath);
1592+
let resolved = windows ? path.win32.resolve(filepath) : path.posix.resolve(filepath);
15621593
// path.resolve strips trailing slashes so we must add them back
15631594
const filePathLast = StringPrototypeCharCodeAt(filepath,
15641595
filepath.length - 1);
@@ -1567,18 +1598,7 @@ function pathToFileURL(filepath, options = kEmptyObject) {
15671598
resolved[resolved.length - 1] !== path.sep)
15681599
resolved += '/';
15691600

1570-
// Call encodePathChars first to avoid encoding % again for ? and #.
1571-
resolved = encodePathChars(resolved, { windows });
1572-
1573-
// Question and hash character should be included in pathname.
1574-
// Therefore, encoding is required to eliminate parsing them in different states.
1575-
// This is done as an optimization to not creating a URL instance and
1576-
// later triggering pathname setter, which impacts performance
1577-
if (StringPrototypeIndexOf(resolved, '?') !== -1)
1578-
resolved = RegExpPrototypeSymbolReplace(questionRegex, resolved, '%3F');
1579-
if (StringPrototypeIndexOf(resolved, '#') !== -1)
1580-
resolved = RegExpPrototypeSymbolReplace(hashRegex, resolved, '%23');
1581-
return new URL(`file://${resolved}`);
1601+
return new URL(`file://${encodePathChars(resolved, { windows })}`);
15821602
}
15831603

15841604
function toPathIfFileURL(fileURLOrPath) {

test/parallel/test-url-pathtofileurl.js

+9-4
Original file line numberDiff line numberDiff line change
@@ -13,10 +13,7 @@ const url = require('url');
1313
{
1414
const fileURL = url.pathToFileURL('test\\').href;
1515
assert.ok(fileURL.startsWith('file:///'));
16-
if (isWindows)
17-
assert.ok(fileURL.endsWith('/'));
18-
else
19-
assert.ok(fileURL.endsWith('%5C'));
16+
assert.match(fileURL, isWindows ? /\/$/ : /%5C$/);
2017
}
2118

2219
{
@@ -104,6 +101,12 @@ const windowsTestCases = [
104101
{ path: 'C:\\€', expected: 'file:///C:/%E2%82%AC' },
105102
// Rocket emoji (non-BMP code point)
106103
{ path: 'C:\\🚀', expected: 'file:///C:/%F0%9F%9A%80' },
104+
// caret
105+
{ path: 'C:\\foo^bar', expected: 'file:///C:/foo%5Ebar' },
106+
// left bracket
107+
{ path: 'C:\\foo[bar', expected: 'file:///C:/foo%5Bbar' },
108+
// right bracket
109+
{ path: 'C:\\foo]bar', expected: 'file:///C:/foo%5Dbar' },
107110
// Local extended path
108111
{ path: '\\\\?\\C:\\path\\to\\file.txt', expected: 'file:///C:/path/to/file.txt' },
109112
// UNC path (see https://docs.microsoft.com/en-us/archive/blogs/ie/file-uris-in-windows)
@@ -154,6 +157,8 @@ const posixTestCases = [
154157
{ path: '/€', expected: 'file:///%E2%82%AC' },
155158
// Rocket emoji (non-BMP code point)
156159
{ path: '/🚀', expected: 'file:///%F0%9F%9A%80' },
160+
// "unsafe" chars
161+
{ path: '/foo\r\n\t<>"#%{}|^[\\~]`?bar', expected: 'file:///foo%0D%0A%09%3C%3E%22%23%25%7B%7D%7C%5E%5B%5C%7E%5D%60%3Fbar' },
157162
];
158163

159164
for (const { path, expected } of windowsTestCases) {

0 commit comments

Comments
 (0)