Skip to content

Commit 83dee11

Browse files
fs: fix glob early return skipping sibling entries
The children loop in the glob traversal returned from the whole method when a child path had already been seen through a different pattern context, silently dropping the remaining sibling entries. Whether this triggered depended on directory iteration order, which also made test-fs-glob.mjs flaky. Remove the check: the cache.add call at the start of the traversal already prevents reprocessing. Fixes: #62897 Co-authored-by: semimikoh <ejffjeosms@gmail.com>
1 parent a6a1bd4 commit 83dee11

2 files changed

Lines changed: 76 additions & 6 deletions

File tree

lib/internal/fs/glob.js

Lines changed: 0 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -576,9 +576,6 @@ class Glob {
576576
const nSymlinks = new SafeSet();
577577
for (const index of pattern.indexes) {
578578
// For each child, check potential patterns
579-
if (this.#cache.seen(entryPath, pattern, index) || this.#cache.seen(entryPath, pattern, index + 1)) {
580-
return;
581-
}
582579
const current = pattern.at(index);
583580
const nextIndex = index + 1;
584581
const next = pattern.at(nextIndex);
@@ -793,9 +790,6 @@ class Glob {
793790
const nSymlinks = new SafeSet();
794791
for (const index of pattern.indexes) {
795792
// For each child, check potential patterns
796-
if (this.#cache.seen(entryPath, pattern, index) || this.#cache.seen(entryPath, pattern, index + 1)) {
797-
return;
798-
}
799793
const current = pattern.at(index);
800794
const nextIndex = index + 1;
801795
const next = pattern.at(nextIndex);

test/parallel/test-fs-glob.mjs

Lines changed: 76 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
import * as common from '../common/index.mjs';
22
import tmpdir from '../common/tmpdir.js';
3+
import { spawnSync } from 'node:child_process';
34
import { resolve, dirname, sep, relative, join, isAbsolute } from 'node:path';
45
import { mkdir, writeFile, symlink, glob as asyncGlob } from 'node:fs/promises';
56
import { glob, globSync, Dirent, chmodSync, writeFileSync, rmSync } from 'node:fs';
@@ -669,3 +670,78 @@ describe('globSync - ENOTDIR', function() {
669670
}
670671
});
671672
});
673+
674+
describe('glob - seen cache', function() {
675+
// Refs: https://github.com/nodejs/node/issues/62897
676+
test('does not skip siblings after a seen child path', () => {
677+
// The glob traversal used to return early from the children loop when a
678+
// child path had already been seen through a different pattern context,
679+
// silently dropping the remaining siblings. Whether the bug triggered
680+
// depended on directory iteration order, so the child process pins the
681+
// order by patching readdir before loading the glob implementation.
682+
const script = `
683+
const assert = require('node:assert');
684+
const fs = require('node:fs');
685+
const fsPromises = require('node:fs/promises');
686+
const path = require('node:path');
687+
688+
const cwd = process.argv[1];
689+
const a = path.join(cwd, 'a');
690+
fs.mkdirSync(path.join(a, 'b', 'c', 'd'), { recursive: true });
691+
fs.mkdirSync(path.join(a, 'c', 'd', 'c'), { recursive: true });
692+
fs.writeFileSync(path.join(a, 'x'), '');
693+
fs.writeFileSync(path.join(a, 'z'), '');
694+
695+
const originalReaddirSync = fs.readdirSync;
696+
const originalReaddir = fsPromises.readdir;
697+
698+
const reorder = (target, entries) => {
699+
if (!Array.isArray(entries) || target !== a) return entries;
700+
const names = ['c', 'b', 'x', 'z'];
701+
return names.map((name) => entries.find((entry) => entry.name === name))
702+
.filter(Boolean);
703+
};
704+
705+
fs.readdirSync = function(target, options) {
706+
return reorder(target, originalReaddirSync.call(this, target, options));
707+
};
708+
fsPromises.readdir = async function(target, options) {
709+
return reorder(target, await originalReaddir.call(this, target, options));
710+
};
711+
712+
const { Glob } = require('internal/fs/glob');
713+
const expected = ['a/b', 'a/c', 'a/x', 'a/z'];
714+
const normalize = (results) =>
715+
results.map((item) => item.replaceAll(path.sep, '/')).sort();
716+
717+
(async () => {
718+
const syncResults = normalize(new Glob('a/**/../*', { cwd }).globSync());
719+
for (const item of expected) {
720+
assert.ok(syncResults.includes(item),
721+
\`missing \${item} from sync results: \${syncResults}\`);
722+
}
723+
724+
const asyncResults = [];
725+
for await (const item of new Glob('a/**/../*', { cwd }).glob()) {
726+
asyncResults.push(item);
727+
}
728+
const normalized = normalize(asyncResults);
729+
for (const item of expected) {
730+
assert.ok(normalized.includes(item),
731+
\`missing \${item} from async results: \${normalized}\`);
732+
}
733+
})().catch((err) => {
734+
console.error(err);
735+
process.exitCode = 1;
736+
});
737+
`;
738+
739+
const seenDir = tmpdir.resolve('glob-seen');
740+
const child = spawnSync(
741+
process.execPath,
742+
['--expose-internals', '-e', script, seenDir],
743+
{ encoding: 'utf8' },
744+
);
745+
assert.strictEqual(child.status, 0, child.stderr || child.stdout);
746+
});
747+
});

0 commit comments

Comments
 (0)