Bug #19798 for Business-CreditCard: Length requirement for verify(), none for generate_last

Thu Jun 08 17:56:49 2006 Guest - Ticket created

Subject:

Length requirement for verify(), none for generate_last_digit()

Here's the problem as I see it, in a nutshell: generate_last_digit('11523'); works correctly (it returns '8'). verify('115238'); returns 0 (failure) because it is less than 13 digits in length. I see this as an inconsistency that should be fixed. The choices I see are to either fail the generate_last_digit() calls for less than 13 digits, or allow verify to verify numbers of any length. The reason this matters is because Authen::PIN uses this module to create PIN numbers of any length. I'm asking Authen::PIN to add a verify capability of its own, and was planning on using the verify from this module. However, I can't if it is restricted to less than 13 digits. So if you don't care which solution, I'd vote for allowing verify to verify numbers of any length. What do you think? Thanks -Craig Votava Lucent Technologies

Tue Oct 10 16:01:42 2006 ivan-pause [...] 420.am - Status changed from 'new' to 'open'

Tue Oct 10 16:06:32 2006 ivan-pause [...] 420.am - Correspondence added

On Thu Jun 08 17:56:49 2006, guest wrote: Show quoted text

> Here's the problem as I see it, in a nutshell: > > generate_last_digit('11523'); > > works correctly (it returns '8'). > > verify('115238'); > > returns 0 (failure) because it is less than 13 digits in length. > > I see this as an inconsistency that should be fixed. The > choices I see are to either fail the generate_last_digit() > calls for less than 13 digits, or allow verify to verify > numbers of any length. > > The reason this matters is because Authen::PIN uses > this module to create PIN numbers of any length. I'm > asking Authen::PIN to add a verify capability of its own, > and was planning on using the verify from this module. > However, I can't if it is restricted to less than 13 digits. > > So if you don't care which solution, I'd vote for allowing > verify to verify numbers of any length. > > What do you think?

Well, this is Business::CreditCard, not Algorithm-LUHN, so I think it makes sense for verify() to check for a length consistant with credit cards. I agree it may be a little odd that generate_last_digit doesn't check the length, but I don't personally find it to be a glaring inconsistancy. That's what verify() is for, after all. Authn::PIN probably shouldn't use this module for a general-purpose LUHN implementation.

Tue Oct 24 11:40:37 2006 craig [...] lucent.com - Correspondence added

From:

craig [...] lucent.com

On Tue Oct 10 16:06:32 2006, IVAN wrote: Show quoted text

> Well, this is Business::CreditCard, not Algorithm-LUHN, so I think it > makes sense for verify() to check for a length consistant with credit > cards. > > I agree it may be a little odd that generate_last_digit doesn't check > the length, but I don't personally find it to be a glaring > inconsistancy. That's what verify() is for, after all. > > Authn::PIN probably shouldn't use this module for a general-purpose > LUHN implementation.

Yes, it is a good point to make that I should be using Algorithm::LUHN and not Business::CreditCard for what I'm trying to do. This is the direction I will probably take. However, as a user of Business::CreditCard, I would expect it to work within the bounds of any valid credit card number. The ISO 7812 standard stipulates a 19 digit maximum (not counting the check digit), and it is broken down as follows: 1-digit Major Industry Identifier (fixed) 6-digit Issuer Identifier Number (fixed) Account number (variable 1-12) 1-digit Check Digit This means that it is possible to have a valid 9-digit credit card number (8-digits without the check), but your software has the minimum fixed at 13 digits. So I guess I'm saying that I would expect Business::CreditCard to work with card numbers between 8-19 digits (9-20 with the check), and that all functions would perform this check and fail when the input doesn't meet these requirements. Does that make sense?

Wed Oct 25 12:57:58 2006 ivan-pause [...] 420.am - Correspondence added

On Tue Oct 24 11:40:37 2006, craig@lucent.com wrote: Show quoted text

> On Tue Oct 10 16:06:32 2006, IVAN wrote:

> > Well, this is Business::CreditCard, not Algorithm-LUHN, so I think

it Show quoted text

> > makes sense for verify() to check for a length consistant with

credit Show quoted text

> > cards. > > > > I agree it may be a little odd that generate_last_digit doesn't

check Show quoted text

> > the length, but I don't personally find it to be a glaring > > inconsistancy. That's what verify() is for, after all. > > > > Authn::PIN probably shouldn't use this module for a

general-purpose Show quoted text

> > LUHN implementation.

> > Yes, it is a good point to make that I should be using

Algorithm::LUHN Show quoted text

> and not Business::CreditCard for what I'm trying to do. This is the > direction I will probably take. > > However, as a user of Business::CreditCard, I would expect it to

work Show quoted text

> within the bounds of any valid credit card number. The ISO 7812

