Skip to content

Commit 64656bd

Browse files
avivkellerlouwers
avivkeller
authored and
louwers
committed
Revert "path: fix bugs and inconsistencies"
This reverts commit efbba60. PR-URL: nodejs#55414 Reviewed-By: Claudio Wunder <[email protected]> Reviewed-By: Richard Lau <[email protected]> Reviewed-By: Luigi Pinca <[email protected]>
1 parent 4d4f05e commit 64656bd

File tree

9 files changed

+71
-186
lines changed

9 files changed

+71
-186
lines changed

‎lib/internal/modules/cjs/loader.js‎

Lines changed: 2 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -700,7 +700,7 @@ Module._findPath = function(request, paths, isMain) {
700700

701701
let exts;
702702
const trailingSlash = request.length > 0 &&
703-
((StringPrototypeCharCodeAt(request, request.length - 1) === CHAR_FORWARD_SLASH || (
703+
(StringPrototypeCharCodeAt(request, request.length - 1) === CHAR_FORWARD_SLASH || (
704704
StringPrototypeCharCodeAt(request, request.length - 1) === CHAR_DOT &&
705705
(
706706
request.length === 1 ||
@@ -710,18 +710,7 @@ Module._findPath = function(request, paths, isMain) {
710710
StringPrototypeCharCodeAt(request, request.length - 3) === CHAR_FORWARD_SLASH
711711
))
712712
)
713-
)) || (isWindows && (
714-
StringPrototypeCharCodeAt(request, request.length - 1) === CHAR_BACKWARD_SLASH || (
715-
StringPrototypeCharCodeAt(request, request.length - 1) === CHAR_DOT &&
716-
(
717-
request.length === 1 ||
718-
StringPrototypeCharCodeAt(request, request.length - 2) === CHAR_BACKWARD_SLASH ||
719-
(StringPrototypeCharCodeAt(request, request.length - 2) === CHAR_DOT && (
720-
request.length === 2 ||
721-
StringPrototypeCharCodeAt(request, request.length - 3) === CHAR_BACKWARD_SLASH
722-
))
723-
)
724-
))));
713+
));
725714

726715
const isRelative = StringPrototypeCharCodeAt(request, 0) === CHAR_DOT &&
727716
(

‎lib/internal/url.js‎

Lines changed: 10 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -74,7 +74,9 @@ const {
7474
} = require('internal/errors');
7575
const {
7676
CHAR_AMPERSAND,
77+
CHAR_BACKWARD_SLASH,
7778
CHAR_EQUAL,
79+
CHAR_FORWARD_SLASH,
7880
CHAR_LOWERCASE_A,
7981
CHAR_LOWERCASE_Z,
8082
CHAR_PERCENT,
@@ -1597,7 +1599,14 @@ function pathToFileURL(filepath, options = kEmptyObject) {
15971599
);
15981600
return outURL;
15991601
}
1600-
const resolved = windows ? path.win32.resolve(filepath) : path.posix.resolve(filepath);
1602+
let resolved = windows ? path.win32.resolve(filepath) : path.posix.resolve(filepath);
1603+
// path.resolve strips trailing slashes so we must add them back
1604+
const filePathLast = StringPrototypeCharCodeAt(filepath,
1605+
filepath.length - 1);
1606+
if ((filePathLast === CHAR_FORWARD_SLASH ||
1607+
((windows ?? isWindows) && filePathLast === CHAR_BACKWARD_SLASH)) &&
1608+
resolved[resolved.length - 1] !== path.sep)
1609+
resolved += '/';
16011610

16021611
return new URL(`file://${encodePathChars(resolved, { windows })}`);
16031612
}

‎lib/path.js‎

