fixup! address review

Author: avivkeller

doc/api/fs.md

@@ -1450,7 +1450,7 @@ changes:
 * `options` {Object}
   * `cwd` {string|URL} current working directory. **Default:** `process.cwd()`
   * `exclude` {Function|string\[]} Function to filter out files/directories or a
-    list of glob patterns to be excluded. If a function is provided, return
+    list of [glob patterns][] to be excluded. If a function is provided, return
     `true` to exclude the item, `false` to include it. **Default:** `undefined`.
     If a string array is provided, each string should be a glob pattern that
     specifies paths to exclude. Note: Negation patterns (e.g., '!foo.js') are
@@ -1462,6 +1462,8 @@ changes:
 * Returns: {AsyncIterator} An AsyncIterator that yields the paths of files
   that match the pattern.
 
+See [Glob patterns][] for the syntax `pattern` accepts.
+
 When `followSymlinks` is enabled, detected symbolic link cycles are not
 traversed recursively.
 
@@ -2227,9 +2229,10 @@ added:
     queued than `maxQueue` allows. `'ignore'` means overflow events are dropped and a
     warning is emitted, while `'throw'` means to throw an exception. **Default:** `'ignore'`.
   * `ignore` {string|RegExp|Function|Array} Pattern(s) to ignore. Strings are
-    glob patterns, RegExp patterns are tested against
-    the filename, and functions receive the filename and return `true` to
-    ignore. **Default:** `undefined`.
+    [glob patterns][] that, when they contain no `/`, are matched against the
+    file's basename; RegExp patterns are tested against the filename, and
+    functions receive the filename and return `true` to ignore.
+    **Default:** `undefined`.
 * Returns: {AsyncIterator} of objects with the properties:
   * `eventType` {string} The type of change
   * `filename` {string|Buffer|null} The name of the file changed.
@@ -3641,7 +3644,7 @@ changes:
 * `options` {Object}
   * `cwd` {string|URL} current working directory. **Default:** `process.cwd()`
   * `exclude` {Function|string\[]} Function to filter out files/directories or a
-    list of glob patterns to be excluded. If a function is provided, return
+    list of [glob patterns][] to be excluded. If a function is provided, return
     `true` to exclude the item, `false` to include it. **Default:** `undefined`.
   * `followSymlinks` {boolean} When `true`, symbolic links to directories are
     followed while expanding `**` patterns. **Default:** `false`.
@@ -3653,6 +3656,8 @@ changes:
 
 * Retrieves the files matching the specified pattern.
 
+See [Glob patterns][] for the syntax `pattern` accepts.
+
 When `followSymlinks` is enabled, detected symbolic link cycles are not
 traversed recursively.
 
@@ -5300,9 +5305,10 @@ changes:
   * `throwIfNoEntry` {boolean} Indicates whether an exception should be thrown when the
     path does not exist. **Default:** `true`.
   * `ignore` {string|RegExp|Function|Array} Pattern(s) to ignore. Strings are
-    glob patterns, RegExp patterns are tested against
-    the filename, and functions receive the filename and return `true` to
-    ignore. **Default:** `undefined`.
+    [glob patterns][] that, when they contain no `/`, are matched against the
+    file's basename; RegExp patterns are tested against the filename, and
+    functions receive the filename and return `true` to ignore.
+    **Default:** `undefined`.
 * `listener` {Function|undefined} **Default:** `undefined`
   * `eventType` {string}
   * `filename` {string|Buffer|null}
@@ -6278,14 +6284,16 @@ changes:
 * `options` {Object}
   * `cwd` {string|URL} current working directory. **Default:** `process.cwd()`
   * `exclude` {Function|string\[]} Function to filter out files/directories or a
-    list of glob patterns to be excluded. If a function is provided, return
+    list of [glob patterns][] to be excluded. If a function is provided, return
     `true` to exclude the item, `false` to include it. **Default:** `undefined`.
   * `followSymlinks` {boolean} When `true`, symbolic links to directories are
     followed while expanding `**` patterns. **Default:** `false`.
   * `withFileTypes` {boolean} `true` if the glob should return paths as Dirents,
     `false` otherwise. **Default:** `false`.
 * Returns: {string\[]} paths of files that match the pattern.
 
