Skip to content

Commit

Permalink
path: fix bugs and inconsistencies
Browse files Browse the repository at this point in the history
Fixes: nodejs#54025
PR-URL: nodejs#54224
Reviewed-By: Yagiz Nizipli <yagiz@nizipli.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
  • Loading branch information
huseyinacacak-janea authored and louwers committed Nov 2, 2024
1 parent 19e4bbb commit 27afe68
Show file tree
Hide file tree
Showing 9 changed files with 185 additions and 70 deletions.
15 changes: 13 additions & 2 deletions lib/internal/modules/cjs/loader.js
Original file line number Diff line number Diff line change
Expand Up @@ -702,7 +702,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 @@ -712,7 +712,18 @@ 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
9 changes: 0 additions & 9 deletions lib/internal/url.js
Original file line number Diff line number Diff line change
Expand Up @@ -74,9 +74,7 @@ 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 @@ -1569,13 +1567,6 @@ function pathToFileURL(filepath, options = kEmptyObject) {
return outURL;
}
let resolved = (windows ?? isWindows) ? 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 += '/';

// Call encodePathChars first to avoid encoding % again for ? and #.
resolved = encodePathChars(resolved, { windows });
Expand Down
124 changes: 98 additions & 26 deletions lib/path.js
Original file line number Diff line number Diff line change
Expand Up @@ -190,6 +190,7 @@ 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 @@ -221,6 +222,10 @@ 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 @@ -268,10 +273,16 @@ const win32 = {
j++;
}
if (j === len || j !== last) {
// We matched a UNC root
device =
`\\\\${firstPart}\\${StringPrototypeSlice(path, last, j)}`;
rootEnd = j;
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;
}
}
}
}
Expand Down Expand Up @@ -323,9 +334,21 @@ const win32 = {
resolvedTail = normalizeString(resolvedTail, !resolvedAbsolute, '\\',
isPathSeparator);

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

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

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

return `${resolvedDevice}\\${resolvedTail}`;
},

/**
Expand Down Expand Up @@ -381,17 +404,22 @@ const win32 = {
!isPathSeparator(StringPrototypeCharCodeAt(path, j))) {
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;
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;
}
}
}
}
Expand Down Expand Up @@ -1162,6 +1190,7 @@ const posix = {
resolve(...args) {
let resolvedPath = '';
let resolvedAbsolute = false;
let slashCheck = false;

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

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

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

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

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

const fromStart = 1;
const fromEnd = from.length;
// 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 fromLen = fromEnd - fromStart;
const toStart = 1;
const toLen = to.length - toStart;

// 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;

// Compare paths to find the longest common path from root
const length = (fromLen < toLen ? fromLen : toLen);
Expand Down
61 changes: 48 additions & 13 deletions src/path.cc
Original file line number Diff line number Diff line change
Expand Up @@ -97,6 +97,7 @@ 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 @@ -126,6 +127,10 @@ 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 @@ -170,9 +175,16 @@ std::string PathResolve(Environment* env,
j++;
}
if (j == len || j != last) {
// We matched a UNC root
device = "\\\\" + firstPart + "\\" + path.substr(last, j - last);
rootEnd = j;
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;
}
}
}
}
Expand Down Expand Up @@ -220,15 +232,27 @@ std::string PathResolve(Environment* env,
// Normalize the tail path
resolvedTail = NormalizeString(resolvedTail, !resolvedAbsolute, "\\");

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

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

return ".";
if (slashCheck) {
if (resolvedTail == "\\") {
return resolvedDevice + "\\";
}
return resolvedDevice + "\\" + resolvedTail + "\\";
}
return resolvedDevice + "\\" + resolvedTail;
}
#else // _WIN32
std::string PathResolve(Environment* env,
Expand All @@ -237,10 +261,15 @@ 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 (static_cast<size_t>(i) == numArgs - 1 && path.back() == '/') {
slashCheck = true;
}

if (!path.empty()) {
resolvedPath = std::string(path) + "/" + resolvedPath;

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

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

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

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

Expand Down
20 changes: 11 additions & 9 deletions test/cctest/test_path.cc
Original file line number Diff line number Diff line change
Expand Up @@ -25,26 +25,28 @@ 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 27afe68

Please sign in to comment.