fix: keep an escaped pipe literal when the pattern has magic - #322
Open
NuubEitrigg wants to merge 1 commit into
Open
NuubEitrigg wants to merge 1 commit into
NuubEitrigg wants to merge 1 commit into
Conversation
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
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
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:
Cause
#parseGlobinsrc/ast.tsre-escapes an escaped glob character only if it is in thereSpecialsset, 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 byregExpEscape, so only the escaped form was wrong.Fix
Add
|toreSpecials.\|is a valid identity escape with and without theuflag, so this is safe for patterns that also use POSIX classes.Tests
test/escaped-pipe.tscovers the escaped pipe alone, with magic, inside extglob alternatives, and the unescaped form that already worked.+(a|*\|c\\|d\\\|e\\\\|f\\\\\|g. The old snapshot regexp contained the bare alternations; the new one escapes them. The match expectations intest/patterns.jsfor that pattern are unchanged and still pass.Found by differential fuzzing against bash's
[[ == ]]withextglobon; this was the only class of mismatch that was not a documented minimatch design choice.🤖 Generated with Claude Code