Merge pull request #3576 from pkilar/epassport-extended-docnum

Fix the ePassport personal number read from the MRZ optional-data field

CHANGELOG.md

@@ -3,6 +3,7 @@ All notable changes to this project will be documented in this file.
 This project uses the changelog in accordance with [keepchangelog](http://keepachangelog.com/). Please use this to write notable changes, which is not the same as git commit log...
 
 ## [unreleased][unreleased]
+- Fixed `tools/ePassport` - the personal number read from the MRZ optional-data field showed a filler character where a space belonged, and a document number longer than nine characters left its overflow there (@pkilar)
 - Fixed `tools/ePassport` - the detail tabs named the files their data came from as fixed text, so a document without EF_DG11 was told otherwise, and the personal-number caption wrapped (@pkilar)
 - Added `tools/ePassport` - EF_DG13 is decoded, and a Polish document's PESEL is shown as the personal number when it carries no EF_DG11 (@pkilar)
 - Fixed `tools/ePassport` - a disabled button drew Kivy's banded disabled texture, which left its label unreadable (@pkilar)

tools/ePassport/epassport/emrtd/model.py

@@ -10,7 +10,7 @@ from __future__ import annotations
 from dataclasses import dataclass, field
 from pathlib import Path
 
-from .mrz import Mrz
+from .mrz import FILLER, Mrz
 
 
 class FileState:
@@ -81,6 +81,17 @@ class DocumentDetails:
         return not any(v for k, v in vars(self).items() if not k.startswith("image_"))
 
 
+def _mrz_personal_number(field: str) -> str:
+    """The MRZ optional-data field read as a personal number.
+
+    9303 replaces every space and special character in the number with ``<``
+    and then pads the field with the same character, so the trailing run is
+    padding while a filler inside the number stood for something we cannot
+    recover.  A space is the closest we can put back.
+    """
+    return field.rstrip(FILLER).replace(FILLER, " ").strip()
+
+
 #: Poland carries the PESEL, its national identity number, in this DG13 tag.
 PESEL_TAG = "5F70"
 
@@ -232,7 +243,7 @@ class PassportRecord:
         if self.personal.personal_number:
             return self.personal.personal_number
         if self.mrz is not None:
-            optional = self.mrz.optional_data.value.strip()
+            optional = _mrz_personal_number(self.mrz.optional_data.value)
             if optional:
                 return optional
         if self.dg13_personal_number:
@@ -258,7 +269,7 @@ class PassportRecord:
         """Where :attr:`personal_number` came from, for an honest caption."""
         if self.personal.personal_number:
             return "DG11"
-        if self.mrz is not None and self.mrz.optional_data.value.strip():
+        if self.mrz is not None and _mrz_personal_number(self.mrz.optional_data.value):
             return "MRZ"
         if self.dg13_personal_number:
             return "DG13"

tools/ePassport/epassport/emrtd/mrz.py

@@ -483,8 +483,12 @@ def _parse_td3(lines: list[str], today: _dt.date | None) -> Mrz:
         date_of_birth=Checked(l2[13:19], l2[19], verify(l2[13:19], l2[19])),
         sex=l2[20],
         date_of_expiry=Checked(l2[21:27], l2[27], verify(l2[21:27], l2[27])),
+        # The value comes from what _extended_document_number left behind: a
+        # long document number spills into this field, and that spill belongs
+        # to the number, not to the State's optional data.  The check digit
+        # still answers for the field as transmitted, overflow included.
         optional_data=Checked(
-            l2[28:42].rstrip(FILLER), l2[42], verify(l2[28:42], l2[42])
+            optional.rstrip(FILLER), l2[42], verify(l2[28:42], l2[42])
         ),
         composite=Checked(
             "",

tools/ePassport/tests/test_mrz.py

@@ -193,3 +193,67 @@ def test_split_names_handles_single_and_missing_given_names() -> None:
     assert mrz.split_names("ERIKSSON<<ANNA<MARIA<<<<") == ("ERIKSSON", "ANNA MARIA")
     assert mrz.split_names("ERIKSSON<<<<<<") == ("ERIKSSON", "")
     assert mrz.split_names("VAN<DER<BERG<<JAN<<<") == ("VAN DER BERG", "JAN")
+
+
+# --------------------------------- document numbers longer than nine characters
+def _td3_with_long_number(number: str = "AB1234567890") -> list[str]:
+    """ICAO puts the overflow in the optional-data field.
+
+    Positions 1-9 hold the first nine characters, position 10 a filler in
+    place of the check digit, and the optional-data field opens with the rest
+    of the number followed by the check digit for the whole of it.
+    """
+    head, rest = number[:9], number[9:]
+    dob, expiry = "740812", "120415"
+    optional = (rest + mrz.check_digit(number)).ljust(14, "<")
+    body = (
+        head
+        + "<"
+        + "UTO"
+        + dob
+        + mrz.check_digit(dob)
+        + "F"
+        + expiry
+        + mrz.check_digit(expiry)
+        + optional
+        + mrz.check_digit(optional)
+    )
+    line2 = body + mrz.check_digit(body[0:10] + body[13:20] + body[21:43])
+    return ["P<UTOERIKSSON<<ANNA<MARIA".ljust(44, "<"), line2]
+
+
+def test_a_long_document_number_is_put_back_together() -> None:
+    parsed = mrz.parse(_td3_with_long_number())
+    assert parsed.document_number.value == "AB1234567890"
+    assert parsed.document_number.ok is True
+
+
+def test_the_overflow_does_not_linger_in_the_optional_field() -> None:
+    """It is part of the document number, not data the State chose to add.
+
+    Left there it was reported as the holder's personal number, since that
+    falls back to the MRZ optional-data field when DG11 carries none.
+    """
+    parsed = mrz.parse(_td3_with_long_number())
+    assert parsed.optional_data.value == ""
+
+
+def test_a_nine_character_number_leaves_the_optional_field_alone() -> None:
+    dob, expiry = "740812", "120415"
+    optional = "ZE184226B".ljust(14, "<")
+    body = (
+        "L898902C"
+        + "<"
+        + mrz.check_digit("L898902C<")
+        + "UTO"
+        + dob
+        + mrz.check_digit(dob)
+        + "F"
+        + expiry
+        + mrz.check_digit(expiry)
+        + optional
+        + mrz.check_digit(optional)
+    )
+    line2 = body + mrz.check_digit(body[0:10] + body[13:20] + body[21:43])
+    parsed = mrz.parse(["P<UTOERIKSSON<<ANNA".ljust(44, "<"), line2])
+    assert parsed.optional_data.value == "ZE184226B"

tools/ePassport/tests/test_personal_number.py

@@ -0,0 +1,53 @@
+"""The personal number taken from the MRZ optional-data field.
+
+ICAO 9303 Part 4, positions 29 to 42: "Any special characters, including
+spaces, in the personal identification number ... shall be replaced by the
+filler character (<). The number shall be followed by the filler character
+(<) repeated up to position 42."
+
+So the trailing run is padding, and a filler inside the number stands for a
+space or a special character that was substituted on the way in.  Printing it
+raw put a '<' in the middle of the number.
+"""
+
+from __future__ import annotations
+
+from epassport.emrtd.model import PassportRecord
+from epassport.emrtd.mrz import Checked, Mrz
+
+
+def _record(optional: str) -> PassportRecord:
+    record = PassportRecord()
+    blank = Checked("")
+    record.mrz = Mrz(
+        kind="TD3",
+        lines=[],
+        document_number=blank,
+        date_of_birth=blank,
+        date_of_expiry=blank,
+        composite=blank,
+        nationality="USA",
+        optional_data=Checked(optional),
+    )
+    return record
+
+
+def test_a_filler_inside_the_number_stands_for_a_space() -> None:
+    assert _record("123456789<0123").personal_number == "123456789 0123"
+
+
+def test_trailing_padding_never_reaches_the_value() -> None:
+    assert _record("ZE184226B").personal_number == "ZE184226B"
+    assert _record("ZE184226B<<<<<").personal_number == "ZE184226B"
+
+
+def test_a_plain_number_is_untouched() -> None:
+    assert _record("6512168").personal_number == "6512168"
+
+
+def test_an_empty_field_yields_no_number() -> None:
+    assert _record("").personal_number_source == ""
+
+
+def test_the_source_is_still_reported_as_the_mrz() -> None:
+    assert _record("123456789<0123").personal_number_source == "MRZ"