Skip to content

Commit

Permalink
Revert "path: fix bugs and inconsistencies"
Browse files Browse the repository at this point in the history 
This reverts commit efbba60.

PR-URL: nodejs#55414
Reviewed-By: Claudio Wunder <cwunder@gnome.org>
Reviewed-By: Richard Lau <rlau@redhat.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
  • Loading branch information
Aviv Keller authored Oct 21, 2024
1 parent 7e60b5e commit ee46d22
Show file tree
Hide file tree
Showing 9 changed files with 71 additions and 186 deletions.
15 changes: 2 additions & 13 deletions lib/internal/modules/cjs/loader.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -700,7 +700,7 @@ Module._findPath = function(request, paths, isMain) {

let exts;
const trailingSlash = request.length > 0 &&
((StringPrototypeCharCodeAt(request, request.length - 1) === CHAR_FORWARD_SLASH || (
(StringPrototypeCharCodeAt(request, request.length - 1) === CHAR_FORWARD_SLASH || (
StringPrototypeCharCodeAt(request, request.length - 1) === CHAR_DOT &&
(
request.length === 1 ||
Expand All @@ -710,18 +710,7 @@ Module._findPath = function(request, paths, isMain) {
StringPrototypeCharCodeAt(request, request.length - 3) === CHAR_FORWARD_SLASH
))
)
)) || (isWindows && (
StringPrototypeCharCodeAt(request, request.length - 1) === CHAR_BACKWARD_SLASH || (
StringPrototypeCharCodeAt(request, request.length - 1) === CHAR_DOT &&
(
request.length === 1 ||
StringPrototypeCharCodeAt(request, request.length - 2) === CHAR_BACKWARD_SLASH ||
(StringPrototypeCharCodeAt(request, request.length - 2) === CHAR_DOT && (
request.length === 2 ||
StringPrototypeCharCodeAt(request, request.length - 3) === CHAR_BACKWARD_SLASH
))
)
))));
));

const isRelative = StringPrototypeCharCodeAt(request, 0) === CHAR_DOT &&
(
Expand Down
11 changes: 10 additions & 1 deletion lib/internal/url.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -74,7 +74,9 @@ const {
} = require('internal/errors');
const {
CHAR_AMPERSAND,
CHAR_BACKWARD_SLASH,
CHAR_EQUAL,
CHAR_FORWARD_SLASH,
CHAR_LOWERCASE_A,
CHAR_LOWERCASE_Z,
CHAR_PERCENT,
Expand Down Expand Up @@ -1597,7 +1599,14 @@ function pathToFileURL(filepath, options = kEmptyObject) {
);
return outURL;
}
const resolved = windows ? path.win32.resolve(filepath) : path.posix.resolve(filepath);
let resolved = windows ? path.win32.resolve(filepath) : path.posix.resolve(filepath);
// path.resolve strips trailing slashes so we must add them back
const filePathLast = StringPrototypeCharCodeAt(filepath,
filepath.length - 1);
if ((filePathLast === CHAR_FORWARD_SLASH ||
((windows ?? isWindows) && filePathLast === CHAR_BACKWARD_SLASH)) &&
resolved[resolved.length - 1] !== path.sep)
resolved += '/';

return new URL(`file://${encodePathChars(resolved, { windows })}`);
}
Expand Down
124 changes: 26 additions & 98 deletions lib/path.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -190,7 +190,6 @@ const win32 = {
let resolvedDevice = '';
let resolvedTail = '';
let resolvedAbsolute = false;
let slashCheck = false;

for (let i = args.length - 1; i >= -1; i--) {
let path;
Expand Down Expand Up @@ -222,10 +221,6 @@ const win32 = {
}
}

if (i === args.length - 1 &&
isPathSeparator(StringPrototypeCharCodeAt(path, path.length - 1))) {
slashCheck = true;
}
const len = path.length;
let rootEnd = 0;
let device = '';
Expand Down Expand Up @@ -273,16 +268,10 @@ const win32 = {
j++;
}
if (j === len || j !== last) {
if (firstPart !== '.' && firstPart !== '?') {
// We matched a UNC root
device =
`\\\\${firstPart}\\${StringPrototypeSlice(path, last, j)}`;
rootEnd = j;
} else {
// We matched a device root (e.g. \\\\.\\PHYSICALDRIVE0)
device = `\\\\${firstPart}`;
rootEnd = 4;
}
// We matched a UNC root
device =
`\\\\${firstPart}\\${StringPrototypeSlice(path, last, j)}`;
rootEnd = j;
}
}
}
Expand Down Expand Up @@ -334,21 +323,9 @@ const win32 = {
resolvedTail = normalizeString(resolvedTail, !resolvedAbsolute, '\\',
isPathSeparator);

if (!resolvedAbsolute) {
return `${resolvedDevice}${resolvedTail}` || '.';
}

if (resolvedTail.length === 0) {
return slashCheck ? `${resolvedDevice}\\` : resolvedDevice;
}

if (slashCheck) {
return resolvedTail === '\\' ?
`${resolvedDevice}\\` :
`${resolvedDevice}\\${resolvedTail}\\`;
}

return `${resolvedDevice}\\${resolvedTail}`;
return resolvedAbsolute ?
`${resolvedDevice}\\${resolvedTail}` :
`${resolvedDevice}${resolvedTail}` || '.';
},

