Skip to content

Commit 758a8be

Browse files
authored
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 <!--- Don't worry if you miss something, chores are automatically tested. --> <!--- This checklist exists to help you remember doing the chores when you submit a PR. --> <!--- Put an `x` in all the boxes that apply. --> - [x] I have read the [Contributing Guide](https://yarnpkg.com/advanced/contributing). <!-- See https://yarnpkg.com/advanced/contributing#preparing-your-pr-to-be-released for more details. --> <!-- Check with `yarn version check` and fix with `yarn version check -i` --> - [x] I have set the packages that need to be released for my changes to be effective. <!-- The "Testing chores" workflow validates that your PR follows our guidelines. --> <!-- If it doesn't pass, click on it to see details as to what your PR might be missing. --> - [x] I will check that all automated PR checks pass before the PR gets reviewed.
1 parent a22486f commit 758a8be

File tree

3 files changed

+57
-1
lines changed

3 files changed

+57
-1
lines changed

.yarn/versions/c255c43f.yml

+34
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,34 @@
1+
releases:
2+
"@yarnpkg/cli": patch
3+
"@yarnpkg/core": patch
4+
5+
declined:
6+
- "@yarnpkg/plugin-compat"
7+
- "@yarnpkg/plugin-constraints"
8+
- "@yarnpkg/plugin-dlx"
9+
- "@yarnpkg/plugin-essentials"
10+
- "@yarnpkg/plugin-exec"
11+
- "@yarnpkg/plugin-file"
12+
- "@yarnpkg/plugin-git"
13+
- "@yarnpkg/plugin-github"
14+
- "@yarnpkg/plugin-http"
15+
- "@yarnpkg/plugin-init"
16+
- "@yarnpkg/plugin-interactive-tools"
17+
- "@yarnpkg/plugin-link"
18+
- "@yarnpkg/plugin-nm"
19+
- "@yarnpkg/plugin-npm"
20+
- "@yarnpkg/plugin-npm-cli"
21+
- "@yarnpkg/plugin-pack"
22+
- "@yarnpkg/plugin-patch"
23+
- "@yarnpkg/plugin-pnp"
24+
- "@yarnpkg/plugin-pnpm"
25+
- "@yarnpkg/plugin-stage"
26+
- "@yarnpkg/plugin-typescript"
27+
- "@yarnpkg/plugin-version"
28+
- "@yarnpkg/plugin-workspace-tools"
29+
- "@yarnpkg/builder"
30+
- "@yarnpkg/doctor"
31+
- "@yarnpkg/extensions"
32+
- "@yarnpkg/nm"
33+
- "@yarnpkg/pnpify"
34+
- "@yarnpkg/sdks"

packages/yarnpkg-core/sources/semverUtils.ts

+20-1
Original file line numberDiff line numberDiff line change
@@ -204,7 +204,7 @@ export function stringifyComparator(comparator: Comparator) {
204204
}
205205

206206
export function simplifyRanges(ranges: Array<string>) {
207-
const parsedRanges = ranges.map(range => validRange(range)!.set.map(comparators => comparators.map(comparator => getComparator(comparator))));
207+
const parsedRanges = ranges.map(removeSubsets).map(range => validRange(range)!.set.map(comparators => comparators.map(comparator => getComparator(comparator))));
208208

209209
let alternatives = parsedRanges.shift()!.map(comparators => mergeComparators(comparators))
210210
.filter((range): range is Comparator => range !== null);
@@ -233,3 +233,22 @@ export function simplifyRanges(ranges: Array<string>) {
233233

234234
return alternatives.map(comparator => stringifyComparator(comparator)).join(` || `);
235235
}
236+
237+
function removeSubsets(rangeString: string) {
238+
const parts = rangeString.split(`||`);
239+
if (parts.length > 1) {
240+
const newParts: Set<string> = new Set();
241+
for (const potentialSubset of parts) {
242+
if (!parts.some(part => part !== potentialSubset && semver.subset(potentialSubset, part))) {
243+
newParts.add(potentialSubset);
244+
}
245+
}
246+
247+
if (newParts.size < parts.length) {
248+
const newRange = [...newParts].join(` || `);
249+
return newRange;
250+
}
251+
}
252+
253+
return rangeString;
254+
}

packages/yarnpkg-core/tests/semverUtils.test.ts

+3
Original file line numberDiff line numberDiff line change
@@ -112,6 +112,9 @@ describe(`semverUtils`, () => {
112112
[[`<=1.5.3`, `1.5.3`], `1.5.3`],
113113
[[`1.5.3`, `1.5.3`], `1.5.3`],
114114
[[`1.5.0`, `1.5.3`], null],
115+
[[`~1.0.1 || ~1.0.2`], `~1.0.1`],
116+
[[`~1.0.1 || ~1.0.2`, `~1.0.1 || ~1.0.2`], `~1.0.1`],
117+
[(new Array(1000)).fill(`~1.0.1 || ~1.0.2`), `~1.0.1`],
115118
])(`should simplify %s into %s`, (ranges, expected) => {
116119
expect(semverUtils.simplifyRanges(ranges)).toEqual(expected);
117120
});

0 commit comments

Comments
 (0)