+See [Glob patterns][] for the syntax `pattern` accepts.
+
 When `followSymlinks` is enabled, detected symbolic link cycles are not
 traversed recursively.
 
@@ -9201,6 +9209,101 @@ example `fs.readdirSync('C:\\')` can potentially return a different result than
 `fs.readdirSync('C:')`. For more information, see
 [this MSDN page][MSDN-Rel-Path].
 
+### Glob patterns
+
+[`fs.glob()`][], [`fs.globSync()`][], [`fsPromises.glob()`][] and
+[`path.matchesGlob()`][] take glob patterns, as do the string forms of the
+`exclude` and `ignore` options. The syntax follows the pattern matching of
+`bash`, with brace expansion and the extended `extglob` operators.
+
+The glob implementation Node.js uses was adopted from [`minimatch`][],
+so as a general rule of thumb, all minimatch-supported glob extensions
+work with `fs`. For simplicity, such extensions have been documented below:
+
+#### Path separators
+
+A pattern is always split on `/`, on every platform. A backslash in a pattern
+is treated as a path separator as well, and never as an escape character, so
+patterns built with `path.join()` on Windows still work. Repeated separators
+are collapsed, so `a//b` and `a/b` are the same pattern.
+
+#### Wildcards
+
+Wildcards match within a single path segment, and never match a `/`:
+
+| Pattern            | Matches                                    |
+| ------------------ | ------------------------------------------ |
+| `*`                | Any run of characters, including none      |
+| `?`                | Exactly one character                      |
+| `[abc]`            | Any one of the characters in the set       |
+| `[a-z]`            | Any one character in the range             |
+| `[!abc]`, `[^abc]` | Any one character not in the set           |
+| `[[:alpha:]]`      | Any one character in the named POSIX class |
+
+```js
+path.matchesGlob('src/index.js', 'src/*.js'); // true
+path.matchesGlob('src/lib/index.js', 'src/*.js'); // false
+path.matchesGlob('file1.txt', 'file[0-9].txt'); // true
+path.matchesGlob('é', '[[:alpha:]]'); // true
+```
+
+#### Globstar
+
+A `**` that makes up a whole path segment matches zero or more segments, so
+`a/**/b` matches both `a/b` and `a/x/y/b`.
+
+```js
+path.matchesGlob('src/a/b/index.js', 'src/**/*.js'); // true
+path.matchesGlob('src/index.js', 'src/**/*.js'); // true
+```
+
+#### Extended globs
+
+Each of these takes a `|`-separated list of alternatives, and they may nest:
+
+| Pattern   | Matches                         |
+| --------- | ------------------------------- |
+| `?(a\|b)` | Zero or one of the alternatives |
+| `*(a\|b)` | Zero or more of them            |
+| `+(a\|b)` | One or more of them             |
+| `@(a\|b)` | Exactly one of them             |
+| `!(a\|b)` | Anything except them            |
+
+```js
+path.matchesGlob('index.ts', '*.@(js|ts)'); // true
+path.matchesGlob('index.css', '!(*.js)'); // true
+```
+
+#### Brace expansion
+
+Braces expand to alternatives before anything else in the pattern is
+interpreted. `{a,b}` is a list, `{1..9}` and `{a..z}` are sequences, and a
+sequence may take a step (`{1..9..3}`) and keep zero padding (`{01..12}`).
+Braces nest. A brace group containing neither a comma nor a sequence is
+literal.
+
+```js
+path.matchesGlob('src/index.ts', 'src/*.{js,ts}'); // true
+path.matchesGlob('page3.html', 'page{1..5}.html'); // true
+path.matchesGlob('a{b}c', 'a{b}c'); // true
+```
+
+#### Dot files
+
+A path segment beginning with a `.` is matched only by a pattern segment
+beginning with a literal `.`, so `*` and `**` do not match dot files or
+directories.
+
+```js
+path.matchesGlob('.env', '*'); // false
+path.matchesGlob('.env', '.*'); // true
+```
+
+#### Case sensitivity
+
+Matching is case-sensitive, except on Windows and macOS, where the file system
+is not case-sensitive itself.
+
 ### File descriptors
 
 On POSIX systems, for every process, the kernel maintains a table of currently
@@ -9372,6 +9475,7 @@ the file contents.
 [FS constants]: #fs-constants
 [File access constants]: #file-access-constants
 [File modes]: #file-modes