standard Show quoted text

> stipulates a 19 digit maximum (not counting the check digit), and it

is Show quoted text

> broken down as follows: > > 1-digit Major Industry Identifier (fixed) > 6-digit Issuer Identifier Number (fixed) > Account number (variable 1-12) > 1-digit Check Digit > > This means that it is possible to have a valid 9-digit credit card > number (8-digits without the check), but your software has the > minimum fixed at 13 digits. > > So I guess I'm saying that I would expect Business::CreditCard > to work with card numbers between 8-19 digits (9-20 with the check), > and that all functions would perform this check and fail when the > input doesn't meet these requirements. Does that make sense?

For this module, it seems more useful to conform to real-world practice than a pedantic reading of the ISO standard, don't you think? Are there any actual less than 13 digit credit cards? Feel free to re-open this bug if verify() doesn't work with any real-world credit card number or you have a patch. :)

Wed Oct 25 12:58:23 2006 ivan-pause [...] 420.am - Status changed from 'open' to 'resolved'

Wed Oct 25 12:58:29 2006 ivan-pause [...] 420.am - Status changed from 'resolved' to 'rejected'

Thu Oct 26 12:17:00 2006 craig [...] lucent.com - Correspondence added

From:

craig [...] lucent.com

On Wed Oct 25 12:57:58 2006, IVAN wrote: Show quoted text

> For this module, it seems more useful to conform to real-world > practice than a pedantic reading of the ISO standard, don't you think? > Are there any actual less than 13 digit credit cards? Feel free to > re-open this bug if verify() doesn't work with any real-world credit > card number or you have a patch. :)

Ivan- No problem, here's the patch: 107c107 < return "Not a credit card" unless length($number) >= 1 && 0+$number; --- Show quoted text

> return "Not a credit card" unless length($number) >= 13 && 0+$number;

172c172 < return 0 unless length($number) >= 1 && 0+$number; --- Show quoted text

> return 0 unless length($number) >= 13 && 0+$number;

Thanks for the help.

Thu Oct 26 12:17:02 2006 The RT System itself - Status changed from 'rejected' to 'open'

Thu Oct 26 12:39:42 2006 ivan-pause [...] 420.am - Status changed from 'open' to 'rejected'

Thu Sep 06 15:52:47 2007 ivan-pause [...] 420.am - Correspondence added

From:

ivan-pause [...] 420.am

This module is for verifying _real world_ *credit cards*. It is *NOT* a pedantic implementation of the ISO 7812 standard, a general-purpose LUHN implementation, or intended for use with "creditcard-like account numbers".

Thu Sep 06 15:52:59 2007 The RT System itself - Status changed from 'rejected' to 'open'

Thu Sep 06 15:54:45 2007 ivan-pause [...] 420.am - Status changed from 'open' to 'rejected'

Thu Sep 06 15:55:10 2007 ivan-pause [...] 420.am - Severity Normal changed to Unimportant

Thu Sep 06 16:22:15 2007 craig [...] alcatel-lucent.com - Correspondence added

