-
-
Notifications
You must be signed in to change notification settings - Fork 267
chore: add regexp related eslint plugins #202
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
Changes from 3 commits
b104b29
dafc3be
4f34bad
059c471
d52acfa
00a212b
e937d58
c8fc071
7ce200c
31ac6d3
e1969e3
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -2,7 +2,7 @@ import type { ErrorMessage, PipeResult } from '../../types.ts'; | |
import { getOutput, getPipeIssues, isLuhnAlgo } from '../../utils/index.ts'; | ||
|
||
/** | ||
* Creates a validation functions that validates a IMEI. | ||
* Creates a validation function that validates a IMEI. | ||
* | ||
* Format: AA-BBBBBB-CCCCCC-D | ||
* | ||
|
@@ -12,8 +12,8 @@ import { getOutput, getPipeIssues, isLuhnAlgo } from '../../utils/index.ts'; | |
*/ | ||
export function imei<TInput extends string>(error?: ErrorMessage) { | ||
return (input: TInput): PipeResult<TInput> => | ||
!/^\d{2}[ |/|-]?\d{6}[ |/|-]?\d{6}[ |/|-]?\d$/.test(input) || | ||
!isLuhnAlgo(input) | ||
// eslint-disable-next-line security/detect-unsafe-regex -- false positive according to https://devina.io/redos-checker | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. If this rule is so buggy, we should consider removing it in general or create an issue on GitHub. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yeah.. this is one of the longest standing issues of the security plugin: eslint-community/eslint-plugin-security#28 Two alternatives exist:
For my personal config I've opted with sticking with eslint-plugin-security until they've sorted out what to use, but I wasn't aware of There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. As long as there is no fix, I prefer to disable or replace this linting rule globally. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'll play with the two plugins and update the PR accordingly There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. colinhacks/zod#2849 the zod PR is also interesting for this PR I'd say There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yes we should keep an eye on this There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I decided to go with redos-detector, since it seems to find more (non-false positives) of redos regexps. |
||
!/^\d{2}(?:[ /|-]?\d{6}){2}[ /|-]?\d$/u.test(input) || !isLuhnAlgo(input) | ||
? getPipeIssues('imei', error || 'Invalid IMEI', input) | ||
: getOutput(input); | ||
} |
Uh oh!
There was an error while loading. Please reload this page.