+[Glob patterns]: #glob-patterns
 [MDN-Date]: https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Date
 [MDN-Number]: https://developer.mozilla.org/en-US/docs/Web/JavaScript/Guide/Data_structures#number_type
 [MSDN-Rel-Path]: https://docs.microsoft.com/en-us/windows/desktop/FileIO/naming-a-file#fully-qualified-vs-relative-paths
@@ -9401,6 +9505,8 @@ the file contents.
 [`fs.fstat()`]: #fsfstatfd-options-callback
 [`fs.ftruncate()`]: #fsftruncatefd-len-callback
 [`fs.futimes()`]: #fsfutimesfd-atime-mtime-callback
+[`fs.glob()`]: #fsglobpattern-options-callback
+[`fs.globSync()`]: #fsglobsyncpattern-options
 [`fs.lstat()`]: #fslstatpath-options-callback
 [`fs.lutimes()`]: #fslutimespath-atime-mtime-callback
 [`fs.mkdir()`]: #fsmkdirpath-options-callback
@@ -9429,6 +9535,7 @@ the file contents.
 [`fs.writev()`]: #fswritevfd-buffers-position-callback
 [`fsPromises.access()`]: #fspromisesaccesspath-mode
 [`fsPromises.copyFile()`]: #fspromisescopyfilesrc-dest-mode
+[`fsPromises.glob()`]: #fspromisesglobpattern-options
 [`fsPromises.mkdtemp()`]: #fspromisesmkdtempprefix-options
 [`fsPromises.open()`]: #fspromisesopenpath-flags-mode
 [`fsPromises.opendir()`]: #fspromisesopendirpath-options
@@ -9437,7 +9544,9 @@ the file contents.
 [`fsPromises.utimes()`]: #fspromisesutimespath-atime-mtime
 [`inotify(7)`]: https://man7.org/linux/man-pages/man7/inotify.7.html
 [`kqueue(2)`]: https://www.freebsd.org/cgi/man.cgi?query=kqueue&sektion=2
+[`minimatch`]: https://github.com/isaacs/minimatch
 [`node:stream/iter`]: stream_iter.md
+[`path.matchesGlob()`]: path.md#pathmatchesglobpath-pattern
 [`statfs.bsize`]: #statfsbsize
 [`stream.getDefaultHighWaterMark()`]: stream.md#streamgetdefaulthighwatermarkobjectmode
 [`stream/iter pipeTo()`]: stream_iter.md#pipetosource-transforms-writer-options

doc/api/path.md

@@ -302,6 +302,7 @@ changes:
 * Returns: {boolean} Whether or not the `path` matched the `pattern`.
 
 The `path.matchesGlob()` method determines if `path` matches the `pattern`.
+See [Glob patterns][] for the syntax `pattern` accepts.
 
 For example:
 
@@ -657,6 +658,7 @@ of the `path` methods.
 
 The API is accessible via `require('node:path').win32` or `require('node:path/win32')`.
 
+[Glob patterns]: fs.md#glob-patterns
 [MSDN-Rel-Path]: https://docs.microsoft.com/en-us/windows/desktop/FileIO/naming-a-file#fully-qualified-vs-relative-paths
 [`TypeError`]: errors.md#class-typeerror
 [`path.parse()`]: #pathparsepath

src/glob/glob_ast.h

