fix(isIBAN): adjusting Ireland and Palestine IBAN regex #2518
Conversation
|
Thanks for this PR. I was able to verify that your changes seem correct. Can you add some test cases so we don't accidentally change it back? |
|
Of course. I've updated the tests to include both (previously) invalid and now correct IBAN's for both of these countries. |
src/lib/isIBAN.js
Outdated
| PK: /^(PK[0-9]{2})[A-Z0-9]{4}\d{16}$/, | ||
| PL: /^(PL[0-9]{2})\d{24}$/, | ||
| PS: /^(PS[0-9]{2})[A-Z0-9]{4}\d{21}$/, | ||
| PS: /^(PS[0-9]{2})[A-Z]{4}\d{21}$/, |
There was a problem hiding this comment.
Kindly correct me, but If I understand this correctly, the Palestine RegExp would be wrong here (the last part):
On page 6 of the supplied PDF we can see the convention:
The supplied PDF says the IBAN structure is PS2!n4!a21!c
The regex here shows /^(PS[0-9]{2})[A-Z]{4}\d{21}$/
So I would argue, that the \d{21} part should actually be [a-zA-Z0-9] or [A-Z0-9] (since we don't care about case, I think).
what do you think?
There was a problem hiding this comment.
Good catch. I was focused on the first part of the regex since it was the one that gave me trouble 😄.
As for the [a-zA-Z0-9] or [A-Z0-9] part, I've adjusted regex to include only capital letters, since from what I was able to gather IBAN should be case-insensitive and we're already converting everything to uppercase. I've added a test case that includes a letter as well.
…haracters instead of only numbers
4 participants
Changes:
Palestine and Ireland IBAN's should consist of
countryCodefollowed by 2 numbers and then 4[A-Z]symbols, not[A-Z0-9]. This PR adjusts these 2 regexes to follow the required standard.Sources:
According to Swift documentation, specifically this file here
Checklist