From 758a8be3cf6ef0327de2de101c4c7c19ed25a982 Mon Sep 17 00:00:00 2001 From: Scott Mikula Date: Fri, 13 Sep 2024 06:51:05 -0700 Subject: [PATCH] Avoid crash in simplifyRanges by removing subsets up front (#6459) ## What's the problem this PR addresses? Resolves #6373. The problem is that `simplifyRanges` doesn't correctly reduce redundant OR ranges. For example, `~1.0.1 || ~1.0.2` should be simplified to `~1.0.1`. As the algorithm runs, it will effectively calculate every _combination_ of terms in such ranges. For example, given two ranges like `~1.0.1 || ~1.0.2`, the `nextAlternatives` array will end up with 2*2 = 4 entries; if you have 100 such ranges you'll end up with 2^100 entries. Growing exponentially like this it's not hard to crash the process. Arguably packages should not specify peer deps with this sort of redundant range, but sometimes they do (I'm working on cleaning up my project now that I know what the problem is!) Regardless, yarn shouldn't crash when it happens. ## How did you fix it? At the beginning of `simplifyRanges`, I reduce any range of this sort by splitting it apart and using `sember.subset` to check if one part of the range is a subset of another, in which case it can be excluded from the simplified range. I short circuit if the range only has one term, to avoid any excess parsing. I think this is the right fix, but I'm happy to take feedback or hand it off if someone knows better. (Maybe @arcanis as author of this code?) ## Checklist - [x] I have read the [Contributing Guide](https://yarnpkg.com/advanced/contributing). - [x] I have set the packages that need to be released for my changes to be effective. - [x] I will check that all automated PR checks pass before the PR gets reviewed. --- .yarn/versions/c255c43f.yml | 34 +++++++++++++++++++ packages/yarnpkg-core/sources/semverUtils.ts | 21 +++++++++++- .../yarnpkg-core/tests/semverUtils.test.ts | 3 ++ 3 files changed, 57 insertions(+), 1 deletion(-) create mode 100644 .yarn/versions/c255c43f.yml diff --git a/.yarn/versions/c255c43f.yml b/.yarn/versions/c255c43f.yml new file mode 100644 index 000000000000..2d6a48f178d8 --- /dev/null +++ b/.yarn/versions/c255c43f.yml @@ -0,0 +1,34 @@ +releases: + "@yarnpkg/cli": patch + "@yarnpkg/core": patch + +declined: + - "@yarnpkg/plugin-compat" + - "@yarnpkg/plugin-constraints" + - "@yarnpkg/plugin-dlx" + - "@yarnpkg/plugin-essentials" + - "@yarnpkg/plugin-exec" + - "@yarnpkg/plugin-file" + - "@yarnpkg/plugin-git" + - "@yarnpkg/plugin-github" + - "@yarnpkg/plugin-http" + - "@yarnpkg/plugin-init" + - "@yarnpkg/plugin-interactive-tools" + - "@yarnpkg/plugin-link" + - "@yarnpkg/plugin-nm" + - "@yarnpkg/plugin-npm" + - "@yarnpkg/plugin-npm-cli" + - "@yarnpkg/plugin-pack" + - "@yarnpkg/plugin-patch" + - "@yarnpkg/plugin-pnp" + - "@yarnpkg/plugin-pnpm" + - "@yarnpkg/plugin-stage" + - "@yarnpkg/plugin-typescript" + - "@yarnpkg/plugin-version" + - "@yarnpkg/plugin-workspace-tools" + - "@yarnpkg/builder" + - "@yarnpkg/doctor" + - "@yarnpkg/extensions" + - "@yarnpkg/nm" + - "@yarnpkg/pnpify" + - "@yarnpkg/sdks" diff --git a/packages/yarnpkg-core/sources/semverUtils.ts b/packages/yarnpkg-core/sources/semverUtils.ts index c0b09aac5450..fb27bd8bc081 100644 --- a/packages/yarnpkg-core/sources/semverUtils.ts +++ b/packages/yarnpkg-core/sources/semverUtils.ts @@ -204,7 +204,7 @@ export function stringifyComparator(comparator: Comparator) { } export function simplifyRanges(ranges: Array) { - const parsedRanges = ranges.map(range => validRange(range)!.set.map(comparators => comparators.map(comparator => getComparator(comparator)))); + const parsedRanges = ranges.map(removeSubsets).map(range => validRange(range)!.set.map(comparators => comparators.map(comparator => getComparator(comparator)))); let alternatives = parsedRanges.shift()!.map(comparators => mergeComparators(comparators)) .filter((range): range is Comparator => range !== null); @@ -233,3 +233,22 @@ export function simplifyRanges(ranges: Array) { return alternatives.map(comparator => stringifyComparator(comparator)).join(` || `); } + +function removeSubsets(rangeString: string) { + const parts = rangeString.split(`||`); + if (parts.length > 1) { + const newParts: Set = new Set(); + for (const potentialSubset of parts) { + if (!parts.some(part => part !== potentialSubset && semver.subset(potentialSubset, part))) { + newParts.add(potentialSubset); + } + } + + if (newParts.size < parts.length) { + const newRange = [...newParts].join(` || `); + return newRange; + } + } + + return rangeString; +} diff --git a/packages/yarnpkg-core/tests/semverUtils.test.ts b/packages/yarnpkg-core/tests/semverUtils.test.ts index 5f0a9e86e515..75fa7b1d29c1 100644 --- a/packages/yarnpkg-core/tests/semverUtils.test.ts +++ b/packages/yarnpkg-core/tests/semverUtils.test.ts @@ -112,6 +112,9 @@ describe(`semverUtils`, () => { [[`<=1.5.3`, `1.5.3`], `1.5.3`], [[`1.5.3`, `1.5.3`], `1.5.3`], [[`1.5.0`, `1.5.3`], null], + [[`~1.0.1 || ~1.0.2`], `~1.0.1`], + [[`~1.0.1 || ~1.0.2`, `~1.0.1 || ~1.0.2`], `~1.0.1`], + [(new Array(1000)).fill(`~1.0.1 || ~1.0.2`), `~1.0.1`], ])(`should simplify %s into %s`, (ranges, expected) => { expect(semverUtils.simplifyRanges(ranges)).toEqual(expected); });