Attention is currently required from: dexter, laforge.

daniel has posted comments on this change by dexter. ( 
https://gerrit.osmocom.org/c/pysim/+/41508?usp=email )

Change subject: card_key_provider: add PostgreSQL support
......................................................................


Patch Set 4: Code-Review-1

(8 comments)

Commit Message:

https://gerrit.osmocom.org/c/pysim/+/41508/comment/830fbe59_38a2a5b1?usp=email :
PS4, Line 15: a
remove
```suggestion
This patch adds PostgreSQL support next to the existing CSV
```


File docs/card-key-provider.rst:

https://gerrit.osmocom.org/c/pysim/+/41508/comment/416458c8_79aedbad?usp=email :
PS4, Line 25: CardKeyProviderCsv
```suggestion
to retrieve the key material from a PostgreSQL database 
(`CardKeyProviderPgsql`).
```


https://gerrit.osmocom.org/c/pysim/+/41508/comment/2ffa0714_cff589f2?usp=email :
PS4, Line 142: ma
may


https://gerrit.osmocom.org/c/pysim/+/41508/comment/1d98c1e2_4365c390?usp=email :
PS4, Line 245: Csv
```suggestion
available from within pySim-shell via the `CardKeyProviderPgsql`.
```


https://gerrit.osmocom.org/c/pysim/+/41508/comment/11c4895c_135a7fa2?usp=email :
PS4, Line 293: CardKeyProviderCsv
```suggestion
As soon as the CardKeyProvider finds a line (row) in your CSV file
```


File pySim-shell.py:

https://gerrit.osmocom.org/c/pysim/+/41508/comment/8fc0f5c2_901d0501?usp=email :
PS4, Line 1186:         
card_key_provider_register(CardKeyProviderPgsql(opts.pqsql, column_keys))
That means you can use both providers at the same time, right?


File pySim/card_key_provider.py:

https://gerrit.osmocom.org/c/pysim/+/41508/comment/ef0e5aef_fab2de36?usp=email :
PS4, Line 218:         column_names = ""
             :         for f in fields:
             :             column_names += f.lower() + ", "
             :         column_names = column_names[:-2]
This can be done with join:
```
column_names = ", ".join([f.lower() for f in fields])
```


https://gerrit.osmocom.org/c/pysim/+/41508/comment/1fed683c_51be4aaf?usp=email :
PS4, Line 229:             cur.execute("SELECT column_name FROM 
information_schema.columns where table_name = '%s';" % t)
I believe here and in all other cur.execute statements you don't want to use 
'%". See https://www.psycopg.org/docs/usage.html#sql-injection

psycopg takes care of escaping etc., just pass the arguments in a tuple as a 
separate parameter (e.g. `, (t,)` instead of `% t` here)

That's probably even more important in the import script above!



--
To view, visit https://gerrit.osmocom.org/c/pysim/+/41508?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: Icba625c02a60d7e1f519b506a46bda5ded0537d3
Gerrit-Change-Number: 41508
Gerrit-PatchSet: 4
Gerrit-Owner: dexter <[email protected]>
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: daniel <[email protected]>
Gerrit-Reviewer: laforge <[email protected]>
Gerrit-Attention: laforge <[email protected]>
Gerrit-Attention: dexter <[email protected]>
Gerrit-Comment-Date: Wed, 26 Nov 2025 11:33:40 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: Yes

Reply via email to