Lines changed: 26 additions & 98 deletions
Original file line numberDiff line numberDiff line change
@@ -190,7 +190,6 @@ const win32 = {
190190
let resolvedDevice = '';
191191
let resolvedTail = '';
192192
let resolvedAbsolute = false;
193-
let slashCheck = false;
194193

195194
for (let i = args.length - 1; i >= -1; i--) {
196195
let path;
@@ -222,10 +221,6 @@ const win32 = {
222221
}
223222
}
224223

225-
if (i === args.length - 1 &&
226-
isPathSeparator(StringPrototypeCharCodeAt(path, path.length - 1))) {
227-
slashCheck = true;
228-
}
229224
const len = path.length;
230225
let rootEnd = 0;
231226
let device = '';
@@ -273,16 +268,10 @@ const win32 = {
273268
j++;
274269
}
275270
if (j === len || j !== last) {
276-
if (firstPart !== '.' && firstPart !== '?') {
277-
// We matched a UNC root
278-
device =
279-
`\\\\${firstPart}\\${StringPrototypeSlice(path, last, j)}`;
280-
rootEnd = j;
281-
} else {
282-
// We matched a device root (e.g. \\\\.\\PHYSICALDRIVE0)
283-
device = `\\\\${firstPart}`;
284-
rootEnd = 4;
285-
}
271+
// We matched a UNC root
272+
device =
273+
`\\\\${firstPart}\\${StringPrototypeSlice(path, last, j)}`;
274+
rootEnd = j;
286275
}
287276
}
288277
}
@@ -334,21 +323,9 @@ const win32 = {
334323
resolvedTail = normalizeString(resolvedTail, !resolvedAbsolute, '\\',
335324
isPathSeparator);
336325

337-
if (!resolvedAbsolute) {
338-
return `${resolvedDevice}${resolvedTail}` || '.';
339-
}
340-
341-
if (resolvedTail.length === 0) {
342-
return slashCheck ? `${resolvedDevice}\\` : resolvedDevice;
343-
}
344-
345-
if (slashCheck) {
346-
return resolvedTail === '\\' ?
347-
`${resolvedDevice}\\` :
348-
`${resolvedDevice}\\${resolvedTail}\\`;
349-
}
350-
351-
return `${resolvedDevice}\\${resolvedTail}`;
326+
return resolvedAbsolute ?
327+
`${resolvedDevice}\\${resolvedTail}` :
328+
`${resolvedDevice}${resolvedTail}` || '.';
352329
},
353330

