Skip to content

fix: keep an escaped pipe literal when the pattern has magic - #322

Open
NuubEitrigg wants to merge 1 commit into
isaacs:mainfrom
NuubEitrigg:fix-escaped-pipe
Open

NuubEitrigg wants to merge 1 commit into
isaacs:mainfrom
NuubEitrigg:fix-escaped-pipe

Conversation

@NuubEitrigg

Copy link
Copy Markdown

When a glob contains an escaped pipe (\|) and any other magic, the escaped character is written into the generated regular expression without its backslash. A bare | in a regular expression is an alternation, so the pattern is silently split in two at that point.

On 10.2.6:

minimatch('',    '.*\\|')   // true,  should be false
minimatch('xy',  '*\\|*')   // true,  should be false
minimatch('a?b', '\\|?-')   // true,  should be false
new Minimatch('.*\\|').makeRe()
// /^(?!(?:^|\/)\.\.?(?:$|\/))\.[^/]*?|$/   <- bare | before the $

Cause

#parseGlob in src/ast.ts re-escapes an escaped glob character only if it is in the reSpecials set, and that set had every regexp special except |. Patterns with no magic never reach this path (they are compared as strings), and an unescaped pipe outside an extglob was already handled by regExpEscape, so only the escaped form was wrong.

Fix

Add | to reSpecials. \| is a valid identity escape with and without the u flag, so this is safe for patterns that also use POSIX classes.

Tests

  • New test/escaped-pipe.ts covers the escaped pipe alone, with magic, inside extglob alternatives, and the unescaped form that already worked.
  • Four snapshots change, all for the same bash-derived pattern +(a|*\|c\\|d\\\|e\\\\|f\\\\\|g. The old snapshot regexp contained the bare alternations; the new one escapes them. The match expectations in test/patterns.js for that pattern are unchanged and still pass.
  • Full suite: 6243 pass, 0 fail, 21 skipped (the existing win32 skips), coverage gate and lint clean.

Found by differential fuzzing against bash's [[ == ]] with extglob on; this was the only class of mismatch that was not a documented minimatch design choice.

🤖 Generated with Claude Code

When a glob contains an escaped pipe and any other magic, the escaped
character was written into the generated regular expression without a
backslash, because the set of characters that need re-escaping did not
include the pipe. A bare pipe in a regular expression is an alternation,
so the pattern was silently split in two at that point.

Examples on 10.2.6:

  minimatch('', '.*\|')          // true, should be false
  minimatch('a?b', '\|?-')       // true, should be false
  minimatch('xy', '*\|*')        // true, should be false
  minimatch('zzz', 'a\|*')       // false (correct), but only by luck

Patterns without magic were unaffected because they never reach the
regular expression path, and an unescaped pipe outside an extglob was
already handled by regExpEscape. Only the escaped form was wrong.

The four snapshot updates are the bash-derived pattern
+(a|*\|c\|d\\|e\\|f\\\|g, whose generated regexp previously
contained the bare alternations and now escapes them.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant