Skip to content

Commit 4f0bff1

Browse files
murataslan1murataslan1
committed
path: fix inconsistent basename suffix stripping with trailing slashes
The basename() suffix matching was done in a single pass with the path component scanning, which caused incorrect results when: - The path had trailing slashes (e.g., 'a/') - The suffix contained path separators - The suffix equaled the basename after a path separator Fix by separating the two operations: first resolve the basename (stripping directory and trailing slashes), then strip the suffix from the result. This restores the pre-#5123 suffix stripping behavior while keeping the optimized path scanning. Fixes: #21358
1 parent e1746a9 commit 4f0bff1

2 files changed

Lines changed: 49 additions & 98 deletions

File tree

‎lib/path.js‎

Lines changed: 28 additions & 92 deletions
Original file line numberDiff line numberDiff line change
@@ -913,51 +913,7 @@ const win32 = {
913913
start = 2;
914914
}
915915

916-
if (suffix !== undefined && suffix.length > 0 && suffix.length <= path.length) {
917-
if (suffix === path)
918-
return '';
919-
let extIdx = suffix.length - 1;
920-
let firstNonSlashEnd = -1;
921-
for (let i = path.length - 1; i >= start; --i) {
922-
const code = StringPrototypeCharCodeAt(path, i);
923-
if (isPathSeparator(code)) {
924-
// If we reached a path separator that was not part of a set of path
925-
// separators at the end of the string, stop now
926-
if (!matchedSlash) {
927-
start = i + 1;
928-
break;
929-
}
930-
} else {
931-
if (firstNonSlashEnd === -1) {
932-
// We saw the first non-path separator, remember this index in case
933-
// we need it if the extension ends up not matching
934-
matchedSlash = false;
935-
firstNonSlashEnd = i + 1;
936-
}
937-
if (extIdx >= 0) {
938-
// Try to match the explicit extension
939-
if (code === StringPrototypeCharCodeAt(suffix, extIdx)) {
940-
if (--extIdx === -1) {
941-
// We matched the extension, so mark this as the end of our path
942-
// component
943-
end = i;
944-
}
945-
} else {
946-
// Extension does not match, so our result is the entire path
947-
// component
948-
extIdx = -1;
949-
end = firstNonSlashEnd;
950-
}
951-
}
952-
}
953-
}
954-
955-
if (start === end)
956-
end = firstNonSlashEnd;
957-
else if (end === -1)
958-
end = path.length;
959-
return StringPrototypeSlice(path, start, end);
960-
}
916+
// Find the basename by scanning backwards, skipping trailing separators
961917
for (let i = path.length - 1; i >= start; --i) {
962918
if (isPathSeparator(StringPrototypeCharCodeAt(path, i))) {
963919
// If we reached a path separator that was not part of a set of path
@@ -976,7 +932,19 @@ const win32 = {
976932

977933
if (end === -1)
978934
return '';
979-
return StringPrototypeSlice(path, start, end);
935+
936+
const base = StringPrototypeSlice(path, start, end);
937+
938+
// Strip suffix if provided and the basename ends with it
939+
if (suffix !== undefined && suffix.length > 0 && suffix.length <= base.length) {
940+
if (base === suffix)
941+
return '';
942+
if (StringPrototypeSlice(base, base.length - suffix.length) === suffix) {
943+
return StringPrototypeSlice(base, 0, base.length - suffix.length);
944+
}
945+
}
946+
947+
return base;
980948
},
981949

982950
/**
@@ -1478,51 +1446,7 @@ const posix = {
14781446
let end = -1;
14791447
let matchedSlash = true;
14801448

1481-
if (suffix !== undefined && suffix.length > 0 && suffix.length <= path.length) {
1482-
if (suffix === path)
1483-
return '';
1484-
let extIdx = suffix.length - 1;
1485-
let firstNonSlashEnd = -1;
1486-
for (let i = path.length - 1; i >= 0; --i) {
1487-
const code = StringPrototypeCharCodeAt(path, i);
1488-
if (code === CHAR_FORWARD_SLASH) {
1489-
// If we reached a path separator that was not part of a set of path
1490-
// separators at the end of the string, stop now
1491-
if (!matchedSlash) {
1492-
start = i + 1;
1493-
break;
1494-
}
1495-
} else {
1496-
if (firstNonSlashEnd === -1) {
1497-
// We saw the first non-path separator, remember this index in case
1498-
// we need it if the extension ends up not matching
1499-
matchedSlash = false;
1500-
firstNonSlashEnd = i + 1;
1501-
}
1502-
if (extIdx >= 0) {
1503-
// Try to match the explicit extension
1504-
if (code === StringPrototypeCharCodeAt(suffix, extIdx)) {
1505-
if (--extIdx === -1) {
1506-
// We matched the extension, so mark this as the end of our path
1507-
// component
1508-
end = i;
1509-
}
1510-
} else {
1511-
// Extension does not match, so our result is the entire path
1512-
// component
1513-
extIdx = -1;
1514-
end = firstNonSlashEnd;
1515-
}
1516-
}
1517-
}
1518-
}
1519-
1520-
if (start === end)
1521-
end = firstNonSlashEnd;
1522-
else if (end === -1)
1523-
end = path.length;
1524-
return StringPrototypeSlice(path, start, end);
1525-
}
1449+
// Find the basename by scanning backwards, skipping trailing slashes
15261450
for (let i = path.length - 1; i >= 0; --i) {
15271451
if (StringPrototypeCharCodeAt(path, i) === CHAR_FORWARD_SLASH) {
15281452
// If we reached a path separator that was not part of a set of path
@@ -1541,7 +1465,19 @@ const posix = {
15411465

15421466
if (end === -1)
15431467
return '';
1544-
return StringPrototypeSlice(path, start, end);
1468+
1469+
const base = StringPrototypeSlice(path, start, end);
1470+
1471+
// Strip suffix if provided and the basename ends with it
1472+
if (suffix !== undefined && suffix.length > 0 && suffix.length <= base.length) {
1473+
if (base === suffix)
1474+
return '';
1475+
if (StringPrototypeSlice(base, base.length - suffix.length) === suffix) {
1476+
return StringPrototypeSlice(base, 0, base.length - suffix.length);
1477+
}
1478+
}
1479+
1480+
return base;
15451481
},
15461482

15471483
/**

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

Lines changed: 21 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -18,14 +18,14 @@ assert.strictEqual(path.basename('basename.ext/'), 'basename.ext');
1818
assert.strictEqual(path.basename('basename.ext//'), 'basename.ext');
1919
assert.strictEqual(path.basename('aaa/bbb', '/bbb'), 'bbb');
2020
assert.strictEqual(path.basename('aaa/bbb', 'a/bbb'), 'bbb');
21-
assert.strictEqual(path.basename('aaa/bbb', 'bbb'), 'bbb');
22-
assert.strictEqual(path.basename('aaa/bbb//', 'bbb'), 'bbb');
21+
assert.strictEqual(path.basename('aaa/bbb', 'bbb'), '');
22+
assert.strictEqual(path.basename('aaa/bbb//', 'bbb'), '');
2323
assert.strictEqual(path.basename('aaa/bbb', 'bb'), 'b');
2424
assert.strictEqual(path.basename('aaa/bbb', 'b'), 'bb');
2525
assert.strictEqual(path.basename('/aaa/bbb', '/bbb'), 'bbb');
2626
assert.strictEqual(path.basename('/aaa/bbb', 'a/bbb'), 'bbb');
27-
assert.strictEqual(path.basename('/aaa/bbb', 'bbb'), 'bbb');
28-
assert.strictEqual(path.basename('/aaa/bbb//', 'bbb'), 'bbb');
27+
assert.strictEqual(path.basename('/aaa/bbb', 'bbb'), '');
28+
assert.strictEqual(path.basename('/aaa/bbb//', 'bbb'), '');
2929
assert.strictEqual(path.basename('/aaa/bbb', 'bb'), 'b');
3030
assert.strictEqual(path.basename('/aaa/bbb', 'b'), 'bb');
3131
assert.strictEqual(path.basename('/aaa/bbb'), 'bbb');
@@ -44,8 +44,8 @@ assert.strictEqual(path.win32.basename('basename.ext\\\\'), 'basename.ext');
4444
assert.strictEqual(path.win32.basename('foo'), 'foo');
4545
assert.strictEqual(path.win32.basename('aaa\\bbb', '\\bbb'), 'bbb');
4646
assert.strictEqual(path.win32.basename('aaa\\bbb', 'a\\bbb'), 'bbb');
47-
assert.strictEqual(path.win32.basename('aaa\\bbb', 'bbb'), 'bbb');
48-
assert.strictEqual(path.win32.basename('aaa\\bbb\\\\\\\\', 'bbb'), 'bbb');
47+
assert.strictEqual(path.win32.basename('aaa\\bbb', 'bbb'), '');
48+
assert.strictEqual(path.win32.basename('aaa\\bbb\\\\\\\\', 'bbb'), '');
4949
assert.strictEqual(path.win32.basename('aaa\\bbb', 'bb'), 'b');
5050
assert.strictEqual(path.win32.basename('aaa\\bbb', 'b'), 'bb');
5151
assert.strictEqual(path.win32.basename('C:'), '');
@@ -60,6 +60,21 @@ assert.strictEqual(path.win32.basename('C:foo'), 'foo');
6060
assert.strictEqual(path.win32.basename('file:stream'), 'file:stream');
6161
assert.strictEqual(path.win32.basename('a', 'a'), '');
6262

63+
// Regression tests for https://github.com/nodejs/node/issues/21358
64+
// Suffix stripping should work consistently regardless of trailing slashes
65+
// or leading path components.
66+
assert.strictEqual(path.posix.basename('a/', 'a'), '');
67+
assert.strictEqual(path.posix.basename('a//', 'a'), '');
68+
assert.strictEqual(path.posix.basename('/dd', 'dd'), '');
69+
assert.strictEqual(path.posix.basename('d/dd', 'dd'), '');
70+
assert.strictEqual(path.posix.basename('d/dd/', 'dd'), '');
71+
assert.strictEqual(path.posix.basename('d/dd/', 'd'), 'd');
72+
assert.strictEqual(path.win32.basename('a\\', 'a'), '');
73+
assert.strictEqual(path.win32.basename('a\\\\', 'a'), '');
74+
assert.strictEqual(path.win32.basename('\\dd', 'dd'), '');
75+
assert.strictEqual(path.win32.basename('d\\dd', 'dd'), '');
76+
assert.strictEqual(path.win32.basename('d\\dd\\', 'dd'), '');
77+
6378
// On unix a backslash is just treated as any other character.
6479
assert.strictEqual(path.posix.basename('\\dir\\basename.ext'),
6580
'\\dir\\basename.ext');

0 commit comments

Comments
 (0)