Skip to content
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

Lowercased country code fails with InvalidStructureResult #24

Closed
Hoffs opened this issue Jul 2, 2020 · 1 comment
Closed

Lowercased country code fails with InvalidStructureResult #24

Hoffs opened this issue Jul 2, 2020 · 1 comment
Milestone

Comments

@Hoffs
Copy link

Hoffs commented Jul 2, 2020

I have been looking at this for a while and can't find concrete information which way it should work. Right now I can see that the validation in StructureValidator tries to match first symbols with expected country code. As I see, because country code is lowercased it will not be equals to expected country code which is always uppercased:

if (iban[0] != _expectedCountryCode[0] && iban[1] != _expectedCountryCode[1])

But in other places, like Mod97 validation or finding iban country it uses upper cased ones, which makes it pass:

private static string? GetCountryCode(string value)
{
return value.Length < 2
? null
: new string(new[]
{
char.ToUpperInvariant(value[0]),
char.ToUpperInvariant(value[1])
});
}

// - For letters, always is two digits so shift 2 digits.
// - Use bitwise OR with ' ' (space, 0x20) to convert char to lowercase.
// - Then subtract 'a' to get value 0, 1, 2, etc.
// - Last, add 10 so: - a = 10, b = 11, c = 12, etc.

So I wonder, is this valid and expected behavior?

@skwasjer
Copy link
Owner

skwasjer commented Jul 3, 2020

The check digits calculation is correctly ignoring case, as is the country code rule. I think this is a remnant of older implementation where IbanNet was built around Swift registry specifically, but was refactored to support multiple providers. This specific country code check should actually be made a 'test' and injected as a StructureSegmentTest, this would solve the problem and allow Swift provider to still be strict but allow more relaxed providers to pass this check.

That said, using strict validation, it would still fail when you'd have an IBAN with lowercase further up in the IBAN (for countries that require upper case), so the next debate would be, should we allow this at all? This is also why there is a loose validation mode. Alternatively, the SwiftStructureValidationFactory could be opened up a bit for extensibility/configurability.

So in short, yes, it is expected behavior, or at least, lets call it originally as intended. The entire IBAN could perhaps be uppercased to solve the problem, but I have to investigate if that breaks the spec.

skwasjer added a commit that referenced this issue Aug 8, 2020
@skwasjer skwasjer added this to the v4.1 milestone Aug 8, 2020
skwasjer added a commit that referenced this issue Aug 8, 2020
@skwasjer skwasjer mentioned this issue Aug 8, 2020
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

No branches or pull requests

2 participants