/**
Expand Down Expand Up @@ -404,22 +381,17 @@ const win32 = {
!isPathSeparator(StringPrototypeCharCodeAt(path, j))) {
j++;
}
if (j === len || j !== last) {
if (firstPart === '.' || firstPart === '?') {
// We matched a device root (e.g. \\\\.\\PHYSICALDRIVE0)
device = `\\\\${firstPart}`;
rootEnd = 4;
} else if (j === len) {
// We matched a UNC root only
// Return the normalized version of the UNC root since there
// is nothing left to process
return `\\\\${firstPart}\\${StringPrototypeSlice(path, last)}\\`;
} else {
// We matched a UNC root with leftovers
device =
`\\\\${firstPart}\\${StringPrototypeSlice(path, last, j)}`;
rootEnd = j;
}
if (j === len) {
// We matched a UNC root only
// Return the normalized version of the UNC root since there
// is nothing left to process
return `\\\\${firstPart}\\${StringPrototypeSlice(path, last)}\\`;
}
if (j !== last) {
// We matched a UNC root with leftovers
device =
`\\\\${firstPart}\\${StringPrototypeSlice(path, last, j)}`;
rootEnd = j;
}
}
}
Expand Down Expand Up @@ -1190,7 +1162,6 @@ const posix = {
resolve(...args) {
let resolvedPath = '';
let resolvedAbsolute = false;
let slashCheck = false;

for (let i = args.length - 1; i >= 0 && !resolvedAbsolute; i--) {
const path = args[i];
Expand All @@ -1200,17 +1171,8 @@ const posix = {
if (path.length === 0) {
continue;
}
if (i === args.length - 1 &&
isPosixPathSeparator(StringPrototypeCharCodeAt(path,
path.length - 1))) {
slashCheck = true;
}

if (resolvedPath.length !== 0) {
resolvedPath = `${path}/${resolvedPath}`;
} else {
resolvedPath = path;
}
resolvedPath = `${path}/${resolvedPath}`;
resolvedAbsolute =
StringPrototypeCharCodeAt(path, 0) === CHAR_FORWARD_SLASH;
}
Expand All @@ -1229,20 +1191,10 @@ const posix = {
resolvedPath = normalizeString(resolvedPath, !resolvedAbsolute, '/',
isPosixPathSeparator);

if (!resolvedAbsolute) {
if (resolvedPath.length === 0) {
return '.';
}
if (slashCheck) {
return `${resolvedPath}/`;
}
return resolvedPath;
}

if (resolvedPath.length === 0 || resolvedPath === '/') {
return '/';
if (resolvedAbsolute) {
return `/${resolvedPath}`;
}
return slashCheck ? `/${resolvedPath}/` : `/${resolvedPath}`;
return resolvedPath.length > 0 ? resolvedPath : '.';
},

/**
Expand Down Expand Up @@ -1326,35 +1278,11 @@ const posix = {
if (from === to)
return '';

// Trim any leading slashes
let fromStart = 0;
while (fromStart < from.length &&
StringPrototypeCharCodeAt(from, fromStart) === CHAR_FORWARD_SLASH) {
fromStart++;
}
// Trim trailing slashes
let fromEnd = from.length;
while (
fromEnd - 1 > fromStart &&
StringPrototypeCharCodeAt(from, fromEnd - 1) === CHAR_FORWARD_SLASH
) {
fromEnd--;
}
const fromStart = 1;
const fromEnd = from.length;
const fromLen = fromEnd - fromStart;

// Trim any leading slashes
let toStart = 0;
while (toStart < to.length &&
StringPrototypeCharCodeAt(to, toStart) === CHAR_FORWARD_SLASH) {
toStart++;
}
// Trim trailing slashes
let toEnd = to.length;
while (toEnd - 1 > toStart &&
StringPrototypeCharCodeAt(to, toEnd - 1) === CHAR_FORWARD_SLASH) {
toEnd--;
}
const toLen = toEnd - toStart;
const toStart = 1;
const toLen = to.length - toStart;

// Compare paths to find the longest common path from root
const length = (fromLen < toLen ? fromLen : toLen);
Expand Down
61 changes: 13 additions & 48 deletions src/path.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -97,7 +97,6 @@ std::string PathResolve(Environment* env,
std::string resolvedDevice = "";
std::string resolvedTail = "";
bool resolvedAbsolute = false;
bool slashCheck = false;
const size_t numArgs = paths.size();
auto cwd = env->GetCwd(env->exec_path());

Expand Down Expand Up @@ -127,10 +126,6 @@ std::string PathResolve(Environment* env,
}
}

if (static_cast<size_t>(i) == numArgs - 1 &&
IsPathSeparator(path[path.length() - 1])) {
slashCheck = true;
}
const size_t len = path.length();
int rootEnd = 0;
std::string device = "";
Expand Down Expand Up @@ -175,16 +170,9 @@ std::string PathResolve(Environment* env,
j++;
}
if (j == len || j != last) {
if (firstPart != "." && firstPart != "?") {
// We matched a UNC root
device =
"\\\\" + firstPart + "\\" + path.substr(last, j - last);
rootEnd = j;
} else {
// We matched a device root (e.g. \\\\.\\PHYSICALDRIVE0)
device = "\\\\" + firstPart;
rootEnd = 4;
}
// We matched a UNC root
device = "\\\\" + firstPart + "\\" + path.substr(last, j - last);
rootEnd = j;
}
}
}
Expand Down Expand Up @@ -232,27 +220,15 @@ std::string PathResolve(Environment* env,
// Normalize the tail path
resolvedTail = NormalizeString(resolvedTail, !resolvedAbsolute, "\\");

if (!resolvedAbsolute) {
if (!resolvedDevice.empty() || !resolvedTail.empty()) {
return resolvedDevice + resolvedTail;
}
return ".";
if (resolvedAbsolute) {
return resolvedDevice + "\\" + resolvedTail;
}

if (resolvedTail.empty()) {
if (slashCheck) {
return resolvedDevice + "\\";
}
return resolvedDevice;
if (!resolvedDevice.empty() || !resolvedTail.empty()) {
return resolvedDevice + resolvedTail;
}

if (slashCheck) {
if (resolvedTail == "\\") {
return resolvedDevice + "\\";
}
return resolvedDevice + "\\" + resolvedTail + "\\";
}
return resolvedDevice + "\\" + resolvedTail;
return ".";
}
#else // _WIN32
std::string PathResolve(Environment* env,
Expand All @@ -261,16 +237,11 @@ std::string PathResolve(Environment* env,
bool resolvedAbsolute = false;
auto cwd = env->GetCwd(env->exec_path());
const size_t numArgs = paths.size();
bool slashCheck = false;

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

if (!path.empty()) {
if (static_cast<size_t>(i) == numArgs - 1 && path.back() == '/') {
slashCheck = true;
}

resolvedPath = std::string(path) + "/" + resolvedPath;

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

if (!resolvedAbsolute) {
if (normalizedPath.empty()) {
return ".";
}
if (slashCheck) {
return normalizedPath + "/";
}
return normalizedPath;
if (resolvedAbsolute) {
return "/" + normalizedPath;
}

if (normalizedPath.empty() || normalizedPath == "/") {
return "/";
if (normalizedPath.empty()) {
return ".";
}

return slashCheck ? "/" + normalizedPath + "/" : "/" + normalizedPath;
return normalizedPath;
}
#endif // _WIN32

Expand Down
20 changes: 9 additions & 11 deletions test/cctest/test_path.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -25,28 +25,26 @@ TEST_F(PathTest, PathResolve) {
"d:\\e.exe");
EXPECT_EQ(PathResolve(*env, {"c:/ignore", "c:/some/file"}), "c:\\some\\file");
EXPECT_EQ(PathResolve(*env, {"d:/ignore", "d:some/dir//"}),
"d:\\ignore\\some\\dir\\");
"d:\\ignore\\some\\dir");
EXPECT_EQ(PathResolve(*env, {"."}), cwd);
EXPECT_EQ(PathResolve(*env, {"//server/share", "..", "relative\\"}),
"\\\\server\\share\\relative\\");
"\\\\server\\share\\relative");
EXPECT_EQ(PathResolve(*env, {"c:/", "//"}), "c:\\");
EXPECT_EQ(PathResolve(*env, {"c:/", "//dir"}), "c:\\dir");
EXPECT_EQ(PathResolve(*env, {"c:/", "//server/share"}), "\\\\server\\share");
EXPECT_EQ(PathResolve(*env, {"c:/", "//server//share"}), "\\\\server\\share");
EXPECT_EQ(PathResolve(*env, {"c:/", "//server/share"}),
"\\\\server\\share\\");
EXPECT_EQ(PathResolve(*env, {"c:/", "//server//share"}),
"\\\\server\\share\\");
EXPECT_EQ(PathResolve(*env, {"c:/", "///some//dir"}), "c:\\some\\dir");
EXPECT_EQ(
PathResolve(*env, {"C:\\foo\\tmp.3\\", "..\\tmp.3\\cycles\\root.js"}),
"C:\\foo\\tmp.3\\cycles\\root.js");
EXPECT_EQ(PathResolve(*env, {"\\\\.\\PHYSICALDRIVE0"}),
"\\\\.\\PHYSICALDRIVE0");
EXPECT_EQ(PathResolve(*env, {"\\\\?\\PHYSICALDRIVE0"}),
"\\\\?\\PHYSICALDRIVE0");
#else
EXPECT_EQ(PathResolve(*env, {"/var/lib", "../", "file/"}), "/var/file/");
EXPECT_EQ(PathResolve(*env, {"/var/lib", "/../", "file/"}), "/file/");
EXPECT_EQ(PathResolve(*env, {"/var/lib", "../", "file/"}), "/var/file");
EXPECT_EQ(PathResolve(*env, {"/var/lib", "/../", "file/"}), "/file");
EXPECT_EQ(PathResolve(*env, {"a/b/c/", "../../.."}), cwd);
EXPECT_EQ(PathResolve(*env, {"."}), cwd);
EXPECT_EQ(PathResolve(*env, {"/some/dir", ".", "/absolute/"}), "/absolute/");
EXPECT_EQ(PathResolve(*env, {"/some/dir", ".", "/absolute/"}), "/absolute");
EXPECT_EQ(PathResolve(*env, {"/foo/tmp.3/", "../tmp.3/cycles/root.js"}),
"/foo/tmp.3/cycles/root.js");
#endif
Expand Down
Loading

0 comments on commit ee46d22

Please sign in to comment.