354331
/**
@@ -404,22 +381,17 @@ const win32 = {
404381
!isPathSeparator(StringPrototypeCharCodeAt(path, j))) {
405382
j++;
406383
}
407-
if (j === len || j !== last) {
408-
if (firstPart === '.' || firstPart === '?') {
409-
// We matched a device root (e.g. \\\\.\\PHYSICALDRIVE0)
410-
device = `\\\\${firstPart}`;
411-
rootEnd = 4;
412-
} else if (j === len) {
413-
// We matched a UNC root only
414-
// Return the normalized version of the UNC root since there
415-
// is nothing left to process
416-
return `\\\\${firstPart}\\${StringPrototypeSlice(path, last)}\\`;
417-
} else {
418-
// We matched a UNC root with leftovers
419-
device =
420-
`\\\\${firstPart}\\${StringPrototypeSlice(path, last, j)}`;
421-
rootEnd = j;
422-
}
384+
if (j === len) {
385+
// We matched a UNC root only
386+
// Return the normalized version of the UNC root since there
387+
// is nothing left to process
388+
return `\\\\${firstPart}\\${StringPrototypeSlice(path, last)}\\`;
389+
}
390+
if (j !== last) {
391+
// We matched a UNC root with leftovers
392+
device =
393+
`\\\\${firstPart}\\${StringPrototypeSlice(path, last, j)}`;
394+
rootEnd = j;
423395
}
424396
}
425397
}
@@ -1190,7 +1162,6 @@ const posix = {
11901162
resolve(...args) {
11911163
let resolvedPath = '';
11921164
let resolvedAbsolute = false;
1193-
let slashCheck = false;
11941165

11951166
for (let i = args.length - 1; i >= 0 && !resolvedAbsolute; i--) {
11961167
const path = args[i];
@@ -1200,17 +1171,8 @@ const posix = {
12001171
if (path.length === 0) {
12011172
continue;
12021173
}
1203-
if (i === args.length - 1 &&
1204-
isPosixPathSeparator(StringPrototypeCharCodeAt(path,
1205-
path.length - 1))) {
1206-
slashCheck = true;
1207-
}
12081174

1209-
if (resolvedPath.length !== 0) {
1210-
resolvedPath = `${path}/${resolvedPath}`;
1211-
} else {
1212-
resolvedPath = path;
1213-
}
1175+
resolvedPath = `${path}/${resolvedPath}`;
12141176
resolvedAbsolute =
12151177
StringPrototypeCharCodeAt(path, 0) === CHAR_FORWARD_SLASH;
12161178
}
@@ -1229,20 +1191,10 @@ const posix = {
12291191
resolvedPath = normalizeString(resolvedPath, !resolvedAbsolute, '/',
12301192
isPosixPathSeparator);
12311193

1232-
if (!resolvedAbsolute) {
1233-
if (resolvedPath.length === 0) {
1234-
return '.';
1235-
}
1236-
if (slashCheck) {
1237-
return `${resolvedPath}/`;
1238-
}
1239-
return resolvedPath;
1240-
}
1241-
1242-
if (resolvedPath.length === 0 || resolvedPath === '/') {
1243-
return '/';
1194+
if (resolvedAbsolute) {
1195+
return `/${resolvedPath}`;
12441196
}
1245-
return slashCheck ? `/${resolvedPath}/` : `/${resolvedPath}`;
1197+
return resolvedPath.length > 0 ? resolvedPath : '.';
12461198
},
12471199

12481200
/**
@@ -1326,35 +1278,11 @@ const posix = {
13261278
if (from === to)
13271279
return '';
13281280

1329-
// Trim any leading slashes
1330-
let fromStart = 0;
1331-
while (fromStart < from.length &&
1332-
StringPrototypeCharCodeAt(from, fromStart) === CHAR_FORWARD_SLASH) {
1333-
fromStart++;
1334-
}
1335-
// Trim trailing slashes
1336-
let fromEnd = from.length;
1337-
while (
1338-
fromEnd - 1 > fromStart &&
1339-
StringPrototypeCharCodeAt(from, fromEnd - 1) === CHAR_FORWARD_SLASH
1340-
) {
1341-
fromEnd--;
1342-
}
1281+
const fromStart = 1;
1282+
const fromEnd = from.length;
13431283
const fromLen = fromEnd - fromStart;
1344-
1345-
// Trim any leading slashes
1346-
let toStart = 0;
1347-
while (toStart < to.length &&
1348-
StringPrototypeCharCodeAt(to, toStart) === CHAR_FORWARD_SLASH) {
1349-
toStart++;
1350-
}
1351-
// Trim trailing slashes
1352-
let toEnd = to.length;
1353-
while (toEnd - 1 > toStart &&
1354-
StringPrototypeCharCodeAt(to, toEnd - 1) === CHAR_FORWARD_SLASH) {
1355-
toEnd--;
1356-
}
1357-
const toLen = toEnd - toStart;
1284+
const toStart = 1;
1285+
const toLen = to.length - toStart;
13581286

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

‎src/path.cc‎

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

@@ -127,10 +126,6 @@ std::string PathResolve(Environment* env,
127126
}
128127
}
129128

130-
if (static_cast<size_t>(i) == numArgs - 1 &&
131-
IsPathSeparator(path[path.length() - 1])) {
132-
slashCheck = true;
133-
}
134129
const size_t len = path.length();
135130
int rootEnd = 0;
136131
std::string device = "";
@@ -175,16 +170,9 @@ std::string PathResolve(Environment* env,
175170
j++;
176171
}
177172
if (j == len || j != last) {
178-
if (firstPart != "." && firstPart != "?") {
179-
// We matched a UNC root
180-
device =
181-
"\\\\" + firstPart + "\\" + path.substr(last, j - last);
182-
rootEnd = j;
183-
} else {
184-
// We matched a device root (e.g. \\\\.\\PHYSICALDRIVE0)
185-
device = "\\\\" + firstPart;
186-
rootEnd = 4;
187-
}
173+
// We matched a UNC root
174+
device = "\\\\" + firstPart + "\\" + path.substr(last, j - last);
175+
rootEnd = j;
188176
}
189177
}
190178
}
@@ -232,27 +220,15 @@ std::string PathResolve(Environment* env,
232220
// Normalize the tail path
233221
resolvedTail = NormalizeString(resolvedTail, !resolvedAbsolute, "\\");
234222

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

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

249-
if (slashCheck) {
250-
if (resolvedTail == "\\") {
251-
return resolvedDevice + "\\";
252-
}
253-
return resolvedDevice + "\\" + resolvedTail + "\\";
254-
}
255-
return resolvedDevice + "\\" + resolvedTail;
231+
return ".";
256232
}
257233
#else // _WIN32
258234
std::string PathResolve(Environment* env,
@@ -261,16 +237,11 @@ std::string PathResolve(Environment* env,
261237
bool resolvedAbsolute = false;
262238
auto cwd = env->GetCwd(env->exec_path());
263239
const size_t numArgs = paths.size();
264-
bool slashCheck = false;
265240

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

269244
if (!path.empty()) {
270-
if (static_cast<size_t>(i) == numArgs - 1 && path.back() == '/') {
271-
slashCheck = true;
272-
}
273-
274245
resolvedPath = std::string(path) + "/" + resolvedPath;
275246

276247
if (path.front() == '/') {
@@ -283,21 +254,15 @@ std::string PathResolve(Environment* env,
283254
// Normalize the path
284255
auto normalizedPath = NormalizeString(resolvedPath, !resolvedAbsolute, "/");
285256

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

296-
if (normalizedPath.empty() || normalizedPath == "/") {
297-
return "/";
261+
if (normalizedPath.empty()) {
262+
return ".";
298263
}
299264

300-
return slashCheck ? "/" + normalizedPath + "/" : "/" + normalizedPath;
265+
return normalizedPath;
301266
}
302267
#endif // _WIN32
303268

‎test/cctest/test_path.cc‎

Lines changed: 9 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -25,28 +25,26 @@ 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"}), "\\\\server\\share");
35-
EXPECT_EQ(PathResolve(*env, {"c:/", "//server//share"}), "\\\\server\\share");
34+
EXPECT_EQ(PathResolve(*env, {"c:/", "//server/share"}),
35+
"\\\\server\\share\\");
36+
EXPECT_EQ(PathResolve(*env, {"c:/", "//server//share"}),
37+
"\\\\server\\share\\");
3638
EXPECT_EQ(PathResolve(*env, {"c:/", "///some//dir"}), "c:\\some\\dir");
3739
EXPECT_EQ(
3840
PathResolve(*env, {"C:\\foo\\tmp.3\\", "..\\tmp.3\\cycles\\root.js"}),
3941
"C:\\foo\\tmp.3\\cycles\\root.js");
40-
EXPECT_EQ(PathResolve(*env, {"\\\\.\\PHYSICALDRIVE0"}),
41-
"\\\\.\\PHYSICALDRIVE0");
42-
EXPECT_EQ(PathResolve(*env, {"\\\\?\\PHYSICALDRIVE0"}),
43-
"\\\\?\\PHYSICALDRIVE0");
4442
#else
45-
EXPECT_EQ(PathResolve(*env, {"/var/lib", "../", "file/"}), "/var/file/");
46-
EXPECT_EQ(PathResolve(*env, {"/var/lib", "/../", "file/"}), "/file/");
43+
EXPECT_EQ(PathResolve(*env, {"/var/lib", "../", "file/"}), "/var/file");
44+
EXPECT_EQ(PathResolve(*env, {"/var/lib", "/../", "file/"}), "/file");
4745
EXPECT_EQ(PathResolve(*env, {"a/b/c/", "../../.."}), cwd);
4846
EXPECT_EQ(PathResolve(*env, {"."}), cwd);
49-
EXPECT_EQ(PathResolve(*env, {"/some/dir", ".", "/absolute/"}), "/absolute/");
47+
EXPECT_EQ(PathResolve(*env, {"/some/dir", ".", "/absolute/"}), "/absolute");
5048
EXPECT_EQ(PathResolve(*env, {"/foo/tmp.3/", "../tmp.3/cycles/root.js"}),
5149
"/foo/tmp.3/cycles/root.js");
5250
#endif

0 commit comments

Comments
 (0)