Skip to content

Commit

Permalink
path: fix bugs and inconsistencies
Browse files Browse the repository at this point in the history 
Fixes: #54025
  • Loading branch information
@huseyinacacak-janea
huseyinacacak-janea committed Aug 15, 2024
1 parent 49a9ba4 commit 888faf2
Show file tree
Hide file tree
Showing 9 changed files with 171 additions and 68 deletions.
9 changes: 0 additions & 9 deletions lib/internal/url.js
View file
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 @@ -1564,13 +1562,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
126 changes: 99 additions & 27 deletions lib/path.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -186,6 +186,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 @@ -217,6 +218,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 @@ -263,11 +268,17 @@ const win32 = {
!isPathSeparator(StringPrototypeCharCodeAt(path, j))) {
j++;
}
if (j === len || j !== last) {
// We matched a UNC root
device =
`\\\\${firstPart}\\${StringPrototypeSlice(path, last, j)}`;
rootEnd = 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;
}
}
}
}
Expand Down Expand Up @@ -319,9 +330,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 @@ -377,17 +400,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 @@ -1122,6 +1150,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 @@ -1131,8 +1160,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 @@ -1144,10 +1182,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 @@ -1231,11 +1279,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
62 changes: 48 additions & 14 deletions src/path.cc
View file
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,9 @@ std::string PathResolve(Environment* env,
}
}

if (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 @@ -169,10 +173,17 @@ std::string PathResolve(Environment* env,
while (j < len && !IsPathSeparator(path[j])) {
j++;
}
if (j == len || j != last) {
// We matched a UNC root
device = "\\\\" + firstPart + "\\" + path.substr(last, j - last);
rootEnd = 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;
}
}
}
}
Expand Down Expand Up @@ -220,15 +231,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 +260,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 (i == numArgs - 1 && path.back() == '/') {
slashCheck = true;
}

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

Expand All @@ -254,15 +282,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
14 changes: 8 additions & 6 deletions test/cctest/test_path.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -25,20 +25,22 @@ 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");
Expand Down
2 changes: 1 addition & 1 deletion test/parallel/test-fs-utils-get-dirents.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -92,7 +92,7 @@ const filename = 'foo';
'DeprecationWarning',
'dirent.path is deprecated in favor of dirent.parentPath',
'DEP0178');
assert.deepStrictEqual(dirent.path, Buffer.from(tmpdir.resolve(`${filename}/`)));
assert.deepStrictEqual(dirent.path, Buffer.from(tmpdir.resolve(`${filename}`)));
},
));
}
Expand Down
6 changes: 3 additions & 3 deletions test/parallel/test-path-makelong.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -76,10 +76,10 @@ if (common.isWindows) {
assert.strictEqual(path.win32.toNamespacedPath('C:\\foo'), '\\\\?\\C:\\foo');
assert.strictEqual(path.win32.toNamespacedPath('C:/foo'), '\\\\?\\C:\\foo');
assert.strictEqual(path.win32.toNamespacedPath('\\\\foo\\bar'),
'\\\\?\\UNC\\foo\\bar\\');
'\\\\?\\UNC\\foo\\bar');
assert.strictEqual(path.win32.toNamespacedPath('//foo//bar'),
'\\\\?\\UNC\\foo\\bar\\');
assert.strictEqual(path.win32.toNamespacedPath('\\\\?\\foo'), '\\\\?\\foo\\');
'\\\\?\\UNC\\foo\\bar');
assert.strictEqual(path.win32.toNamespacedPath('\\\\?\\foo'), '\\\\?\\foo');
assert.strictEqual(path.win32.toNamespacedPath('\\\\?\\c:\\Windows/System'), '\\\\?\\c:\\Windows\\System');
assert.strictEqual(path.win32.toNamespacedPath(null), null);
assert.strictEqual(path.win32.toNamespacedPath(true), true);
Expand Down
2 changes: 2 additions & 0 deletions test/parallel/test-path-normalize.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,8 @@ assert.strictEqual(
'..\\..\\..\\..\\baz'
);
assert.strictEqual(path.win32.normalize('foo/bar\\baz'), 'foo\\bar\\baz');
assert.strictEqual(path.win32.normalize('\\\\.\\foo'), '\\\\.\\foo');
assert.strictEqual(path.win32.normalize('\\\\.\\foo\\'), '\\\\.\\foo\\');

assert.strictEqual(path.posix.normalize('./fixtures///b/../b/c.js'),
'fixtures/b/c.js');
Expand Down
Loading

0 comments on commit 888faf2

Please sign in to comment.