Skip to content

Commit 5c88dc3

Browse files
anonriganonrig
authored
apply latest changes to node.js to fix path (#2964)
1 parent b387422 commit 5c88dc3

File tree

4 files changed

+180
-122
lines changed

4 files changed

+180
-122
lines changed

‎src/node/internal/internal_path.ts‎

Lines changed: 24 additions & 74 deletions
Original file line numberDiff line numberDiff line change
@@ -157,12 +157,11 @@ const win32 = {
157157
let resolvedDevice = '';
158158
let resolvedTail = '';
159159
let resolvedAbsolute = false;
160-
let slashCheck = false;
161160

162161
for (let i = args.length - 1; i >= -1; i--) {
163-
let path: string;
162+
let path;
164163
if (i >= 0) {
165-
path = args[i] as string;
164+
path = args[i];
166165
validateString(path, `paths[${i}]`);
167166

168167
// Skip empty entries
@@ -189,12 +188,6 @@ const win32 = {
189188
}
190189
}
191190

192-
if (
193-
i === args.length - 1 &&
194-
isPathSeparator(path.charCodeAt(path.length - 1))
195-
) {
196-
slashCheck = true;
197-
}
198191
const len = path.length;
199192
let rootEnd = 0;
200193
let device = '';
@@ -239,15 +232,9 @@ const win32 = {
239232
j++;
240233
}
241234
if (j === len || j !== last) {
242-
if (firstPart !== '.' && firstPart !== '?') {
243-
// We matched a UNC root
244-
device = `\\\\${firstPart}\\${path.slice(last, j)}`;
245-
rootEnd = j;
246-
} else {
247-
// We matched a device root (e.g. \\\\.\\PHYSICALDRIVE0)
248-
device = `\\\\${firstPart}`;
249-
rootEnd = 4;
250-
}
235+
// We matched a UNC root
236+
device = `\\\\${firstPart}\\${path.slice(last, j)}`;
237+
rootEnd = j;
251238
}
252239
}
253240
}
@@ -302,21 +289,9 @@ const win32 = {
302289
isPathSeparator
303290
);
304291

305-
if (!resolvedAbsolute) {
306-
return `${resolvedDevice}${resolvedTail}` || '.';
307-
}
308-
309-
if (resolvedTail.length === 0) {
310-
return slashCheck ? `${resolvedDevice}\\` : resolvedDevice;
311-
}
312-
313-
if (slashCheck) {
314-
return resolvedTail === '\\'
315-
? `${resolvedDevice}\\`
316-
: `${resolvedDevice}\\${resolvedTail}\\`;
317-
}
318-
319-
return `${resolvedDevice}\\${resolvedTail}`;
292+
return resolvedAbsolute
293+
? `${resolvedDevice}\\${resolvedTail}`
294+
: `${resolvedDevice}${resolvedTail}` || '.';
320295
},
321296

