-
-
Notifications
You must be signed in to change notification settings - Fork 9.6k
[Validator] Improve regexp for Credit Cards and some more tests #6583
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
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Thanks for working on this. It would be very valuable if you can add information about these regexes as comments (with links to relevant sources -- like what you've done in the PR description). Thanks. |
The more information we can add in the class, the better it is. |
Can you keep the commit from #6603 to keep ownership? |
I actually have thought about that... let me try my git skills :) |
…non-numeric. Making scheme option accept strings in addition to arrays.
There you go! |
fabpot
added a commit
that referenced
this pull request
Jan 7, 2013
This PR was merged into the master branch. Commits ------- 5be0042 better regexp, more test cases, added comments about each credit card cc278af [Validator] Fix `CardSchemeValidator` double violation when value is non-numeric. Making scheme option accept strings in addition to arrays. Discussion ---------- [Validator] Improve regexp for Credit Cards and some more tests Bug fix: yes Feature addition: no Backwards compatibility break: no Symfony2 tests pass: yes Fixes the following tickets: Todo: Ensure these regexps are proper (credit card validation is always a pain) License of the code: MIT Documentation PR: Regarding Cases excluded from new Regular Expressions: - Credit card lengths should be respected, these regexp cover lengths in http://en.wikipedia.org/wiki/Bank_card_number - Visa length can only be 16 and 13 (older ones) - Diners Cards starting by 5 come from a joint venture between Diners Club and MasterCard, and should be processed like a MasterCard (according to http://www.regular-expressions.info/creditcard.html). - There seems to be JCB cards starting by 2131 and 1800, I could find them is some places, also found these numbers being tested in Credit Card generators, but some people don't cover them. I don't know their story either Any comments will be much appreciated! --------------------------------------------------------------------------- by fabpot at 2013-01-06T19:33:27Z Thanks for working on this. It would be very valuable if you can add information about these regexes as comments (with links to relevant sources -- like what you've done in the PR description). Thanks. --------------------------------------------------------------------------- by ricardclau at 2013-01-06T21:01:52Z Always glad to be able to contribute a little bit @fabpot you mean @link / @see PHPDoc inside CardSchemeValidator.php? Or further comments in this discussion before adding them? --------------------------------------------------------------------------- by fabpot at 2013-01-06T21:16:48Z The more information we can add in the class, the better it is. --------------------------------------------------------------------------- by ricardclau at 2013-01-07T20:56:05Z I've added comments and included code from #6603 as I've said there. If you need something else, please let me know, once this is merged, #6603 can also be closed --------------------------------------------------------------------------- by fabpot at 2013-01-07T21:41:40Z Can you keep the commit from #6603 to keep ownership? --------------------------------------------------------------------------- by ricardclau at 2013-01-07T21:44:16Z I actually have thought about that... let me try my git skills :) --------------------------------------------------------------------------- by ricardclau at 2013-01-07T21:59:16Z There you go!
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Bug fix: yes
Feature addition: no
Backwards compatibility break: no
Symfony2 tests pass: yes
Fixes the following tickets:
Todo: Ensure these regexps are proper (credit card validation is always a pain)
License of the code: MIT
Documentation PR:
Regarding Cases excluded from new Regular Expressions:
Any comments will be much appreciated!