@@ -10,6 +10,7 @@
 
 namespace node::glob {
 
+// UTF-16, because that is how patterns arrive from V8
 using PatternString = std::u16string;
 using PatternView = std::u16string_view;
 
@@ -44,15 +45,29 @@ bool IsWindowsDrive(std::basic_string_view<Char> s) {
   return s.size() == 2 && StartsWithWindowsDrive(s);
 }
 
+// Caps on what one pattern may cost, inherited from minimatch
 inline constexpr size_t kMaxPatternLength = 64 * 1024;
+// Nesting depth for extglobs like `*(a|@(b|c))`
 inline constexpr int kMaxExtglobRecursion = 2;
+// Non-adjacent `**` parts one match may recurse through
 inline constexpr int kMaxGlobstarRecursion = 200;
+// How far, in number of braces, braces may expand
 inline constexpr size_t kBraceExpansionMax = 100'000;
+// How far, in length, braces may expand
 inline constexpr size_t kBraceExpansionMaxLength = 4'000'000;
+// Braces and extglobs are parsed by recursing once per level of nesting, so
+// nesting deep enough to exhaust the C stack has to be refused up front:
+// `@(` and `{,` cost two or three code units per level, which leaves room for
+// thousands of levels under kMaxPatternLength. Minimatch has no such cap and
+// simply throws RangeError once V8's stack runs out. Real patterns nest a
+// handful of levels, and 256 keeps the parser under ~150 KB of stack even on
+// the smallest worker thread stacks.
+inline constexpr int kMaxNestingDepth = 256;
 
 enum class CompileError {
   kNone,
   kPatternTooLong,
+  kPatternTooDeep,
   kInvalidRegExp,
 };
 

src/glob/glob_matcher.cc

@@ -2,6 +2,7 @@
 
 #include <algorithm>
 #include <bit>
+#include <limits>
 #include <ranges>
 #include <span>
 
@@ -13,18 +14,25 @@ namespace node::glob {
 
 namespace {
 
-// A set of match positions within one segment.
+// The set of positions in the subject a segment's program can currently be
+// at. Carrying every position at once is what makes matching linear in
+// (positions x nodes) instead of backtracking, so a pattern like
+// `*(a|aa)*(a|aa)b` costs the same as any other.
 class PosSet {
  public:
   void Init(size_t positions) {
-    nwords_ = (positions + 63) / 64;
+    nwords_ = (positions + kBitsPerWord - 1) / kBitsPerWord;
     if (nwords_ > kInlineWords)
       heap_.assign(nwords_, 0);
     else
       std::fill_n(inline_, nwords_, 0);
   }
-  void Set(size_t i) { words()[i >> 6] |= uint64_t{1} << (i & 63); }
-  bool Test(size_t i) const { return (words()[i >> 6] >> (i & 63)) & 1; }
+  void Set(size_t i) {
+    words()[i >> kWordShift] |= uint64_t{1} << (i & kBitInWordMask);
+  }
+  bool Test(size_t i) const {
+    return (words()[i >> kWordShift] >> (i & kBitInWordMask)) & 1;
+  }
   bool Empty() const {
     return std::ranges::all_of(words(), [](uint64_t v) { return v == 0; });
   }
@@ -34,7 +42,7 @@ class PosSet {
     for (size_t w = 0; w < nwords_; w++) {
       uint64_t word = ws[w];
       while (word != 0) {
-        f(w * 64 + std::countr_zero(word));
+        f(w * kBitsPerWord + std::countr_zero(word));
         word &= word - 1;
       }
     }
@@ -46,7 +54,12 @@ class PosSet {
   }
 
  private:
-  static constexpr size_t kInlineWords = 4;  // 256 positions
+  static constexpr size_t kBitsPerWord = std::numeric_limits<uint64_t>::digits;
+  static constexpr size_t kWordShift = std::countr_zero(kBitsPerWord);
+  static constexpr size_t kBitInWordMask = kBitsPerWord - 1;
+  // Four words hold 256 positions, one more than the longest name the
+  // common filesystems allow, so only a synthetic subject reaches the heap.
+  static constexpr size_t kInlineWords = 4;
 
   std::span<uint64_t> words() {
     return {nwords_ <= kInlineWords ? inline_ : heap_.data(), nwords_};
@@ -306,7 +319,7 @@ bool EndsWith(std::basic_string_view<Char> s, PatternView suffix) {
 template <typename Char>
 bool EndsWithLowered(std::basic_string_view<Char> f, PatternView lowered_ext) {
   const bool ascii = std::ranges::all_of(
-      f, [](Char c) { return static_cast<uint32_t>(c) < 0x80; });
+      f, [](Char c) { return static_cast<uint32_t>(c) <= kMaxAsciiCodePoint; });
   if (ascii) {
     return f.size() >= lowered_ext.size() &&
            std::ranges::equal(lowered_ext,
@@ -446,6 +459,8 @@ class PathMatcher {
 
  private:
   using RowParts = std::vector<PartMatcher>;
+  // Path depth held without allocating. Deeper paths still work; they just
+  // grow the buffer, and they are rare enough not to matter.
   static constexpr size_t kInlineParts = 32;
 
   // Minimatch#slashSplit