Skip to content

[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
merged 2 commits into from
Jan 7, 2013
Merged

[Validator] Improve regexp for Credit Cards and some more tests #6583

merged 2 commits into from
Jan 7, 2013

Conversation

ricardclau
Copy link
Contributor

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!

@fabpot
Copy link
Member

fabpot commented Jan 6, 2013

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.

@ricardclau
Copy link
Contributor Author

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?

@fabpot
Copy link
Member

fabpot commented Jan 6, 2013

The more information we can add in the class, the better it is.

@ricardclau
Copy link
Contributor Author

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

@fabpot
Copy link
Member

fabpot commented Jan 7, 2013

Can you keep the commit from #6603 to keep ownership?

@ricardclau
Copy link
Contributor Author

I actually have thought about that... let me try my git skills :)

@ricardclau
Copy link
Contributor Author

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!
@fabpot fabpot merged commit 5be0042 into symfony:master Jan 7, 2013
@ricardclau ricardclau deleted the improve-creditcard-regexp branch January 7, 2013 22:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants