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=ema... : 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=ema... : 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=ema... : PS4, Line 142: ma may
https://gerrit.osmocom.org/c/pysim/+/41508/comment/1d98c1e2_4365c390?usp=ema... : PS4, Line 245: Csv ```suggestion available from within pySim-shell via the `CardKeyProviderPgsql`. ```
https://gerrit.osmocom.org/c/pysim/+/41508/comment/11c4895c_135a7fa2?usp=ema... : 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=ema... : 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=ema... : 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=ema... : 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!