FazBrowse GitHub Viewer | Trending |
URL:
| Home
Tools: [Download Repo ZIP]   [Original HTTPS Page]

fix(finance): validate the ISIN check digit (#440) by apoorvdarshan · Pull Request #473 · python-validators/validators · GitHub

Repository navigation

fix(finance): validate the ISIN check digit (#440) - #473

Open
apoorvdarshan wants to merge 1 commit into
python-validators:masterfrom
apoorvdarshan:fix-440-isin-checksum
Open

apoorvdarshan wants to merge 1 commit into
python-validators:masterfrom
apoorvdarshan:fix-440-isin-checksum

Conversation

Copy link
Copy Markdown

Summary

Fixes #440 (the ISIN portion). validators.isin() accepts any invalid ISIN check digit.

>>> import validators
>>> bool(validators.isin("US0378331005"))   # Apple, genuinely valid
True
>>> bool(validators.isin("US0378331004"))   # same but wrong check digit
True                                          # <-- should be False

Root cause

_isin_checksum() computes a per-character val and doubles it for odd positions, but never accumulates it into check:

check, val = 0, None
for idx in range(12):
    ...
    if idx & 1:
        val += val      # doubles val, but val is then discarded
return (check % 10) == 0  # check is always 0 -> always True

So the checksum is never actually validated — every 12-character string with a two-letter country code and a trailing digit passes. (_cusip_checksum and sedol accumulate 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

  • Validated against 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).
  • This cross-check also revealed the existing test fixture JP000K0VF054 was itself an invalid ISIN (correct check digit is 5); it only passed before because the checksum was never computed. Corrected to JP000K0VF055 and added wrong-check-digit fixtures.
  • New tests fail on master and pass with this change. tests/test_finance.py: 31 passed (incl. doctests), up from 21.
  • ruff check and ruff format --check clean on the changed files.
  • The 17 pre-existing tests/crypto_addresses/test_eth_address.py failures are unrelated (they fail identically on master).

Disclosure: prepared with AI assistance; reviewed and cross-checked locally (incl. against python-stdnum).

_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 branch has not been deployed

No deployments
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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Incorrect finance Validation Logic

1 participant


Back | FazBrowse Home | New Git URL