-
-
Notifications
You must be signed in to change notification settings - Fork 2.3k
Added RegEx for Insta Payment Card and Maestro Cards #1701
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
base: master
Are you sure you want to change the base?
Conversation
Codecov Report
@@ Coverage Diff @@
## master #1701 +/- ##
=========================================
Coverage 100.00% 100.00%
=========================================
Files 102 102
Lines 2029 2029
Branches 457 457
=========================================
Hits 2029 2029
Continue to review full report at Codecov.
|
src/lib/isCreditCard.js
Outdated
const creditCard = /^(?:4[0-9]{12}(?:[0-9]{3,6})?|5[1-5][0-9]{14}|(222[1-9]|22[3-9][0-9]|2[3-6][0-9]{2}|27[01][0-9]|2720)[0-9]{12}|6(?:011|5[0-9][0-9])[0-9]{12,15}|3[47][0-9]{13}|3(?:0[0-5]|[68][0-9])[0-9]{11}|(?:2131|1800|35\d{3})\d{11}|6[27][0-9]{14})$/; | ||
const creditCard = /^(?:4[0-9]{12}(?:[0-9]{3,6})?|5[1-5][0-9]{14}|(222[1-9]|22[3-9][0-9]|2[3-6][0-9]{2}|27[01][0-9]|2720)[0-9]{12}|6(?:011|5[0-9][0-9])[0-9]{12,15}|3[47][0-9]{13}|3(?:0[0-5]|[68][0-9])[0-9]{11}|(?:2131|1800|35\d{3})\d{11}|6[27][0-9]{14}|((5018|5020|5038|6304|6759|6761|6763)[0-9]{8,15})|(63[7-9][0-9]{13}))$/; |
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.
(not related to this PR)
Haven't run any test but this regex smells like vulnerable to redos attacks.
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.
@fedeci Could you please test this PR?
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.
You should add tests in the proper file.
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.
Test is already passed, you can review this.
Codecov Report
Merging #1701 (90599bc) into master (8c4b3b3) will not change coverage.
The diff coverage is100.00%
.@@ Coverage Diff @@ ## master #1701 +/- ## ========================================= Coverage 100.00% 100.00% ========================================= Files 101 101 Lines 2005 2005 Branches 452 452 ========================================= Hits 2005 2005Impacted Files Coverage Δ
src/lib/isCreditCard.js100.00% <100.00%> (ø)
Continue to review full report at Codecov.Legend - Click here to learn more
Δ = absolute <relative> (impact)
,ø = not affected
,? = missing data
Powered by Codecov. Last update 8c4b3b3...90599bc. Read the comment docs.
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.
We should add both successful and failing test cases for the new features introduced in the PR even if the tests are passing
Is someone testing this PR? |
I have update the PR with tests in commit 4a0239f |
Is some one reviewing this PR? |
@profnandaa , could you take a look on this PR |
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.
Thank you for your PR @metajunaid and sorry for the late review.
Can you please fix merge conflicts?
Hi @tux-tn, I have resolved the conflicts. |
@metajunaid can you fix the merge conflicts |
This PR aims to solve issue #1559.
Added regex to validate Maestro and Insta Payment Card.
RegEx Reference taken from: https://gist.github.com/michaelkeevildown/9096cd3aac9029c4e6e05588448a8841
Insta Payment Card: ^63[7-9][0-9]{13}$
Maestro Card: ^(5018|5020|5038|6304|6759|6761|6763)[0-9]{8,15}$