fix: loose mode no longer drops input that is not range syntax

classes/range.js

@@ -145,7 +145,18 @@ class Range {
       // in loose mode, throw out any that are not valid comparators
       rangeList = rangeList.filter(comp => {
         debug('loose invalid filter', comp, this.options)
-        return !!comp.match(re[t.COMPARATORLOOSE])
+        if (comp.match(re[t.COMPARATORLOOSE])) {
+          return true
+        }
+        // Loose mode tolerates the leftovers of its own normalization -- a
+        // stray `==`, a bare `foo` -- and drops them. What it must not drop is
+        // input that is not range syntax at all, because the range then reads
+        // as something weaker than what was written: `* (4.1.x 4.1.2)` parsed
+        // as `*`, matching every version, and `>=1.0.0 (foo)` as `>=1.0.0`.
+        if (NOTRANGESYNTAX.test(comp)) {
+          throw new TypeError(`Invalid SemVer Range: ${this.raw}`)
+        }
+        return false
       })
     }
     debug('range list', rangeList)
@@ -235,6 +246,14 @@ const {
 const { FLAG_INCLUDE_PRERELEASE, FLAG_LOOSE } = require('../internal/constants')
 
 // unbounded global build-metadata stripper used by parseRange
+// Characters that appear in no comparator and no version, in loose mode or any
+// other. A token carrying one is not sloppy range syntax, it is not range
+// syntax, and dropping it would change what the range means.
+//
+// The comma is deliberately absent: `1.2.3, <2.0.0` is tolerated today and
+// pinned by a test, so it keeps being dropped. See the pull request for why
+// that one is left alone.
+const NOTRANGESYNTAX = /[^0-9a-zA-Z\s<>=~^|*.+,-]/
 const BUILDSTRIPRE = new RegExp(src[t.BUILD], 'g')
 
 const isNullSet = c => c.value === '<0.0.0-0'

test/classes/range.js

@@ -8,6 +8,7 @@ const rangeIntersection = require('../fixtures/range-intersection.js')
 const rangeInclude = require('../fixtures/range-include.js')
 const rangeExclude = require('../fixtures/range-exclude.js')
 const rangeParse = require('../fixtures/range-parse.js')
+const validRange = require('../../ranges/valid.js')
 
 test('range tests', t => {
   t.plan(rangeInclude.length)
@@ -50,8 +51,8 @@ test('range parsing covers every option combination', t => {
   const combinations = ['false:false', 'true:false', 'false:true', 'true:true']
   t.strictSame([...inputsByOptions.keys()], combinations, 'all four modes are present')
   const defaultInputs = inputsByOptions.get('false:false')
-  t.equal(defaultInputs.length, 348, 'retains every source fixture row')
-  t.equal(new Set(defaultInputs).size, 335, 'retains every distinct source input')
+  t.equal(defaultInputs.length, 354, 'retains every source fixture row')
+  t.equal(new Set(defaultInputs).size, 341, 'retains every distinct source input')
   for (const key of combinations.slice(1)) {
     t.strictSame(inputsByOptions.get(key), defaultInputs, `${key} covers the same inputs`)
   }
@@ -82,6 +83,47 @@ test('throw for empty comparator set, even in loose mode', t => {
   t.end()
 })
 
+// Loose mode dropped any token it could not parse and kept the rest, so a
+// range could silently come out weaker than what was written. `(1.2.3)` threw
+// only because nothing survived the drop; one valid comparator anywhere in the
+// range masked the rest. #387
+test('loose mode does not drop input that is not range syntax', t => {
+  for (const range of [
+    '* (4.1.x  4.1.2)', // parsed as '*', which matches every version
+    '>=1.0.0 (foo)', // parsed as '>=1.0.0'
+    '* (bogus)',
+    '>=1.0.0 [foo]',
+    '1.0.0 {x}',
+  ]) {
+    t.throws(() => new Range(range, { loose: true }), TypeError,
+      `loose mode rejects ${range}`)
+    t.equal(validRange(range, { loose: true }), null,
+      `validRange does not call ${range} valid`)
+  }
+  t.end()
+})
+
+// The counterpart: loose mode still tolerates the leftovers of its own
+// normalization. These are what the drop exists for, and they must keep
+// parsing -- a fix that rejected everything unparseable would break them.
+test('loose mode still tolerates its own leftovers', t => {
+  for (const [range, expected] of [
+    ['== 1', '>=1.0.0 <2.0.0-0'],
+    ['== 1.0.0 || foo', '1.0.0'],
+    ['1.2.3 || 1.2.3= x', '1.2.3'],
+    ['1.2.3-dev= 1 || 2.0.0', '2.0.0'],
+    // A comma is tolerated and dropped today, pinned by a test in
+    // test/ranges/valid.js, so it is deliberately left alone here.
+    ['1.2.3, <2.0.0', '<2.0.0'],
+    ['v1.2.3', '1.2.3'],
+    ['=1.2.3', '1.2.3'],
+  ]) {
+    t.equal(new Range(range, { loose: true }).format(), expected,
+      `loose mode still parses ${range}`)
+  }
+  t.end()
+})
+
 test('convert comparator to range', t => {
   const c = new Comparator('>=1.2.3')
   const r = new Range(c)

test/fixtures/range-parse.js

@@ -53,6 +53,15 @@ const ranges = [
   ['1.2.3+dev = 1.2.3+build', '1.2.3'],
   ['>= 1 +build < 2', '>=1.0.0 <2.0.0-0'],
   ['1.2.3 +build = 1.2.3', null],
+  // Parentheses and brackets are not range syntax in any mode. Loose mode used
+  // to drop them and keep whatever was left, so these read as something weaker
+  // than what was written -- `* (4.1.x 4.1.2)` as `*`, matching everything.
+  ['(1.2.3)', null],
+  ['* (4.1.x  4.1.2)', null],
+  ['>=1.0.0 (foo)', null],
+  ['* (bogus)', null],
+  ['>=1.0.0 [foo]', null],
+  ['1.0.0 {x}', null],
   ['= 01.2.3', null],
   ['1.2.3 = 1.2.3 || >= 2 < 3', '1.2.3||>=2.0.0 <3.0.0-0'],
   ['1.2.3+build - 2.0.0+build', '>=1.2.3 <=2.0.0'],