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

feat: add whitespace filtering to iban validator #391

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

Erawpalassalg
Copy link

  • add a new function to filter whitespace to IBAN validator

- add a new function to filter whitespace to IBAN validator
@yozachar
Copy link
Collaborator

yozachar commented Jul 19, 2024

Hi, @Erawpalassalg, this library does not perform input data sanitization (mostly). See the cards module. Those numbers could be separated by spaces, hyphens, or slashes, but that's not taken into account for validation.

@Erawpalassalg
Copy link
Author

Erawpalassalg commented Jul 19, 2024

Hi @yozachar, thanks for letting me know.

Would this matter be open to discussion? As I believe that by only validating the machine-readable format this lib puts the formatting pressure on the end user rather than on the developer/company storing the numbers.

If not, could this be stated very clearly in the docs? (For the IBAN case, the discrepancy between the human-readable format and the machine-readable format is a whole part of the Wikipedia related page)

@yozachar
Copy link
Collaborator

Would this matter be open to discussion? As I believe that by only validating the machine-readable format this lib puts the formatting pressure on the end user rather than on the developer/company storing the numbers.

I suppose an end-user would be a non-developer, in that case, they are never going to use this library directly. Another developer/company who works for the end-user/client, making use of this library, can and should, sanitize the human-readable input (from the end-user) before validating it.

Regardless, if spaces are the only separators, I suppose it can be considered (even though input sanitization is beyond the scope of this library).

@@ -37,8 +42,12 @@ def iban(value: str, /):
(Literal[True]): If `value` is a valid IBAN code.
(ValidationError): If `value` is an invalid IBAN code.
"""
filtered_value = _filter_whitespace(value)
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

- filtered_value = _filter_whitespace(value)
+ if not value:
+     return False
+ 
+ # Remove Unicode white-spaces from the string.
+ value = "".join(filter(lambda x: not x.isspace(), value))

I think the function call can be skipped.

@Erawpalassalg
Copy link
Author

Erawpalassalg commented Jul 19, 2024

I went a little further with this IBAN stuff.

Apparently a lib already does all the validation taking into account the specifics of the ISO-13616 mentioned in this document, and it's called schwifty.

The interesting things when playing around with schwifty are:

  • They don't care about spaces in the input (suppressing them internally)
  • Some of their invalid test cases actually pass validators' validation process (I double-checked with an online validator, which is consistent with schwifty), said test cases are:
    • GB96 BARC 2020 1530 0934 591 -- Too long
    • GB12 BARC 2020 1530 093A 59 -- Wrong account format (country specific)
    • GB01 BARC 2071 4583 6083 87 -- Wrong checksum digits
    • GB00 HLFX 1101 6111 4553 65 -- Wrong checksum digits

So, I was wondering if it would be interesting to start using schwifty as a dependency to be compliant with ISO-13616?

@yozachar
Copy link
Collaborator

Cool, I'm open to adding it as an optional dependency.

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

Successfully merging this pull request may close these issues.

2 participants