-
Notifications
You must be signed in to change notification settings - Fork 492
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
fix: properly escape dots in GTE0
regexes
#432
Conversation
Previously the dots were not properly escaped causing them to match any character. This caused ranges like `>=09090` to evaulate to `*` after they were incorrectly matched by the `GTE0` regex. This only happened in strict mode since in loose mode the leading 0 is allowed and parsed into `>=9090.0.0` before the `GTE0` check. After this fix, this range now will throw an error. This also affected prerelease versions in both strict and loose mode.
This also includes a small amount of refactoring and a few added debug statements that made it easier for me to dig into the details here. The behavior and tests were trivial to change, but it was my first time in the codebase so I was curious to see why it happened. |
This adds `@npmcli/template-oss` to manage GitHub Actions, linting, and other chores. It specifically pins to the latest version of the library in order to allow for the following manual changes: - Files outside of `lib/` to avoid breaking public API - Keeping engines (and testing) on `>=10` - Installs `npm@7` in CI to work with node 10 This surfaced a few bugs which I opted to fix in separate issues: - #432 - #434
This adds `@npmcli/template-oss` to manage GitHub Actions, linting, and other chores. It specifically pins to the latest version of the library in order to allow for the following manual changes: - Files outside of `lib/` to avoid breaking public API - Keeping engines (and testing) on `>=10` - Installs `npm@7` in CI to work with node 10 This surfaced a few bugs which I opted to fix in separate issues: - #432 - #434
This adds `@npmcli/template-oss` to manage GitHub Actions, linting, and other chores. It specifically pins to the latest version of the library in order to allow for the following manual changes: - Files outside of `lib/` to avoid breaking public API - Keeping engines (and testing) on `>=10` - Installs `npm@7` in CI to work with node 10 This surfaced a few bugs which I opted to fix in separate issues: - #432 - #434
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this looks good!
Previously the dots were not properly escaped causing them to match any character. This caused ranges like `>=09090` to evaulate to `*` after they were incorrectly matched by the `GTE0` regex. This only happened in strict mode since in loose mode the leading 0 is allowed and parsed into `>=9090.0.0` before the `GTE0` check. After this fix, this range now will throw an error. This also affected prerelease versions in both strict and loose mode.
Previously the dots were not properly escaped causing them to match any character. This caused ranges like `>=09090` to evaulate to `*` after they were incorrectly matched by the `GTE0` regex. This only happened in strict mode since in loose mode the leading 0 is allowed and parsed into `>=9090.0.0` before the `GTE0` check. After this fix, this range now will throw an error. This also affected prerelease versions in both strict and loose mode.
Previously the dots were not properly escaped causing them to match any
character. This caused ranges like
>=09090
to evaulate to*
afterthey were incorrectly matched by the
GTE0
regex. This only happened instrict mode since in loose mode the leading 0 is allowed and parsed into
>=9090.0.0
before theGTE0
check. After this fix, this range nowwill throw an error. This also affected prerelease versions in both
strict and loose mode.