Subject:	Re: [rt.cpan.org #19798] Length requirement for verify(), none for generate_last_digit()
Date:	Thu, 6 Sep 2007 15:20:57 -0500
To:	via RT <bug-Business-CreditCard [...] rt.cpan.org>
From:	Craig Votava <craig [...] alcatel-lucent.com>

Ivan- My goodness. Why so much animosity over such a trivial request? Is it so unreasonable to ask for these two functions to handle length in the same way? I can't see that it would be, which is why I don't understand all the nastiness that my request has generated. Anyways, it wasn't my intention to provoke you, but instead to make the code more useful to a wider variety of folks. Clearly, this is your module and you can do what you like with it. As interest continues to grow in a module that actually implements the ISO 7812 standard, I may take the CPAN FAQ advice: Show quoted text

> If you need changes in order for another module or application to > work, consider making the needed changes and bundling the new > version with your own distribution and noting the change well in > the documentation. Do not upload the new version under the same > namespace to CPAN until the matter has been resolved with the > author or CPAN.

-Craig On Sep 6, 2007, at 2:53 PM, Ivan Kohler via RT wrote: Show quoted text

> > <URL: http://rt.cpan.org/Ticket/Display.html?id=19798 > > > This module is for verifying _real world_ *credit cards*. It is *NOT* > a pedantic implementation of the ISO 7812 standard, a general-purpose > LUHN implementation, or intended for use with "creditcard-like account > numbers". >

Thu Sep 06 16:22:27 2007 The RT System itself - Status changed from 'rejected' to 'open'

Thu Sep 06 17:38:23 2007 ivan-pause [...] 420.am - Correspondence added

On Thu Sep 06 16:22:15 2007, cmv wrote: Show quoted text

> Ivan- > > My goodness. Why so much animosity over such a trivial > request?

I'm not particularly provoked or offended. I was attempting to clarify the purpose of the module, and why your patch was rejected, since you link to this bug report in your rather inappropriate, sour-grapes CPAN review. Show quoted text

> Is it so unreasonable to ask for these two > functions to handle length in the same way?

These functions are for completely different purposes; one is a verification function and the other is not. generate_last_digit is not a verification function. It does not check length. It will not be given verification functionality and I do not agree with your labyrinthine logic that this is somehow "inconsistent" with the verification function. It is likewise unreasonable to ask that the verification function be relaxed to include cases that do not involve REAL WORLD CREDIT CARDS, the verification of which is the entire purpose of the module. The module does not purport to implement "the ISO 7812 standard", be a general purpose LUHN implementation or verify "credit-card like account numbers". I hope the documentation will make that clear enough for you going forward. Show quoted text

> I can't see > that it would be, which is why I don't understand all > the nastiness that my request has generated.

I've attempted to explain it to you many times, but it does not seem I am able to make it clear to you. I would suggest to perhaps have an impartial friend re-read the correspondence with you and see if they can't help you understand. Show quoted text

> Anyways, it wasn't my intention to provoke you, but > instead to make the code more useful to a wider variety > of folks. Clearly, this is your module and you can > do what you like with it. As interest continues to > grow in a module that actually implements the ISO 7812 > standard, I may take the CPAN FAQ advice:

I'm not personally aware of said growing interest, but by all means, please certainly feel free to fork the module in accordance with the license and upload and maintain your own Business::ISO7812 module.

Thu Sep 06 17:38:27 2007 ivan-pause [...] 420.am - Status changed from 'open' to 'rejected'

Thu Sep 20 17:12:57 2007 craig [...] alcatel-lucent.com - Correspondence added

Subject:	Re: [rt.cpan.org #19798] Length requirement for verify(), none for generate_last_digit()
Date:	Thu, 20 Sep 2007 16:12:18 -0500
To:	bug-Business-CreditCard [...] rt.cpan.org
From:	Craig Votava <craig [...] alcatel-lucent.com>

Ivan- I want to publicly apologize for being a jerk -- or if not actually being one, being within the event horizon. On Sep 6, 2007, at 4:38 PM, Ivan Kohler via RT wrote: Show quoted text

> I would suggest to perhaps have an > impartial friend re-read the correspondence with you and see if they > can't help you understand.

I took your suggestion, and asked for help. I got a surprising amount of feedback - your position was unanimously supported. Read more at: http://www.perlmonks.org/?node_id=640142 Show quoted text

> I'm not particularly provoked or offended. I was attempting to > clarify the purpose of the module, and why your patch was rejected, > since you link to this bug report in your rather inappropriate, > sour-grapes CPAN review.

I am glad you are neither provoked or offended, and I also apologize for the review. Please chalk this up to the fact that I have never authored a perl module (yet), and am just now becoming aware of the larger issues you good people have to deal with. It was wrong of me to criticize without having walked some in your shoes. Sorry. -Craig

Thu Sep 20 17:12:59 2007 The RT System itself - Status changed from 'rejected' to 'open'

Thu Sep 20 23:44:36 2007 ivan-pause [...] 420.am - Correspondence added

Don't sweat it one bit. :) It takes some serious cojones to admit to a mistake. I hope next time I do something silly I can be half as gracious as you are about it. On Thu Sep 20 17:12:57 2007, cmv wrote: Show quoted text

> Ivan- > > I want to publicly apologize for being a jerk -- or if > not actually being one, being within the event horizon. > > On Sep 6, 2007, at 4:38 PM, Ivan Kohler via RT wrote:

> > I would suggest to perhaps have an > > impartial friend re-read the correspondence with you and see if

they Show quoted text

> > can't help you understand.

> > I took your suggestion, and asked for help. I got a surprising > amount of feedback - your position was unanimously supported. > Read more at: > http://www.perlmonks.org/?node_id=640142 >

> > I'm not particularly provoked or offended. I was attempting to > > clarify the purpose of the module, and why your patch was

rejected, Show quoted text

> > since you link to this bug report in your rather inappropriate, > > sour-grapes CPAN review.

> > I am glad you are neither provoked or offended, and I also > apologize for the review. > > Please chalk this up to the fact that I have never authored > a perl module (yet), and am just now becoming aware of the > larger issues you good people have to deal with. > > It was wrong of me to criticize without having walked some > in your shoes. > > Sorry. > > -Craig >

Thu Sep 20 23:44:38 2007 ivan-pause [...] 420.am - Status changed from 'open' to 'rejected'

Bug #19798 for Business-CreditCard: Length requirement for verify(), none for generate_last_digit()