Attention is currently required from: laforge, neels.

fixeria has posted comments on this change by neels. ( 
https://gerrit.osmocom.org/c/pysim/+/41920?usp=email )

Change subject: saip SmspTpScAddr: safeguard against decoding error
......................................................................


Patch Set 5: Code-Review-2

(3 comments)

Commit Message:

https://gerrit.osmocom.org/c/pysim/+/41920/comment/ec26580a_e896b2fa?usp=email :
PS2, Line 13: The underlying issue, I presume, is that either the data needs
            : validation before decode_record_bin(), or decode_record_bin() 
needs
            : well-defined error handling.
> We are here reading an example template from GSMA and get an exception.


But with this patch we're silently dropping `EF.SMPP` as if it was not present; 
we're not even logging an error. And we're not catching other possible kinds of 
exceptions here, like `ConstructError`, so this does not solve the problem 
fundamentally.


Commit Message:

https://gerrit.osmocom.org/c/pysim/+/41920/comment/963afce7_3df32a12?usp=email :
PS5, Line 55:   File 
"/application/venv/lib/python3.13/site-packages/osmocom/construct.py", line 
268, in _decode
            :     if r[-1] == 'f':
            :        ~^^^^
            :   File 
"/application/venv/lib/python3.13/site-packages/osmocom/utils.py", line 50, in 
__getitem__
            :     return hexstr(super().__getitem__(val))
            :                   ~~~~~~~~~~~~~~~~~~~^^^^^
            : In
Looking at the backtrace you provided, the `IndexError` comes from our
pyosmocom code, not from construct. I checked the related code path and it 
looks like the problem has been fixed in 
https://gerrit.osmocom.org/c/python/pyosmocom/+/41731? I think we can abandon 
this patch.


File pySim/esim/saip/personalization.py:

https://gerrit.osmocom.org/c/pysim/+/41920/comment/46840694_deda9b7a?usp=email :
PS5, Line 618:             f_smsp = pe.files.get('ef-smsp', None)
             :             if f_smsp is None:
             :
We may still want to merge this part though.



--
To view, visit https://gerrit.osmocom.org/c/pysim/+/41920?usp=email
To unsubscribe, or for help writing mail filters, visit 
https://gerrit.osmocom.org/settings?usp=email

Gerrit-MessageType: comment
Gerrit-Project: pysim
Gerrit-Branch: master
Gerrit-Change-Id: Ic436e206776b81f24de126e8ee0ae8bf5f3e8d7a
Gerrit-Change-Number: 41920
Gerrit-PatchSet: 5
Gerrit-Owner: neels <[email protected]>
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: fixeria <[email protected]>
Gerrit-CC: laforge <[email protected]>
Gerrit-Attention: neels <[email protected]>
Gerrit-Attention: laforge <[email protected]>
Gerrit-Comment-Date: Sat, 25 Apr 2026 20:30:43 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: Yes
Comment-In-Reply-To: neels <[email protected]>
Comment-In-Reply-To: laforge <[email protected]>

Reply via email to