Validator ignores extra characters

[Arkkimaagi]

For me it seems that the validator is too flexible. It says that this is valid: FI@ +425*000*151=0 == #000$ 0° 23€€€!!!!!

And altho technically it does contain valid numbers for an fictional IBAN number, that gibberish in my mind should not count as a valid IBAN.

[Arkkimaagi]

At least provide a strict mode and/or warn people that they have to sanitize themself and how to do it.

[LaurentVB]

I'll have a look, but you're right, this should obviously be marked as invalid. I think whitespace should be the only type of characters ignored.

[ostrolucky]

It shouldn't even convert input to uppercase. We had a problem when Sk2683300000002700890226 passed the validation of this library and then bank transfer failed because of "Sk" instead of "SK"

[mpkorstanje]

Right now the correct way to use isValid seems to be:

if(IBAN.isValid(input){
  model = IBAN.electronicFormat(input);
  view = IBAN.printFormat(input);
}

This makes sense when dealing with user input as they are entering it but less so when merely validating if the input adheres to a format.

So I think the validation should be

    exports.isValid = function(input){
        if (!isString(input)){
            return false;
        }
        var iban = electronicFormat(input);
        if(iban !==input ){
            return false;
        }
        var countryStructure = countries[iban.slice(0,2)];
        return !!countryStructure && countryStructure.isValid(iban);
    };

And then we might add a parse function to facilitate the other use case

    exports.parse = function(input){
         var iban = electronicFormat(input);
         if(isValid(iban)){
              return iban;
         }
        return undefined;
    };

So we can do

var iban = IBAN.parse(input);
if(iban){
  model = iban;
  view = IBAN.printFormat(input);
}

[LaurentVB]

Hey guys, thanks for your feedback. I think we do not have the same use case in mind for this library. Fundamentally, if the user misplaces a space, or types lowercase letters, it can still be validated and accepted as a valid IBAN. The internal representation, though, is subject to specifications (captured in the electronicFormat and printFormat functions). So I think what we have here is a problem of documentation. Yes, you should always use the electronicFormat when you store or send the IBAN. It's the canonical representation of the IBAN. Yes, isValid will return true for anything that can be trivially converted to a valid IBAN in canonical format. This library aims to adhere to the principle of robustness. But you're right that it deserves to be documented 👍

For your use case, I think this lib can also help, though:

var strictlyValid = (input === IBAN.electronicFormat(input)) && IBAN.isValid(input);

That said, a bank that drops a payment because the IBAN was not all uppercase is 😱

[Arkkimaagi]

This one caught us by surprise as the naming is confusing. It's not just documentation that should be improved.

Instead of ‘isValid‘ it would be clearer if ‘containsIBAN‘ or similar would be used. Generally I feel that a isValid should be strict by default and lax alternatives should be named differently.

[Moejoe90]

Whats the status of this issue?

[mpkorstanje]

The only thing on which there is a consensus is that the documentation could stand some improvement but so far no one has bothered to do so yet.

I submitted https://github.com/mmjmanders/ng-iban/pull/19 a while ago and that has been accepted and merged. As far as I am concerned the problem has been resolved.

[stefanve]

I think maybe a IBAN.clean(iban) could be helpful because although spaces and some other strengess are allowed in the specs, most banks does not validate them. clean() should make uppercase remove all that is not A-Z 0-9 I had a IBAN with ^ in it, it came out valid....