From 435bf372ee9a22b61ea0c412b69b26fe06d66ede Mon Sep 17 00:00:00 2001 From: Kumar Swamy R Y Date: Thu, 22 Oct 2026 23:51:03 +0530 Subject: [PATCH] fix: loose mode no longer drops input that is not range syntax In loose mode `parseRange` filtered out every token that did not match `COMPARATORLOOSE` and kept whatever was left. A range containing text that is not range syntax therefore parsed successfully as something weaker than what was written: new Range('* (4.1.x 4.1.2)', { loose: true }).format() // '' validRange('* (4.1.x 4.1.2)', { loose: true }) // '*' new Range('>=1.0.0 (foo)', { loose: true }).format() // '>=1.0.0' validRange('* (bogus)', { loose: true }) // '*' The first is the dangerous one: something written to pin 4.1.2 ends up as `*`, which matches every version. `validRange` reporting `'*'` means a caller that validates input before storing it is told the string is fine and then gets a different range than the one it handed in. `(1.2.3)` already threw, but only because nothing survived the filter and the empty set was rejected. One valid comparator anywhere in the range masked the rest, which is why this looked inconsistent. Rejecting everything unparseable is the obvious fix and it is wrong. I measured it: loose mode relies on the drop for the leftovers of its own normalization, and three test files fail, among them `== 1`, `== 1.0.0 || foo`, `1.2.3 || 1.2.3= x` and `1.2.3-dev= 1 || 2.0.0`, all of which are pinned as parsing today. So the rule is narrower: a token is dropped as before unless it carries a character that appears in no comparator and no version, in which case the range is invalid. A stray `==` or a bare `foo` is sloppy range syntax; `(foo)` is not range syntax. The error is the `Invalid SemVer Range` the constructor already raises, so nothing new appears in the API. The comma is deliberately excluded from that character class. `1.2.3, <2.0.0` parses as `<2.0.0` today -- the same shape of problem, the lower bound silently vanishing -- but it is pinned by a test in test/ranges/valid.js, so changing it is a separate decision for the maintainers rather than something to slip in here. It is called out in the pull request and in a comment next to the character class. Strict mode is untouched: it already threw `Invalid comparator` for all of these. Tests: six rows in the range-parse fixture, which runs each input in all four option combinations, so the parenthesised forms are now asserted invalid in loose and strict alike. Two focused tests either side of the line -- one that the bracketed forms are rejected and `validRange` returns null, one that loose mode still parses its own leftovers, including the comma case, so a future widening of this check gets caught. The fixture's hardcoded row counts move from 348/335 to 354/341. --- classes/range.js | 21 +++++++++++++++- test/classes/range.js | 46 ++++++++++++++++++++++++++++++++++-- test/fixtures/range-parse.js | 9 +++++++ 3 files changed, 73 insertions(+), 3 deletions(-) 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'],