treat invalid classes as non-matching patterns

isaacs · Dec 15, 2022 · 56a2b05 · 56a2b05
1 parent f6fd04d
commit 56a2b05
Show file tree

Hide file tree

Showing 5 changed files with 119 additions and 16 deletions.
diff --git a/minimatch.js b/minimatch.js
@@ -611,8 +611,6 @@ class Minimatch {
             continue
           }
 
-          // handle the case where we left a class open.
-          // "[z-a]" is valid, equivalent to "\[z-a\]"
           // split where the last [ was, make sure we don't have
           // an invalid re. if so, re-walk the contents of the
           // would-be class to re-translate any characters that
@@ -623,19 +621,15 @@ class Minimatch {
           cs = pattern.substring(classStart + 1, i)
           try {
             RegExp('[' + braExpEscape(charUnescape(cs)) + ']')
+            // looks good, finish up the class.
+            re += c
           } catch (er) {
-            // not a valid class!
-            sp = this.parse(cs, SUBPARSE)
-            re = re.substring(0, reClassStart) + '\\[' + sp[0] + '\\]'
-            hasMagic = hasMagic || sp[1]
-            inClass = false
-            continue
+            // out of order ranges in JS are errors, but in glob syntax,
+            // they're just a range that matches nothing.
+            re = re.substring(0, reClassStart) + '(?:$.)' // match nothing ever
           }
-
-          // finish up the class.
           hasMagic = true
           inClass = false
-          re += c
         continue
 
         default:

diff --git a/tap-snapshots/test/basic.js.test.cjs b/tap-snapshots/test/basic.js.test.cjs
@@ -198,11 +198,11 @@ exports[`test/basic.js TAP basic tests > makeRe [\\\\] 1`] = `
 `
 
 exports[`test/basic.js TAP basic tests > makeRe [\\b-a] 1`] = `
-/^(?:\\[b\\-a\\])$/
+/^(?:(?!\\.)(?=.)(?:$.))$/
 `
 
 exports[`test/basic.js TAP basic tests > makeRe [\\z-a] 1`] = `
-/^(?:\\[z\\-a\\])$/
+/^(?:(?!\\.)(?=.)(?:$.))$/
 `
 
 exports[`test/basic.js TAP basic tests > makeRe [] 1`] = `
@@ -226,7 +226,11 @@ exports[`test/basic.js TAP basic tests > makeRe [^a-c]* 1`] = `
 `
 
 exports[`test/basic.js TAP basic tests > makeRe [a-0][a-Ā] 1`] = `
-/^(?:(?=.)\\[a-0\\][a-Ā])$/
+/^(?:(?!\\.)(?=.)(?:$.)[a-Ā])$/
+`
+
+exports[`test/basic.js TAP basic tests > makeRe [a-b-c] 1`] = `
+/^(?:(?!\\.)(?=.)[a-b-c])$/
 `
 
 exports[`test/basic.js TAP basic tests > makeRe [a-c]b* 1`] = `
@@ -254,7 +258,7 @@ exports[`test/basic.js TAP basic tests > makeRe [ia]?[ck] 1`] = `
 `
 
 exports[`test/basic.js TAP basic tests > makeRe [z-a] 1`] = `
-/^(?:\\[z\\-a\\])$/
+/^(?:(?!\\.)(?=.)(?:$.))$/
 `
 
 exports[`test/basic.js TAP basic tests > makeRe [z\\-a] 1`] = `
@@ -345,10 +349,14 @@ exports[`test/basic.js TAP basic tests > makeRe a/.*/b 2`] = `
 /^(?:a\\/(?=.)\\.[^/]*?\\/b)$/
 `
 
-exports[`test/basic.js TAP basic tests > makeRe a/[2015-03-10T00:23:08.647Z]/z 1`] = `
+exports[`test/basic.js TAP basic tests > makeRe a/[2015-03-10T00:23:08.647Z\\]/z 1`] = `
 /^(?:a\\/\\[2015\\-03\\-10T00:23:08\\.647Z\\]\\/z)$/
 `
 
+exports[`test/basic.js TAP basic tests > makeRe a/[2015-03-10T00:23:08.647Z]/z 1`] = `
+/^(?:a\\/(?!\\.)(?=.)(?:$.)\\/z)$/
+`
+
 exports[`test/basic.js TAP basic tests > makeRe a?b 1`] = `
 /^(?:(?=.)a[^/]b)$/
 `

diff --git a/tap-snapshots/test/class-edge-cases.js.test.cjs b/tap-snapshots/test/class-edge-cases.js.test.cjs
@@ -0,0 +1,51 @@
+/* IMPORTANT
+ * This snapshot file is auto-generated, but designed for humans.
+ * It should be checked into source control and tracked carefully.
+ * Re-generate by setting TAP_SNAPSHOT=1 and running tests.
+ * Make sure to inspect the output below.  Do not ignore changes!
+ */
+'use strict'
+exports[`test/class-edge-cases.js TAP > must match snapshot 1`] = `
+Object {
+  "files": Array [
+    "a[]b",
+    "[b-a]x",
+    "a]b",
+    "a[]]b",
+    "a[[]b",
+  ],
+  "matches": Object {
+    "[b-a\\\\]*": Array [
+      "[b-a]x",
+    ],
+    "\\\\[b-a]*": Array [
+      "[b-a]x",
+    ],
+    "a[[]]*": Array [
+      "a[]b",
+      "a[]]b",
+    ],
+    "a[[]*": Array [
+      "a[]b",
+      "a[]]b",
+      "a[[]b",
+    ],
+    "a[]]*": Array [
+      "a]b",
+    ],
+    "a[]*": Array [
+      "a[]b",
+      "a[]]b",
+    ],
+  },
+  "patterns": Array [
+    "\\\\[b-a]*",
+    "[b-a\\\\]*",
+    "a[]*",
+    "[b-a]*",
+    "a[]]*",
+    "a[[]*",
+    "a[[]]*",
+  ],
+}
+`
diff --git a/test/class-edge-cases.js b/test/class-edge-cases.js
@@ -0,0 +1,48 @@
+const t = require('tap')
+const minimatch = require('../')
+
+const files = [
+  'a[]b',
+  '[b-a]x',
+  'a]b',
+  'a[]]b',
+  'a[[]b',
+]
+
+const patterns = [
+  '\\[b-a]*',
+  '[b-a\\]*',
+  'a[]*',
+  '[b-a]*',
+  'a[]]*',
+  'a[[]*',
+  'a\[[]]*'
+]
+
+// uncomment and pipe stderr into a bash script to test bash behavior
+// console.error('mkdir x; cd x; shopt -s nullglob')
+// for (const f of files) {
+//   console.error(`touch '${JSON.stringify(f).slice(1, -1)}'`)
+// }
+//
+// console.error('ls -laF')
+// for (const p of patterns) {
+//   console.error(`echo PATTERN='${p}'`)
+//   console.error(`for i in ${p}; do echo "$i"; done`)
+// }
+//
+// console.error('cd ..; rm -rf x')
+
+const matches = {}
+
+for (const pattern of patterns) {
+  for (const file of files) {
+    if (minimatch(file, pattern)) {
+      matches[pattern] = matches[pattern] || []
+      matches[pattern].push(file)
+    }
+  }
+}
+
+t.comment({files, patterns, matches})
+t.matchSnapshot({files, patterns, matches})
diff --git a/test/patterns.js b/test/patterns.js
@@ -262,6 +262,7 @@ module.exports = [
   'https://github.com/isaacs/minimatch/issues/59',
   ['[z-a]', []],
   ['a/[2015-03-10T00:23:08.647Z]/z', []],
+  ['a/[2015-03-10T00:23:08.647Z\\]/z', []],
   ['[a-0][a-\u0100]', []],
 
   'comments match nothing',
@@ -273,6 +274,7 @@ module.exports = [
   ['[]+*]', []],
   ['[z\\-a]', []],
   ['[\\-\\]]', []],
+  ['[a-b-c]', []],
 ]
 
 Object.defineProperty(module.exports, 'files', {