From 59d6f4fc80eee648f6fad89581056186fe45f305 Mon Sep 17 00:00:00 2001 From: robobun <117481402+robobun@users.noreply.github.com> Date: Tue, 15 Sep 2026 18:55:36 +0000 Subject: [PATCH] fs.glob: do not drop sibling entries after a seen child path The children loop in the sync walker and in the async walker returned from the whole method when a child path was already in the seen cache. Every later sibling was dropped: - ['src/*.ts', './src/helpers/*.ts'] did not return 'src/index.ts'. Bun 1.3 returned it. - 'a/**/../*' did not return 'a', 'a/c', 'a/x' or 'a/z'. Remove the check at both sites, as Node.js did in v26.8.0 and v24.21.0: https://github.com/nodejs/node/commit/0ea2c86b5b70fdab268597e8c51040c703ee1328 Update test/js/node/test/parallel/test-fs-glob.mjs to the v26.8.2 copy, which adds the upstream test for this bug. --- src/js/internal/fs/glob.ts | 8 +-- test/js/node/fs/glob.test.ts | 52 ++++++++++++++ test/js/node/test/parallel/test-fs-glob.mjs | 78 ++++++++++++++++++++- 3 files changed, 131 insertions(+), 7 deletions(-) diff --git a/src/js/internal/fs/glob.ts b/src/js/internal/fs/glob.ts index bec1c1aeecd6..5abd9e5647f0 100644 --- a/src/js/internal/fs/glob.ts +++ b/src/js/internal/fs/glob.ts @@ -3,6 +3,8 @@ // // Port of Node.js lib/internal/fs/glob.js (v26.3.0): // https://github.com/nodejs/node/blob/50c35fea9e64d50ab3bb5f359e8523de89d6c798/lib/internal/fs/glob.js +// plus "fs: fix glob early return skipping sibling entries" (v26.8.0): +// https://github.com/nodejs/node/commit/0ea2c86b5b70fdab268597e8c51040c703ee1328 // backed by a vendored copy of minimatch (Node's deps/minimatch/index.js, ISC license): // https://github.com/nodejs/node/blob/50c35fea9e64d50ab3bb5f359e8523de89d6c798/deps/minimatch/index.js // embedded verbatim below lazyMinimatch(); the vendored block is third-party @@ -637,9 +639,6 @@ class Glob { const nSymlinks = new Set(); 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); @@ -856,9 +855,6 @@ class Glob { const nSymlinks = new Set(); 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/js/node/fs/glob.test.ts b/test/js/node/fs/glob.test.ts index 66392bf66513..4697e2c00759 100644 --- a/test/js/node/fs/glob.test.ts +++ b/test/js/node/fs/glob.test.ts @@ -5,6 +5,7 @@ import { afterAll, beforeAll, describe, expect, it } from "bun:test"; import { isWindows, tempDir, tempDirWithFiles } from "harness"; import fs from "node:fs"; +import { sep } from "node:path"; let tmp: string; beforeAll(() => { @@ -295,3 +296,54 @@ describe("fs.globSync exclude with withFileTypes", () => { expect(String(inners[0].parentPath).replaceAll("\\", "/")).toEndWith("keep"); }); }); + +// https://github.com/nodejs/node/issues/62897 +describe("a directory that is listed after one of its children was traversed", () => { + const src = { src: { helpers: { "a.ts": "", "b.ts": "" }, "index.ts": "" } }; + const cases = [ + { + // "a/**" matches "a" and every directory below it. "../*" lists the parent + // of each one: ".", "a", "a/b", "a/b/c", "a/c" and "a/c/d". + name: "'..' after '**'", + tree: { a: { b: { c: { d: {} } }, c: { d: { c: {} } }, x: "", z: "" } }, + pattern: "a/**/../*", + expected: ["a", "a/b", "a/b/c", "a/b/c/d", "a/c", "a/c/d", "a/c/d/c", "a/x", "a/z"], + }, + { + // The "./" pattern is traversed first, then "src/*.ts" lists "src". + // Entries are visited in sorted order, so "index.ts" must sort after "helpers". + name: "a pattern array with a './' pattern", + tree: src, + pattern: ["src/*.ts", "./src/helpers/*.ts"], + expected: ["src/helpers/a.ts", "src/helpers/b.ts", "src/index.ts"], + }, + { + name: "a brace group with a './' alternative", + tree: src, + pattern: "{src/*.ts,./src/helpers/*.ts}", + expected: ["src/helpers/a.ts", "src/helpers/b.ts", "src/index.ts"], + }, + ]; + + describe.each(cases)("$name", ({ tree, pattern, expected }) => { + const sorted = expected.map(path => path.replaceAll("/", sep)).sort(); + + it("fs.globSync", () => { + using dir = tempDir("fs-glob-seen", tree); + expect(fs.globSync(pattern, { cwd: String(dir) }).sort()).toEqual(sorted); + }); + + it("fs.promises.glob", async () => { + using dir = tempDir("fs-glob-seen", tree); + const paths = await Array.fromAsync(fs.promises.glob(pattern, { cwd: String(dir) })); + expect(paths.sort()).toEqual(sorted); + }); + + it("fs.glob", async () => { + using dir = tempDir("fs-glob-seen", tree); + const { promise, resolve, reject } = Promise.withResolvers(); + fs.glob(pattern, { cwd: String(dir) }, (err, paths) => (err ? reject(err) : resolve(paths))); + expect((await promise).sort()).toEqual(sorted); + }); + }); +}); diff --git a/test/js/node/test/parallel/test-fs-glob.mjs b/test/js/node/test/parallel/test-fs-glob.mjs index 560b4e72e4ad..9226e491358d 100644 --- a/test/js/node/test/parallel/test-fs-glob.mjs +++ b/test/js/node/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'; @@ -393,7 +394,7 @@ describe('fsPromises.glob - with file: URL as cwd', function() { const normalizeDirent = (dirent) => relative(fixtureDir, join(dirent.parentPath, dirent.name)); // The call to `join()` with only one argument is important, as -// it ensures that the proper path seperators are applied. +// it ensures that the proper path separators are applied. const normalizePath = (path) => (isAbsolute(path) ? relative(fixtureDir, path) : join(path)); describe('glob - withFileTypes', function() { @@ -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); + }); +});