-
-
Notifications
You must be signed in to change notification settings - Fork 34.3k
fs: add followSymlinks option to glob and globSync #61317
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -16,8 +16,8 @@ const { | |
| StringPrototypeEndsWith, | ||
| } = primordials; | ||
|
|
||
| const { lstatSync, readdirSync } = require('fs'); | ||
| const { lstat, readdir } = require('fs/promises'); | ||
| const { lstatSync, readdirSync, statSync: fsStatSync } = require('fs'); | ||
| const { lstat, readdir, stat: fsStat } = require('fs/promises'); | ||
|
Comment on lines
+19
to
+20
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Why are these being renamed?
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
If you believe that I should make the change, do let me know, I'll commit and push ASAP :) |
||
| const { join, resolve, basename, isAbsolute, dirname } = require('path'); | ||
|
|
||
| const { | ||
|
|
@@ -182,12 +182,14 @@ class Pattern { | |
| indexes; | ||
| symlinks; | ||
| last; | ||
| visited; | ||
|
|
||
| constructor(pattern, globStrings, indexes, symlinks) { | ||
| constructor(pattern, globStrings, indexes, symlinks, visited = null) { | ||
| this.#pattern = pattern; | ||
| this.#globStrings = globStrings; | ||
| this.indexes = indexes; | ||
| this.symlinks = symlinks; | ||
| this.visited = visited; | ||
| this.last = pattern.length - 1; | ||
| } | ||
|
|
||
|
|
@@ -205,8 +207,8 @@ class Pattern { | |
| at(index) { | ||
| return ArrayPrototypeAt(this.#pattern, index); | ||
| } | ||
| child(indexes, symlinks = new SafeSet()) { | ||
| return new Pattern(this.#pattern, this.#globStrings, indexes, symlinks); | ||
| child(indexes, symlinks = new SafeSet(), visited = null) { | ||
| return new Pattern(this.#pattern, this.#globStrings, indexes, symlinks, visited ?? this.visited); | ||
| } | ||
| test(index, path) { | ||
| if (index > this.#pattern.length) { | ||
|
|
@@ -264,12 +266,14 @@ class Glob { | |
| #subpatterns = new SafeMap(); | ||
| #patterns; | ||
| #withFileTypes; | ||
| #followSymlinks; | ||
| #isExcluded = () => false; | ||
| constructor(pattern, options = kEmptyObject) { | ||
| validateObject(options, 'options'); | ||
| const { exclude, cwd, withFileTypes } = options; | ||
| const { exclude, cwd, withFileTypes, followSymlinks } = options; | ||
| this.#root = toPathIfFileURL(cwd) ?? '.'; | ||
| this.#withFileTypes = !!withFileTypes; | ||
| this.#followSymlinks = !!followSymlinks; | ||
| if (exclude != null) { | ||
| validateStringArrayOrFunction(exclude, 'options.exclude'); | ||
| if (ArrayIsArray(exclude)) { | ||
|
|
@@ -429,6 +433,28 @@ class Glob { | |
| const entryPath = join(path, entry.name); | ||
| this.#cache.addToStatCache(join(fullpath, entry.name), entry); | ||
|
|
||
| let isDirectory = entry.isDirectory(); | ||
| let symlinkStat = null; | ||
| if (entry.isSymbolicLink() && this.#followSymlinks) { | ||
| const stat = fsStatSync(join(fullpath, entry.name)); | ||
| isDirectory = stat.isDirectory(); | ||
| if (isDirectory) { | ||
| // Check for cycles by looking at visited inodes | ||
| let node = pattern.visited; | ||
| while (node) { | ||
| if (node.ino === stat.ino && node.dev === stat.dev) { | ||
| // Cycle detected, skip this symlink | ||
| isDirectory = false; | ||
| break; | ||
| } | ||
| node = node.parent; | ||
| } | ||
| if (isDirectory) { | ||
| symlinkStat = stat; | ||
| } | ||
| } | ||
| } | ||
|
|
||
| const subPatterns = new SafeSet(); | ||
| const nSymlinks = new SafeSet(); | ||
| for (const index of pattern.indexes) { | ||
|
|
@@ -456,7 +482,7 @@ class Glob { | |
| (this.#exclude && this.#exclude(this.#withFileTypes ? entry : entry.name))) { | ||
| continue; | ||
| } | ||
| if (!fromSymlink && entry.isDirectory()) { | ||
| if (!fromSymlink && isDirectory) { | ||
| // If directory, add ** to its potential patterns | ||
| subPatterns.add(index); | ||
| } else if (!fromSymlink && index === last) { | ||
|
|
@@ -469,24 +495,24 @@ class Glob { | |
| if (nextMatches && nextIndex === last && !isLast) { | ||
| // If next pattern is the last one, add to results | ||
| this.#results.add(entryPath); | ||
| } else if (nextMatches && entry.isDirectory()) { | ||
| } else if (nextMatches && isDirectory) { | ||
| // Pattern matched, meaning two patterns forward | ||
| // are also potential patterns | ||
| // e.g **/b/c when entry is a/b - add c to potential patterns | ||
| subPatterns.add(index + 2); | ||
| } | ||
| if ((nextMatches || pattern.at(0) === '.') && | ||
| (entry.isDirectory() || entry.isSymbolicLink()) && !fromSymlink) { | ||
| (isDirectory || entry.isSymbolicLink()) && !fromSymlink) { | ||
| // If pattern after ** matches, or pattern starts with "." | ||
| // and entry is a directory or symlink, add to potential patterns | ||
| subPatterns.add(nextIndex); | ||
| } | ||
|
|
||
| if (entry.isSymbolicLink()) { | ||
| if (entry.isSymbolicLink() && !this.#followSymlinks) { | ||
| nSymlinks.add(index); | ||
| } | ||
|
|
||
| if (next === '..' && entry.isDirectory()) { | ||
| if (next === '..' && isDirectory) { | ||
| // In case pattern is "**/..", | ||
| // both parent and current directory should be added to the queue | ||
| // if this is the last pattern, add to results instead | ||
|
|
@@ -529,14 +555,19 @@ class Glob { | |
| // add next pattern to potential patterns, or to results if it's the last pattern | ||
| if (index === last) { | ||
| this.#results.add(entryPath); | ||
| } else if (entry.isDirectory()) { | ||
| } else if (isDirectory) { | ||
| subPatterns.add(nextIndex); | ||
| } | ||
| } | ||
| } | ||
| if (subPatterns.size > 0) { | ||
| // If there are potential patterns, add to queue | ||
| this.#addSubpattern(entryPath, pattern.child(subPatterns, nSymlinks)); | ||
| let newVisited = pattern.visited; | ||
| if (symlinkStat) { | ||
| // Add this symlink target to the visited chain | ||
| newVisited = { __proto__: null, ino: symlinkStat.ino, dev: symlinkStat.dev, parent: pattern.visited }; | ||
| } | ||
| this.#addSubpattern(entryPath, pattern.child(subPatterns, nSymlinks, newVisited)); | ||
| } | ||
| } | ||
| } | ||
|
|
@@ -639,6 +670,28 @@ class Glob { | |
| const entryPath = join(path, entry.name); | ||
| this.#cache.addToStatCache(join(fullpath, entry.name), entry); | ||
|
|
||
| let isDirectory = entry.isDirectory(); | ||
| let symlinkStat = null; | ||
| if (entry.isSymbolicLink() && this.#followSymlinks) { | ||
| const s = await fsStat(join(fullpath, entry.name)); | ||
| isDirectory = s.isDirectory(); | ||
| if (isDirectory) { | ||
| // Check for cycles by looking at visited inodes | ||
| let node = pattern.visited; | ||
| while (node) { | ||
| if (node.ino === s.ino && node.dev === s.dev) { | ||
| // Cycle detected, skip this symlink | ||
| isDirectory = false; | ||
| break; | ||
| } | ||
| node = node.parent; | ||
| } | ||
| if (isDirectory) { | ||
| symlinkStat = s; | ||
| } | ||
| } | ||
| } | ||
|
|
||
| const subPatterns = new SafeSet(); | ||
| const nSymlinks = new SafeSet(); | ||
| for (const index of pattern.indexes) { | ||
|
|
@@ -666,7 +719,7 @@ class Glob { | |
| (this.#exclude && this.#exclude(this.#withFileTypes ? entry : entry.name))) { | ||
| continue; | ||
| } | ||
| if (!fromSymlink && entry.isDirectory()) { | ||
| if (!fromSymlink && isDirectory) { | ||
| // If directory, add ** to its potential patterns | ||
| subPatterns.add(index); | ||
| } else if (!fromSymlink && index === last) { | ||
|
|
@@ -683,24 +736,24 @@ class Glob { | |
| if (!this.#results.has(entryPath) && this.#results.add(entryPath)) { | ||
| yield this.#withFileTypes ? entry : entryPath; | ||
| } | ||
| } else if (nextMatches && entry.isDirectory()) { | ||
| } else if (nextMatches && isDirectory) { | ||
| // Pattern matched, meaning two patterns forward | ||
| // are also potential patterns | ||
| // e.g **/b/c when entry is a/b - add c to potential patterns | ||
| subPatterns.add(index + 2); | ||
| } | ||
| if ((nextMatches || pattern.at(0) === '.') && | ||
| (entry.isDirectory() || entry.isSymbolicLink()) && !fromSymlink) { | ||
| (isDirectory || entry.isSymbolicLink()) && !fromSymlink) { | ||
| // If pattern after ** matches, or pattern starts with "." | ||
| // and entry is a directory or symlink, add to potential patterns | ||
| subPatterns.add(nextIndex); | ||
| } | ||
|
|
||
| if (entry.isSymbolicLink()) { | ||
| if (entry.isSymbolicLink() && !this.#followSymlinks) { | ||
| nSymlinks.add(index); | ||
| } | ||
|
|
||
| if (next === '..' && entry.isDirectory()) { | ||
| if (next === '..' && isDirectory) { | ||
| // In case pattern is "**/..", | ||
| // both parent and current directory should be added to the queue | ||
| // if this is the last pattern, add to results instead | ||
|
|
@@ -759,14 +812,18 @@ class Glob { | |
| yield this.#withFileTypes ? entry : entryPath; | ||
| } | ||
| } | ||
| } else if (entry.isDirectory()) { | ||
| } else if (isDirectory) { | ||
| subPatterns.add(nextIndex); | ||
| } | ||
| } | ||
| } | ||
| if (subPatterns.size > 0) { | ||
| // If there are potential patterns, add to queue | ||
| this.#addSubpattern(entryPath, pattern.child(subPatterns, nSymlinks)); | ||
| let newVisited = pattern.visited; | ||
| if (symlinkStat) { | ||
| newVisited = { __proto__: null, ino: symlinkStat.ino, dev: symlinkStat.dev, parent: pattern.visited }; | ||
| } | ||
| this.#addSubpattern(entryPath, pattern.child(subPatterns, nSymlinks, newVisited)); | ||
| } | ||
| } | ||
| } | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,34 @@ | ||
| // test-stat-identity.mjs | ||
| import { statSync, rmSync } from 'fs'; | ||
| import { mkdirSync, symlinkSync } from 'fs'; | ||
| import { join, resolve } from 'path'; | ||
|
|
||
| const tmp = resolve('tmp_stat_test'); | ||
| try { | ||
| rmSync(tmp, { recursive: true, force: true }); | ||
| } catch {} | ||
| try { | ||
| mkdirSync(tmp); | ||
| } catch {} | ||
|
|
||
| const dir = join(tmp, 'real_dir'); | ||
| try { | ||
| mkdirSync(dir); | ||
| } catch {} | ||
|
|
||
| const link1 = join(tmp, 'link1'); | ||
| // Link points to 'real_dir' which is in the same folder as 'link1' | ||
| try { | ||
| symlinkSync('real_dir', link1); | ||
| } catch {} | ||
|
|
||
| try { | ||
| const s1 = statSync(dir); | ||
| const s2 = statSync(link1); | ||
|
|
||
| console.log(`Real: dev=${s1.dev}, ino=${s1.ino}`); | ||
| console.log(`Link: dev=${s2.dev}, ino=${s2.ino}`); | ||
| console.log(`Match? ${s1.dev === s2.dev && s1.ino === s2.ino}`); | ||
| } catch (e) { | ||
| console.error(e); | ||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -2,7 +2,7 @@ import * as common from '../common/index.mjs'; | |
| import tmpdir from '../common/tmpdir.js'; | ||
| 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 } from 'node:fs'; | ||
| import { glob, globSync, Dirent, chmodSync, mkdirSync, writeFileSync, symlinkSync } from 'node:fs'; | ||
| import { test, describe } from 'node:test'; | ||
| import { pathToFileURL } from 'node:url'; | ||
| import { promisify } from 'node:util'; | ||
|
|
@@ -543,3 +543,67 @@ describe('glob - with restricted directory', function() { | |
| } | ||
| }); | ||
| }); | ||
|
|
||
| describe('glob followSymlinks', () => { | ||
| test('should not throw ELOOP with followSymlinks on symlink loop (async)', async () => { | ||
| if (common.isWindows) return; | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. can you explain why we completely bail on this test if we're on Windows?
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The setup function excluded it specifically as well. I did not question why. From what I am understanding, symlinks need admin permissions or developer mode enabled on Windows which is perhaps why the test does not exist for it. |
||
| const files = []; | ||
| for await (const file of asyncGlob('**', { cwd: fixtureDir, followSymlinks: true })) { | ||
| files.push(file); | ||
| } | ||
| // Should have results but no infinite loop | ||
| assert.ok(files.length > 0); | ||
| assert.ok(files.some((f) => f === 'a/symlink/a/b/c')); | ||
| assert.ok(files.some((f) => f.startsWith('a/symlink/a/b/c/a/'))); | ||
| const depth = Math.max(...files.filter((f) => f.startsWith('a/symlink/')).map((f) => f.split('/').length)); | ||
| assert.ok(depth < 20); | ||
| }); | ||
|
|
||
| test('should not throw ELOOP with followSymlinks on symlink loop (sync)', () => { | ||
| if (common.isWindows) return; | ||
| const files = globSync('**', { cwd: fixtureDir, followSymlinks: true }); | ||
| assert.ok(files.length > 0); | ||
| assert.ok(files.some((f) => f === 'a/symlink/a/b/c')); | ||
| assert.ok(files.some((f) => f.startsWith('a/symlink/a/b/c/a/'))); | ||
| const depth = Math.max(...files.filter((f) => f.startsWith('a/symlink/')).map((f) => f.split('/').length)); | ||
| assert.ok(depth < 20); | ||
| }); | ||
|
|
||
| test('should handle symlinks without cycles (async)', async () => { | ||
| if (common.isWindows) return; | ||
| const deepLinkDir = tmpdir.resolve('deep-links'); | ||
| await mkdir(deepLinkDir, { recursive: true }); | ||
|
|
||
| await mkdir(join(deepLinkDir, 'level1'), { recursive: true }); | ||
| await mkdir(join(deepLinkDir, 'level1', 'level2'), { recursive: true }); | ||
| await writeFile(join(deepLinkDir, 'level1', 'level2', 'file.txt'), 'deep'); | ||
| await symlink('level1/level2', join(deepLinkDir, 'link-to-level2')); | ||
|
|
||
| const files = []; | ||
| for await (const file of asyncGlob('**/*.txt', { cwd: deepLinkDir, followSymlinks: true })) { | ||
| files.push(file); | ||
| } | ||
|
|
||
| assert.ok(files.some((f) => f.includes('level1/level2/file.txt'))); | ||
| assert.ok(files.some((f) => f.includes('link-to-level2/file.txt'))); | ||
| }); | ||
|
|
||
| test('should handle symlinks without cycles (sync)', () => { | ||
| if (common.isWindows) return; | ||
| const deepLinkDir = tmpdir.resolve('deep-links-sync'); | ||
| try { | ||
| mkdirSync(deepLinkDir, { recursive: true }); | ||
| mkdirSync(join(deepLinkDir, 'level1'), { recursive: true }); | ||
| mkdirSync(join(deepLinkDir, 'level1', 'level2'), { recursive: true }); | ||
| writeFileSync(join(deepLinkDir, 'level1', 'level2', 'file.txt'), 'deep'); | ||
| symlinkSync('level1/level2', join(deepLinkDir, 'link-to-level2')); | ||
|
|
||
| const files = globSync('**/*.txt', { cwd: deepLinkDir, followSymlinks: true }); | ||
|
|
||
| assert.ok(files.some((f) => f.includes('level1/level2/file.txt'))); | ||
| assert.ok(files.some((f) => f.includes('link-to-level2/file.txt'))); | ||
| } catch { | ||
| // Cleanup errors are ok | ||
| } | ||
| }); | ||
| }); | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Make sure to update the changes entry above
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What should be the version tag for the change?