fix(finance): validate the ISIN check digit (#440)#473
Open
apoorvdarshan wants to merge 1 commit into
Open
Conversation
_isin_checksum() never accumulated into `check`, so it stayed 0 and `check % 10 == 0` was always true. Any 12-character string with a two-letter country code and a trailing digit was accepted regardless of its check digit (e.g. US0378331004, an invalid variant of Apple's US0378331005, validated as a valid ISIN). Implement the real algorithm: expand letters to digits (A=10..Z=35), keep digits as-is, then run the Luhn checksum over the result. Also enforce that the country code is alphabetic and the check digit numeric, rejecting lowercase/malformed input as requested in the issue. Cross-checked against python-stdnum: this also revealed that the existing 'valid' test fixture JP000K0VF054 was itself an invalid ISIN (correct check digit is 5) that only passed because the checksum was never computed; corrected to JP000K0VF055 and added wrong-check-digit cases. Fixes python-validators#440.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Fixes #440 (the ISIN portion).
validators.isin()accepts any invalid ISIN check digit.Root cause
_isin_checksum()computes a per-charactervaland doubles it for odd positions, but never accumulates it intocheck:So the checksum is never actually validated — every 12-character string with a two-letter country code and a trailing digit passes. (
_cusip_checksumandsedolaccumulate correctly and are unaffected.)Fix
Implement the real ISIN algorithm: expand letters to digits (
A=10 … Z=35), keep digits as-is, then run the Luhn checksum over the resulting digit string. Also enforce that the country code (first two chars) is alphabetic and the check digit (last char) numeric, so lowercase/malformed inputs are rejected (as the issue requests).Verification
python-stdnum's ISIN validator: my implementation agrees on every case tested (real ISINs for Apple/Microsoft/BAE/Alphabet/Bayer/etc. → valid; wrong-check-digit variants → invalid).JP000K0VF054was itself an invalid ISIN (correct check digit is5); it only passed before because the checksum was never computed. Corrected toJP000K0VF055and added wrong-check-digit fixtures.masterand pass with this change.tests/test_finance.py: 31 passed (incl. doctests), up from 21.ruff checkandruff format --checkclean on the changed files.tests/crypto_addresses/test_eth_address.pyfailures are unrelated (they fail identically onmaster).Disclosure: prepared with AI assistance; reviewed and cross-checked locally (incl. against
python-stdnum).