credit.module -- Improve _valid_card_number()

Stupid weakly-typed languages... Yes, that should be fixed.

But Ubercart wouldn't pass a non-numeric to the gateway, because it would fail the card number checksum calculation. The checksum is the important part (and is "on" by default), the other stuff is, as you say, a sanity check.

There are several issues here, and it's not clear we're talking about the same thing.

When I say "checksum" I mean a checksum computed using all the digits of the card number, according to the Luhn algorithm. All the test card numbers on the page you reference above pass this test (I checked them just now).

This checksum is computed in the function _valid_card_number(). That function *also* does 2 other checks - the faulty is_numeric() check, and a check on the first digit of the card number. On examination, it seems that the first digit will fail for International Maestro if and only if you don't also accept American Express. That's a valid point, and should be fixed.

I'm in favor of *more* validation, not less, since catching bad card numbers up-front avoids service charges from payment gateways and helps prevent problems for those stores that are doing manual processing of credit card numbers.

I'm going through and doing some housecleaning so we can put out a 1.1 release and wanted to follow up here.

I don't see any bugs associated with this issue but take from it that the sanity checking should be more specific for the optional validation and inclusive of other cards. I'm not too fond of the whole checkboxes approach to specifying valid card types anyways, as it's limited as has been pointed out here.

The code block in question in the original post was

// Validate the CC number if that's turned on.
if (variable_get('uc_credit_validate_numbers', TRUE) || intval($cc_data['cc_number']) == 0) {
  if (!_valid_card_number($cc_data['cc_number'])) {
    drupal_set_message(t('You have entered an invalid credit card number.'), 'error');
    $return = FALSE;
  }
}

EDIT: Ok, Lyle pointed out I was reading the original post wrong. When you're saying it would pass, you're saying it would == 3 and not that it would == 0. So, this would still fail a checksum but only if you have number validation turned on. I don't see any reason for it not to be turned on, so that's a good workaround.

I committed a change to ctype_digit() as recommended for the 1.1.

I see what ya mean. Made your change. Is this issue closed?

fwiw, most card types check out w/ the Luhn algorithm (there's a Wikipedia article on it), and when I tested the Maestro test number from Protx it worked fine.

BrianV came by #drupal-ubercart today wondering why Ubercart rejects card numbers with spaces in them. For example 4111 1111 1111 1111. He stated...
"has anyone else had an issue like this? I mean, the client gets several phone calls per day from various customers about this. I would think other people would have noticed this"

Do gateways generally accept this type of number and is Ubercart incorrectly filtering these out as invalid?

afaik, any payment gateway I've worked with does not accept spaces. I imagine we could strip them out when we filter the input. I guess this was a non-issue before we updated the CC number field to be 19 characters long instead of the old 16.

Yeah, it would be nice if a string replace was added where the $_POST is processed into the $cc_data array in uc_payment_method_credit()

  'cc_number' => check_plain(str_replace(" ", "",  $_POST['cc_number'])),

Regarding the CC# being able to accept numbers with spaces, I just ran into this as well - due to a weird bug (http://www.ubercart.org/forum/bug_reports/8098/cc_number_format_validati...) where optional review checkout review was causing "invalid" card numbers with spaces in them to be submitted to Authorize.net in spite of being flagged as invalid by the checkout form, it appears that the CC#'s with spaces were properly accepted by Auth.net in my case...

I agree that it would be nice to collapse out the spaces in the CC# field, for usability sake.

Funny how fixing one issue opens up others... never had this problem before upping the size of that textfield to 19 characters for Maestro cards. :P

Anyways, tweaked it to remove spaces and am closing this issue out.

I am a beginner at PHP codes. Can I get assistance on how to validate a cc number? Here is what I have started and I know I am missing a function() but not sure how to write the code:

if (!isset($_GET['ccnumber']))
echo "<p>Enter your credit card number.</p>";
else {
$Payment = $_GET['ccnumber'];
$ValidPayment = str_replace("-", "", $Payment);
$ValidPayment = str_replace(" ", "", $ValidPayment);
}
if(is_numeric($ValidPayment))
echo "<p>You did not enter a valid credit card number!</p>";
else {
echo "<p>Your credit card number is {$ValidPayment}.</p>";
}

 if
(!empty($_GET['ccnumber'])) echo $_GET['ccnumber'] 

" />