322297
normalize(path: string): string {
@@ -364,21 +339,16 @@ const win32 = {
364339
while (j < len && !isPathSeparator(path.charCodeAt(j))) {
365340
j++;
366341
}
367-
if (j === len || j !== last) {
368-
if (firstPart === '.' || firstPart === '?') {
369-
// We matched a device root (e.g. \\\\.\\PHYSICALDRIVE0)
370-
device = `\\\\${firstPart}`;
371-
rootEnd = 4;
372-
} else if (j === len) {
373-
// We matched a UNC root only
374-
// Return the normalized version of the UNC root since there
375-
// is nothing left to process
376-
return `\\\\${firstPart}\\${path.slice(last)}\\`;
377-
} else {
378-
// We matched a UNC root with leftovers
379-
device = `\\\\${firstPart}\\${path.slice(last, j)}`;
380-
rootEnd = j;
381-
}
342+
if (j === len) {
343+
// We matched a UNC root only
344+
// Return the normalized version of the UNC root since there
345+
// is nothing left to process
346+
return `\\\\${firstPart}\\${path.slice(last)}\\`;
347+
}
348+
if (j !== last) {
349+
// We matched a UNC root with leftovers
350+
device = `\\\\${firstPart}\\${path.slice(last, j)}`;
351+
rootEnd = j;
382352
}
383353
}
384354
}
@@ -1071,7 +1041,6 @@ const posix = {
10711041
resolve(...args: string[]): string {
10721042
let resolvedPath = '';
10731043
let resolvedAbsolute = false;
1074-
let slashCheck = false;
10751044

10761045
for (let i = args.length - 1; i >= 0 && !resolvedAbsolute; i--) {
10771046
const path = args[i];
@@ -1081,25 +1050,15 @@ const posix = {
10811050
if (path.length === 0) {
10821051
continue;
10831052
}
1084-
if (
1085-
i === args.length - 1 &&
1086-
isPosixPathSeparator(path.charCodeAt(path.length - 1))
1087-
) {
1088-
slashCheck = true;
1089-
}
10901053

1091-
if (resolvedPath.length !== 0) {
1092-
resolvedPath = `${path}/${resolvedPath}`;
1093-
} else {
1094-
resolvedPath = path;
1095-
}
1054+
resolvedPath = `${path}/${resolvedPath}`;
10961055
resolvedAbsolute = path.charCodeAt(0) === CHAR_FORWARD_SLASH;
10971056
}
10981057

10991058
if (!resolvedAbsolute) {
11001059
const cwd = '/';
11011060
resolvedPath = `${cwd}/${resolvedPath}`;
1102-
resolvedAbsolute = cwd.charCodeAt(0) === CHAR_FORWARD_SLASH;
1061+
resolvedAbsolute = true;
11031062
}
11041063

11051064
// At this point the path should be resolved to a full absolute path, but
@@ -1113,20 +1072,11 @@ const posix = {
11131072
isPosixPathSeparator
11141073
);
11151074

1116-
if (!resolvedAbsolute) {
1117-
if (resolvedPath.length === 0) {
1118-
return '.';
1119-
}
1120-
if (slashCheck) {
1121-
return `${resolvedPath}/`;
1122-
}
1123-
return resolvedPath;
1124-
}
1125-
1126-
if (resolvedPath.length === 0 || resolvedPath === '/') {
1127-
return '/';
1075+
// eslint-disable-next-line @typescript-eslint/no-unnecessary-condition
1076+
if (resolvedAbsolute) {
1077+
return `/${resolvedPath}`;
11281078
}
1129-
return slashCheck ? `/${resolvedPath}/` : `/${resolvedPath}`;
1079+
return resolvedPath.length > 0 ? resolvedPath : '.';
11301080
},
11311081

11321082
/**

‎src/node/internal/internal_url.ts‎

Lines changed: 70 additions & 36 deletions
Original file line numberDiff line numberDiff line change
@@ -11,49 +11,85 @@ import {
1111
ERR_INVALID_URL_SCHEME,
1212
} from 'node-internal:internal_errors';
1313
import { default as urlUtil } from 'node-internal:url';
14-
import { CHAR_LOWERCASE_A, CHAR_LOWERCASE_Z } from 'node-internal:constants';
14+
import {
15+
CHAR_LOWERCASE_A,
16+
CHAR_LOWERCASE_Z,
17+
CHAR_FORWARD_SLASH,
18+
CHAR_BACKWARD_SLASH,
19+
} from 'node-internal:constants';
1520
import {
1621
win32 as pathWin32,
1722
posix as pathPosix,
1823
} from 'node-internal:internal_path';
1924

2025
const FORWARD_SLASH = /\//g;
2126

22-
// The following characters are percent-encoded when converting from file path
23-
// to URL:
24-
// - %: The percent character is the only character not encoded by the
25-
// `pathname` setter.
26-
// - \: Backslash is encoded on non-windows platforms since it's a valid
27-
// character but the `pathname` setters replaces it by a forward slash.
28-
// - LF: The newline character is stripped out by the `pathname` setter.
29-
// (See whatwg/url#419)
30-
// - CR: The carriage return character is also stripped out by the `pathname`
31-
// setter.
32-
// - TAB: The tab character is also stripped out by the `pathname` setter.
27+
// RFC1738 defines the following chars as "unsafe" for URLs
28+
// @see https://www.ietf.org/rfc/rfc1738.txt 2.2. URL Character Encoding Issues
3329
const percentRegEx = /%/g;
34-
const backslashRegEx = /\\/g;
3530
const newlineRegEx = /\n/g;
3631
const carriageReturnRegEx = /\r/g;
3732
const tabRegEx = /\t/g;
38-
const questionRegex = /\?/g;
33+
const quoteRegEx = /"/g;
3934
const hashRegex = /#/g;
35+
const spaceRegEx = / /g;
36+
const questionMarkRegex = /\?/g;
37+
const openSquareBracketRegEx = /\[/g;
38+
const backslashRegEx = /\\/g;
39+
const closeSquareBracketRegEx = /]/g;
40+
const caretRegEx = /\^/g;
41+
const verticalBarRegEx = /\|/g;
42+
const tildeRegEx = /~/g;
4043

4144
function encodePathChars(
4245
filepath: string,
43-
options?: { windows?: boolean | undefined }
46+
options?: { windows: boolean | undefined }
4447
): string {
45-
const windows = options?.windows;
46-
if (filepath.indexOf('%') !== -1)
48+
if (filepath.includes('%')) {
4749
filepath = filepath.replace(percentRegEx, '%25');
48-
// In posix, backslash is a valid character in paths:
49-
if (!windows && filepath.indexOf('\\') !== -1)
50-
filepath = filepath.replace(backslashRegEx, '%5C');
51-
if (filepath.indexOf('\n') !== -1)
50+
}
51+
52+
if (filepath.includes('\t')) {
53+
filepath = filepath.replace(tabRegEx, '%09');
54+
}
55+
if (filepath.includes('\n')) {
5256
filepath = filepath.replace(newlineRegEx, '%0A');
53-
if (filepath.indexOf('\r') !== -1)
57+
}
58+
if (filepath.includes('\r')) {
5459
filepath = filepath.replace(carriageReturnRegEx, '%0D');
55-
if (filepath.indexOf('\t') !== -1)
56-
filepath = filepath.replace(tabRegEx, '%09');
60+
}
61+
if (filepath.includes(' ')) {
62+
filepath = filepath.replace(spaceRegEx, '%20');
63+
}
64+
if (filepath.includes('"')) {
65+
filepath = filepath.replace(quoteRegEx, '%22');
66+
}
67+
if (filepath.includes('#')) {
68+
filepath = filepath.replace(hashRegex, '%23');
69+
}
70+
if (filepath.includes('?')) {
71+
filepath = filepath.replace(questionMarkRegex, '%3F');
72+
}
73+
if (filepath.includes('[')) {
74+
filepath = filepath.replace(openSquareBracketRegEx, '%5B');
75+
}
76+
// Back-slashes must be special-cased on Windows, where they are treated as path separator.
77+
if (!options?.windows && filepath.includes('\\')) {
78+
filepath = filepath.replace(backslashRegEx, '%5C');
79+
}
80+
if (filepath.includes(']')) {
81+
filepath = filepath.replace(closeSquareBracketRegEx, '%5D');
82+
}
83+
if (filepath.includes('^')) {
84+
filepath = filepath.replace(caretRegEx, '%5E');
85+
}
86+
if (filepath.includes('|')) {
87+
filepath = filepath.replace(verticalBarRegEx, '%7C');
88+
}
89+
if (filepath.includes('~')) {
90+
filepath = filepath.replace(tildeRegEx, '%7E');
91+
}
92+
5793
return filepath;
5894
}
5995

@@ -205,19 +241,17 @@ export function pathToFileURL(
205241
let resolved = windows
206242
? pathWin32.resolve(filepath)
207243
: pathPosix.resolve(filepath);
244+
const sep = windows ? pathWin32.sep : pathPosix.sep;
245+
// path.resolve strips trailing slashes so we must add them back
246+
const filePathLast = filepath.charCodeAt(filepath.length - 1);
247+
if (
248+
(filePathLast === CHAR_FORWARD_SLASH ||
249+
(windows && filePathLast === CHAR_BACKWARD_SLASH)) &&
250+
resolved[resolved.length - 1] !== sep
251+
)
252+
resolved += '/';
208253

209-
// Call encodePathChars first to avoid encoding % again for ? and #.
210-
resolved = encodePathChars(resolved, { windows });
211-
212-
// Question and hash character should be included in pathname.
213-
// Therefore, encoding is required to eliminate parsing them in different states.
214-
// This is done as an optimization to not creating a URL instance and
215-
// later triggering pathname setter, which impacts performance
216-
if (resolved.indexOf('?') !== -1)
217-
resolved = resolved.replace(questionRegex, '%3F');
218-
if (resolved.indexOf('#') !== -1)
219-
resolved = resolved.replace(hashRegex, '%23');
220-
return new URL(`file://${resolved}`);
254+
return new URL(`file://${encodePathChars(resolved, { windows })}`);
221255
}
222256

223257
export function toPathIfFileURL(fileURLOrPath: URL | string): string {

‎src/workerd/api/node/tests/path-test.js‎

Lines changed: 52 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -107,11 +107,11 @@ export const test_path_resolve = {
107107
const posixyCwd = '/';
108108

109109
const resolveTests = [
110-
[['/var/lib', '../', 'file/'], '/var/file/'],
111-
[['/var/lib', '/../', 'file/'], '/file/'],
110+
[['/var/lib', '../', 'file/'], '/var/file'],
111+
[['/var/lib', '/../', 'file/'], '/file'],
112112
[['a/b/c/', '../../..'], posixyCwd],
113113
[['.'], posixyCwd],
114-
[['/some/dir', '.', '/absolute/'], '/absolute/'],
114+
[['/some/dir', '.', '/absolute/'], '/absolute'],
115115
[['/foo/tmp.3/', '../tmp.3/cycles/root.js'], '/foo/tmp.3/cycles/root.js'],
116116
];
117117
resolveTests.forEach(([test, expected]) => {
@@ -305,6 +305,55 @@ export const test_path_parse_format = {
305305

306306
export const test_path_normalize = {
307307
test(ctrl, env, ctx) {
308+
strictEqual(
309+
path.win32.normalize('./fixtures///b/../b/c.js'),
310+
'fixtures\\b\\c.js'
311+
);
312+
strictEqual(path.win32.normalize('/foo/../../../bar'), '\\bar');
313+
strictEqual(path.win32.normalize('a//b//../b'), 'a\\b');
314+
strictEqual(path.win32.normalize('a//b//./c'), 'a\\b\\c');
315+
strictEqual(path.win32.normalize('a//b//.'), 'a\\b');
316+
strictEqual(
317+
path.win32.normalize('//server/share/dir/file.ext'),
318+
'\\\\server\\share\\dir\\file.ext'
319+
);
320+
strictEqual(path.win32.normalize('/a/b/c/../../../x/y/z'), '\\x\\y\\z');
321+
strictEqual(path.win32.normalize('C:'), 'C:.');
322+
strictEqual(path.win32.normalize('C:..\\abc'), 'C:..\\abc');
323+
strictEqual(
324+
path.win32.normalize('C:..\\..\\abc\\..\\def'),
325+
'C:..\\..\\def'
326+
);
327+
strictEqual(path.win32.normalize('C:\\.'), 'C:\\');
328+
strictEqual(path.win32.normalize('file:stream'), 'file:stream');
329+
strictEqual(path.win32.normalize('bar\\foo..\\..\\'), 'bar\\');
330+
strictEqual(path.win32.normalize('bar\\foo..\\..'), 'bar');
331+
strictEqual(path.win32.normalize('bar\\foo..\\..\\baz'), 'bar\\baz');
332+
strictEqual(path.win32.normalize('bar\\foo..\\'), 'bar\\foo..\\');
333+
strictEqual(path.win32.normalize('bar\\foo..'), 'bar\\foo..');
334+
strictEqual(path.win32.normalize('..\\foo..\\..\\..\\bar'), '..\\..\\bar');
335+
strictEqual(
336+
path.win32.normalize('..\\...\\..\\.\\...\\..\\..\\bar'),
337+
'..\\..\\bar'
338+
);
339+
strictEqual(
340+
path.win32.normalize('../../../foo/../../../bar'),
341+
'..\\..\\..\\..\\..\\bar'
342+
);
343+
strictEqual(
344+
path.win32.normalize('../../../foo/../../../bar/../../'),
345+
'..\\..\\..\\..\\..\\..\\'
346+
);
347+
strictEqual(
348+
path.win32.normalize('../foobar/barfoo/foo/../../../bar/../../'),
349+
'..\\..\\'
350+
);
351+
strictEqual(
352+
path.win32.normalize('../.../../foobar/../../../bar/../../baz'),
353+
'..\\..\\..\\..\\baz'
354+
);
355+
strictEqual(path.win32.normalize('foo/bar\\baz'), 'foo\\bar\\baz');
356+
308357
strictEqual(
309358
path.posix.normalize('./fixtures///b/../b/c.js'),
310359
'fixtures/b/c.js'

0 commit comments

Comments
 (0)