diff --git a/classes/range.js b/classes/range.js index a7d6556f..39c80e7f 100644 --- a/classes/range.js +++ b/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' diff --git a/test/classes/range.js b/test/classes/range.js index 33aef61c..a5ec7eed 100644 --- a/test/classes/range.js +++ b/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) diff --git a/test/fixtures/range-parse.js b/test/fixtures/range-parse.js index 1e08a029..40c6e84b 100644 --- a/test/fixtures/range-parse.js +++ b/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'],