Skip to content

Commit 4e2dddd

Browse files
huseyinacacak-janeahuseyinacacak-janea
committed
path: fix bugs and inconsistencies
Fixes: #54025
1 parent 49a9ba4 commit 4e2dddd

File tree

9 files changed

+174
-71
lines changed

9 files changed

+174
-71
lines changed

‎lib/internal/url.js‎

Lines changed: 0 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -74,9 +74,7 @@ const {
7474
} = require('internal/errors');
7575
const {
7676
CHAR_AMPERSAND,
77-
CHAR_BACKWARD_SLASH,
7877
CHAR_EQUAL,
79-
CHAR_FORWARD_SLASH,
8078
CHAR_LOWERCASE_A,
8179
CHAR_LOWERCASE_Z,
8280
CHAR_PERCENT,
@@ -1564,13 +1562,6 @@ function pathToFileURL(filepath, options = kEmptyObject) {
15641562
return outURL;
15651563
}
15661564
let resolved = (windows ?? isWindows) ? path.win32.resolve(filepath) : path.posix.resolve(filepath);
1567-
// path.resolve strips trailing slashes so we must add them back
1568-
const filePathLast = StringPrototypeCharCodeAt(filepath,
1569-
filepath.length - 1);
1570-
if ((filePathLast === CHAR_FORWARD_SLASH ||
1571-
((windows ?? isWindows) && filePathLast === CHAR_BACKWARD_SLASH)) &&
1572-
resolved[resolved.length - 1] !== path.sep)
1573-
resolved += '/';
15741565

15751566
// Call encodePathChars first to avoid encoding % again for ? and #.
15761567
resolved = encodePathChars(resolved, { windows });

‎lib/path.js‎

Lines changed: 99 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -186,6 +186,7 @@ const win32 = {
186186
let resolvedDevice = '';
187187
let resolvedTail = '';
188188
let resolvedAbsolute = false;
189+
let slashCheck = false;
189190

190191
for (let i = args.length - 1; i >= -1; i--) {
191192
let path;
@@ -217,6 +218,10 @@ const win32 = {
217218
}
218219
}
219220

221+
if (i === args.length - 1 &&
222+
isPathSeparator(StringPrototypeCharCodeAt(path, path.length - 1))) {
223+
slashCheck = true;
224+
}
220225
const len = path.length;
221226
let rootEnd = 0;
222227
let device = '';
@@ -263,11 +268,17 @@ const win32 = {
263268
!isPathSeparator(StringPrototypeCharCodeAt(path, j))) {
264269
j++;
265270
}
266-
if (j === len || j !== last) {
267-
// We matched a UNC root
268-
device =
269-
`\\\\${firstPart}\\${StringPrototypeSlice(path, last, j)}`;
270-
rootEnd = j;
271+
if ((j === len || j !== last)) {
272+
if (firstPart !== '.' && firstPart !== '?') {
273+
// We matched a UNC root
274+
device =
275+
`\\\\${firstPart}\\${StringPrototypeSlice(path, last, j)}`;
276+
rootEnd = j;
277+
} else {
278+
// We matched a device root (e.g. \\\\.\\PHYSICALDRIVE0)
279+
device = `\\\\${firstPart}`;
280+
rootEnd = 4;
281+
}
271282
}
272283
}
273284
}
@@ -319,9 +330,21 @@ const win32 = {
319330
resolvedTail = normalizeString(resolvedTail, !resolvedAbsolute, '\\',
320331
isPathSeparator);
321332

322-
return resolvedAbsolute ?
323-
`${resolvedDevice}\\${resolvedTail}` :
324-
`${resolvedDevice}${resolvedTail}` || '.';
333+
if (!resolvedAbsolute) {
334+
return `${resolvedDevice}${resolvedTail}` || '.';
335+
}
336+
337+
if (resolvedTail.length === 0) {
338+
return slashCheck ? `${resolvedDevice}\\` : resolvedDevice;
339+
}
340+
341+
if (slashCheck) {
342+
return resolvedTail === '\\' ?
343+
`${resolvedDevice}\\` :
344+
`${resolvedDevice}\\${resolvedTail}\\`;
345+
}
346+
347+
return `${resolvedDevice}\\${resolvedTail}`;
325348
},
326349

327350
/**
@@ -377,17 +400,22 @@ const win32 = {
377400
!isPathSeparator(StringPrototypeCharCodeAt(path, j))) {
378401
j++;
379402
}
380-
if (j === len) {
381-
// We matched a UNC root only
382-
// Return the normalized version of the UNC root since there
383-
// is nothing left to process
384-
return `\\\\${firstPart}\\${StringPrototypeSlice(path, last)}\\`;
385-
}
386-
if (j !== last) {
387-
// We matched a UNC root with leftovers
388-
device =
389-
`\\\\${firstPart}\\${StringPrototypeSlice(path, last, j)}`;
390-
rootEnd = j;
403+
if (j === len || j !== last) {
404+
if (firstPart === '.' || firstPart === '?') {
405+
// We matched a device root (e.g. \\\\.\\PHYSICALDRIVE0)
406+
device = `\\\\${firstPart}`;
407+
rootEnd = 4;
408+
} else if (j === len) {
409+
// We matched a UNC root only
410+
// Return the normalized version of the UNC root since there
411+
// is nothing left to process
412+
return `\\\\${firstPart}\\${StringPrototypeSlice(path, last)}\\`;
413+
} else {
414+
// We matched a UNC root with leftovers
415+
device =
416+
`\\\\${firstPart}\\${StringPrototypeSlice(path, last, j)}`;
417+
rootEnd = j;
418+
}
391419
}
392420
}
393421
}
@@ -1122,6 +1150,7 @@ const posix = {
11221150
resolve(...args) {
11231151
let resolvedPath = '';
11241152
let resolvedAbsolute = false;
1153+
let slashCheck = false;
11251154

11261155
for (let i = args.length - 1; i >= -1 && !resolvedAbsolute; i--) {
11271156
const path = i >= 0 ? args[i] : posixCwd();
@@ -1131,8 +1160,17 @@ const posix = {
11311160
if (path.length === 0) {
11321161
continue;
11331162
}
1163+
if (i === args.length - 1 &&
1164+
isPosixPathSeparator(StringPrototypeCharCodeAt(path,
1165+
path.length - 1))) {
1166+
slashCheck = true;
1167+
}
11341168

1135-
resolvedPath = `${path}/${resolvedPath}`;
1169+
if (resolvedPath.length !== 0) {
1170+
resolvedPath = `${path}/${resolvedPath}`;
1171+
} else {
1172+
resolvedPath = path;
1173+
}
11361174
resolvedAbsolute =
11371175
StringPrototypeCharCodeAt(path, 0) === CHAR_FORWARD_SLASH;
11381176
}
@@ -1144,10 +1182,20 @@ const posix = {
11441182
resolvedPath = normalizeString(resolvedPath, !resolvedAbsolute, '/',
11451183
isPosixPathSeparator);
11461184

1147-
if (resolvedAbsolute) {
1148-
return `/${resolvedPath}`;
1185+
if (!resolvedAbsolute) {
1186+
if (resolvedPath.length === 0) {
1187+
return '.';
1188+
}
1189+
if (slashCheck) {
1190+
return `${resolvedPath}/`;
1191+
}
1192+
return resolvedPath;
1193+
}
1194+
1195+
if (resolvedPath.length === 0 || resolvedPath === '/') {
1196+
return '/';
11491197
}
1150-
return resolvedPath.length > 0 ? resolvedPath : '.';
1198+
return slashCheck ? `/${resolvedPath}/` : `/${resolvedPath}`;
11511199
},
11521200

11531201
/**
@@ -1231,11 +1279,35 @@ const posix = {
12311279
if (from === to)
12321280
return '';
12331281

1234-
const fromStart = 1;
1235-
const fromEnd = from.length;
1282+
// Trim any leading slashes
1283+
let fromStart = 0;
1284+
while (fromStart < from.length &&
1285+
StringPrototypeCharCodeAt(from, fromStart) === CHAR_FORWARD_SLASH) {
1286+
fromStart++;
1287+
}
1288+
// Trim trailing slashes
1289+
let fromEnd = from.length;
1290+
while (
1291+
fromEnd - 1 > fromStart &&
1292+
StringPrototypeCharCodeAt(from, fromEnd - 1) === CHAR_FORWARD_SLASH
1293+
) {
1294+
fromEnd--;
1295+
}
12361296
const fromLen = fromEnd - fromStart;
1237-
const toStart = 1;
1238-
const toLen = to.length - toStart;
1297+
1298+
// Trim any leading slashes
1299+
let toStart = 0;
1300+
while (toStart < to.length &&
1301+
StringPrototypeCharCodeAt(to, toStart) === CHAR_FORWARD_SLASH) {
1302+
toStart++;
1303+
}
1304+
// Trim trailing slashes
1305+
let toEnd = to.length;
1306+
while (toEnd - 1 > toStart &&
1307+
StringPrototypeCharCodeAt(to, toEnd - 1) === CHAR_FORWARD_SLASH) {
1308+
toEnd--;
1309+
}
1310+
const toLen = toEnd - toStart;
12391311

12401312
// Compare paths to find the longest common path from root
12411313
const length = (fromLen < toLen ? fromLen : toLen);

‎src/path.cc‎

Lines changed: 48 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -97,6 +97,7 @@ std::string PathResolve(Environment* env,
9797
std::string resolvedDevice = "";
9898
std::string resolvedTail = "";
9999
bool resolvedAbsolute = false;
100+
bool slashCheck = false;
100101
const size_t numArgs = paths.size();
101102
auto cwd = env->GetCwd(env->exec_path());
102103

@@ -126,6 +127,9 @@ std::string PathResolve(Environment* env,
126127
}
127128
}
128129

130+
if (static_cast<size_t>(i) == numArgs - 1 && IsPathSeparator(path[path.length() - 1])) {
131+
slashCheck = true;
132+
}
129133
const size_t len = path.length();
130134
int rootEnd = 0;
131135
std::string device = "";
@@ -169,10 +173,17 @@ std::string PathResolve(Environment* env,
169173
while (j < len && !IsPathSeparator(path[j])) {
170174
j++;
171175
}
172-
if (j == len || j != last) {
173-
// We matched a UNC root
174-
device = "\\\\" + firstPart + "\\" + path.substr(last, j - last);
175-
rootEnd = j;
176+
if ((j == len || j != last)) {
177+
if (firstPart != "." && firstPart != "?") {
178+
// We matched a UNC root
179+
device =
180+
"\\\\" + firstPart + "\\" + path.substr(last, j - last);
181+
rootEnd = j;
182+
} else {
183+
// We matched a device root (e.g. \\\\.\\PHYSICALDRIVE0)
184+
device = "\\\\" + firstPart;
185+
rootEnd = 4;
186+
}
176187
}
177188
}
178189
}
@@ -220,15 +231,27 @@ std::string PathResolve(Environment* env,
220231
// Normalize the tail path
221232
resolvedTail = NormalizeString(resolvedTail, !resolvedAbsolute, "\\");
222233

223-
if (resolvedAbsolute) {
224-
return resolvedDevice + "\\" + resolvedTail;
234+
if (!resolvedAbsolute) {
235+
if (!resolvedDevice.empty() || !resolvedTail.empty()) {
236+
return resolvedDevice + resolvedTail;
237+
}
238+
return ".";
225239
}
226240

227-
if (!resolvedDevice.empty() || !resolvedTail.empty()) {
228-
return resolvedDevice + resolvedTail;
241+
if (resolvedTail.empty()) {
242+
if (slashCheck) {
243+
return resolvedDevice + "\\";
244+
}
245+
return resolvedDevice;
229246
}
230247

231-
return ".";
248+
if (slashCheck) {
249+
if (resolvedTail == "\\") {
250+
return resolvedDevice + "\\";
251+
}
252+
return resolvedDevice + "\\" + resolvedTail + "\\";
253+
}
254+
return resolvedDevice + "\\" + resolvedTail;
232255
}
233256
#else // _WIN32
234257
std::string PathResolve(Environment* env,
@@ -237,10 +260,15 @@ std::string PathResolve(Environment* env,
237260
bool resolvedAbsolute = false;
238261
auto cwd = env->GetCwd(env->exec_path());
239262
const size_t numArgs = paths.size();
263+
bool slashCheck = false;
240264

241265
for (int i = numArgs - 1; i >= -1 && !resolvedAbsolute; i--) {
242266
const std::string& path = (i >= 0) ? std::string(paths[i]) : cwd;
243267

268+
if (static_cast<size_t>(i) == numArgs - 1 && path.back() == '/') {
269+
slashCheck = true;
270+
}
271+
244272
if (!path.empty()) {
245273
resolvedPath = std::string(path) + "/" + resolvedPath;
246274

@@ -254,15 +282,21 @@ std::string PathResolve(Environment* env,
254282
// Normalize the path
255283
auto normalizedPath = NormalizeString(resolvedPath, !resolvedAbsolute, "/");
256284

257-
if (resolvedAbsolute) {
258-
return "/" + normalizedPath;
285+
if (!resolvedAbsolute) {
286+
if (normalizedPath.empty()) {
287+
return ".";
288+
}
289+
if (slashCheck) {
290+
return normalizedPath + "/";
291+
}
292+
return normalizedPath;
259293
}
260294

261-
if (normalizedPath.empty()) {
262-
return ".";
295+
if (normalizedPath.empty() || normalizedPath == "/") {
296+
return "/";
263297
}
264298

265-
return normalizedPath;
299+
return slashCheck ? "/" + normalizedPath + "/" : "/" + normalizedPath;
266300
}
267301
#endif // _WIN32
268302

‎test/cctest/test_path.cc‎

Lines changed: 11 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -25,26 +25,28 @@ TEST_F(PathTest, PathResolve) {
2525
"d:\\e.exe");
2626
EXPECT_EQ(PathResolve(*env, {"c:/ignore", "c:/some/file"}), "c:\\some\\file");
2727
EXPECT_EQ(PathResolve(*env, {"d:/ignore", "d:some/dir//"}),
28-
"d:\\ignore\\some\\dir");
28+
"d:\\ignore\\some\\dir\\");
2929
EXPECT_EQ(PathResolve(*env, {"."}), cwd);
3030
EXPECT_EQ(PathResolve(*env, {"//server/share", "..", "relative\\"}),
31-
"\\\\server\\share\\relative");
31+
"\\\\server\\share\\relative\\");
3232
EXPECT_EQ(PathResolve(*env, {"c:/", "//"}), "c:\\");
3333
EXPECT_EQ(PathResolve(*env, {"c:/", "//dir"}), "c:\\dir");
34-
EXPECT_EQ(PathResolve(*env, {"c:/", "//server/share"}),
35-
"\\\\server\\share\\");
36-
EXPECT_EQ(PathResolve(*env, {"c:/", "//server//share"}),
37-
"\\\\server\\share\\");
34+
EXPECT_EQ(PathResolve(*env, {"c:/", "//server/share"}), "\\\\server\\share");
35+
EXPECT_EQ(PathResolve(*env, {"c:/", "//server//share"}), "\\\\server\\share");
3836
EXPECT_EQ(PathResolve(*env, {"c:/", "///some//dir"}), "c:\\some\\dir");
3937
EXPECT_EQ(
4038
PathResolve(*env, {"C:\\foo\\tmp.3\\", "..\\tmp.3\\cycles\\root.js"}),
4139
"C:\\foo\\tmp.3\\cycles\\root.js");
40+
EXPECT_EQ(PathResolve(*env, {"\\\\.\\PHYSICALDRIVE0"}),
41+
"\\\\.\\PHYSICALDRIVE0");
42+
EXPECT_EQ(PathResolve(*env, {"\\\\?\\PHYSICALDRIVE0"}),
43+
"\\\\?\\PHYSICALDRIVE0");
4244
#else
43-
EXPECT_EQ(PathResolve(*env, {"/var/lib", "../", "file/"}), "/var/file");
44-
EXPECT_EQ(PathResolve(*env, {"/var/lib", "/../", "file/"}), "/file");
45+
EXPECT_EQ(PathResolve(*env, {"/var/lib", "../", "file/"}), "/var/file/");
46+
EXPECT_EQ(PathResolve(*env, {"/var/lib", "/../", "file/"}), "/file/");
4547
EXPECT_EQ(PathResolve(*env, {"a/b/c/", "../../.."}), cwd);
4648
EXPECT_EQ(PathResolve(*env, {"."}), cwd);
47-
EXPECT_EQ(PathResolve(*env, {"/some/dir", ".", "/absolute/"}), "/absolute");
49+
EXPECT_EQ(PathResolve(*env, {"/some/dir", ".", "/absolute/"}), "/absolute/");
4850
EXPECT_EQ(PathResolve(*env, {"/foo/tmp.3/", "../tmp.3/cycles/root.js"}),
4951
"/foo/tmp.3/cycles/root.js");
5052
#endif

‎test/parallel/test-fs-utils-get-dirents.js‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -92,7 +92,7 @@ const filename = 'foo';
9292
'DeprecationWarning',
9393
'dirent.path is deprecated in favor of dirent.parentPath',
9494
'DEP0178');
95-
assert.deepStrictEqual(dirent.path, Buffer.from(tmpdir.resolve(`${filename}/`)));
95+
assert.deepStrictEqual(dirent.path, Buffer.from(tmpdir.resolve(`${filename}`)));
9696
},
9797
));
9898
}

‎test/parallel/test-path-makelong.js‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -76,10 +76,10 @@ if (common.isWindows) {
7676
assert.strictEqual(path.win32.toNamespacedPath('C:\\foo'), '\\\\?\\C:\\foo');
7777
assert.strictEqual(path.win32.toNamespacedPath('C:/foo'), '\\\\?\\C:\\foo');
7878
assert.strictEqual(path.win32.toNamespacedPath('\\\\foo\\bar'),
79-
'\\\\?\\UNC\\foo\\bar\\');
79+
'\\\\?\\UNC\\foo\\bar');
8080
assert.strictEqual(path.win32.toNamespacedPath('//foo//bar'),
81-
'\\\\?\\UNC\\foo\\bar\\');
82-
assert.strictEqual(path.win32.toNamespacedPath('\\\\?\\foo'), '\\\\?\\foo\\');
81+
'\\\\?\\UNC\\foo\\bar');
82+
assert.strictEqual(path.win32.toNamespacedPath('\\\\?\\foo'), '\\\\?\\foo');
8383
assert.strictEqual(path.win32.toNamespacedPath('\\\\?\\c:\\Windows/System'), '\\\\?\\c:\\Windows\\System');
8484
assert.strictEqual(path.win32.toNamespacedPath(null), null);
8585
assert.strictEqual(path.win32.toNamespacedPath(true), true);

0 commit comments

Comments
 (0)