From 83dee11c8b2d246a57082e5abc66be00d41c97b0 Mon Sep 17 00:00:00 2001 From: webdevelopersrinu Date: Sat, 1 Aug 2026 09:43:03 +0530 Subject: [PATCH] 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: https://github.com/nodejs/node/issues/62897 Co-authored-by: semimikoh --- lib/internal/fs/glob.js | 6 --- test/parallel/test-fs-glob.mjs | 76 ++++++++++++++++++++++++++++++++++ 2 files changed, 76 insertions(+), 6 deletions(-) diff --git a/lib/internal/fs/glob.js b/lib/internal/fs/glob.js index c608016833f9..efa590a55749 100644 --- a/lib/internal/fs/glob.js +++ b/lib/internal/fs/glob.js @@ -576,9 +576,6 @@ class Glob { const nSymlinks = new SafeSet(); for (const index of pattern.indexes) { // For each child, check potential patterns - if (this.#cache.seen(entryPath, pattern, index) || this.#cache.seen(entryPath, pattern, index + 1)) { - return; - } const current = pattern.at(index); const nextIndex = index + 1; const next = pattern.at(nextIndex); @@ -793,9 +790,6 @@ class Glob { const nSymlinks = new SafeSet(); for (const index of pattern.indexes) { // For each child, check potential patterns - if (this.#cache.seen(entryPath, pattern, index) || this.#cache.seen(entryPath, pattern, index + 1)) { - return; - } const current = pattern.at(index); const nextIndex = index + 1; const next = pattern.at(nextIndex); diff --git a/test/parallel/test-fs-glob.mjs b/test/parallel/test-fs-glob.mjs index bd95bce7d0e3..9226e491358d 100644 --- a/test/parallel/test-fs-glob.mjs +++ b/test/parallel/test-fs-glob.mjs @@ -1,5 +1,6 @@ import * as common from '../common/index.mjs'; import tmpdir from '../common/tmpdir.js'; +import { spawnSync } from 'node:child_process'; import { resolve, dirname, sep, relative, join, isAbsolute } from 'node:path'; import { mkdir, writeFile, symlink, glob as asyncGlob } from 'node:fs/promises'; import { glob, globSync, Dirent, chmodSync, writeFileSync, rmSync } from 'node:fs'; @@ -669,3 +670,78 @@ describe('globSync - ENOTDIR', function() { } }); }); + +describe('glob - seen cache', function() { + // Refs: https://github.com/nodejs/node/issues/62897 + test('does not skip siblings after a seen child path', () => { + // The glob traversal used to return early from the children loop when a + // child path had already been seen through a different pattern context, + // silently dropping the remaining siblings. Whether the bug triggered + // depended on directory iteration order, so the child process pins the + // order by patching readdir before loading the glob implementation. + const script = ` + const assert = require('node:assert'); + const fs = require('node:fs'); + const fsPromises = require('node:fs/promises'); + const path = require('node:path'); + + const cwd = process.argv[1]; + const a = path.join(cwd, 'a'); + fs.mkdirSync(path.join(a, 'b', 'c', 'd'), { recursive: true }); + fs.mkdirSync(path.join(a, 'c', 'd', 'c'), { recursive: true }); + fs.writeFileSync(path.join(a, 'x'), ''); + fs.writeFileSync(path.join(a, 'z'), ''); + + const originalReaddirSync = fs.readdirSync; + const originalReaddir = fsPromises.readdir; + + const reorder = (target, entries) => { + if (!Array.isArray(entries) || target !== a) return entries; + const names = ['c', 'b', 'x', 'z']; + return names.map((name) => entries.find((entry) => entry.name === name)) + .filter(Boolean); + }; + + fs.readdirSync = function(target, options) { + return reorder(target, originalReaddirSync.call(this, target, options)); + }; + fsPromises.readdir = async function(target, options) { + return reorder(target, await originalReaddir.call(this, target, options)); + }; + + const { Glob } = require('internal/fs/glob'); + const expected = ['a/b', 'a/c', 'a/x', 'a/z']; + const normalize = (results) => + results.map((item) => item.replaceAll(path.sep, '/')).sort(); + + (async () => { + const syncResults = normalize(new Glob('a/**/../*', { cwd }).globSync()); + for (const item of expected) { + assert.ok(syncResults.includes(item), + \`missing \${item} from sync results: \${syncResults}\`); + } + + const asyncResults = []; + for await (const item of new Glob('a/**/../*', { cwd }).glob()) { + asyncResults.push(item); + } + const normalized = normalize(asyncResults); + for (const item of expected) { + assert.ok(normalized.includes(item), + \`missing \${item} from async results: \${normalized}\`); + } + })().catch((err) => { + console.error(err); + process.exitCode = 1; + }); + `; + + const seenDir = tmpdir.resolve('glob-seen'); + const child = spawnSync( + process.execPath, + ['--expose-internals', '-e', script, seenDir], + { encoding: 'utf8' }, + ); + assert.strictEqual(child.status, 0, child.stderr || child.stdout); + }); +});