search

MariaDB / mariadb_kernel

A MariaDB Jupyter kernel
BSD 3-Clause "New" or "Revised" License
30 stars 21 forks source link

Switch to the official Python package to connect with the server #27

Open KartikSoneji opened 3 years ago

KartikSoneji commented 3 years ago

https://github.com/MariaDB/mariadb_kernel/blob/0dddfe5cd8c3c9ad80687fed05401cc5313e6d86/mariadb_kernel/mariadb_client.py#L21-L27 The current implementation of using the mariadb client cli has some limitations.

https://github.com/MariaDB/mariadb_kernel/blob/0dddfe5cd8c3c9ad80687fed05401cc5313e6d86/mariadb_kernel/mariadb_client.py#L100

The run_statement doesn't support parameter substitution, which can lead to unintentional SQL injections while adding more magics:

https://github.com/MariaDB/mariadb_kernel/blob/362e378c15d985779c83404796a2f4aa14578535/mariadb_kernel/maria_magics/load.py#L46 https://github.com/MariaDB/mariadb_kernel/blob/362e378c15d985779c83404796a2f4aa14578535/mariadb_kernel/maria_magics/load.py#L57

Security is not as much of an issue, but it can lead to issues with some commands, for example if the file is named ' a.csv. Trying to escape these edge cases in Python will lead to an imperfect re-implementation of the escaping logic like the original connector.

Ideally, the run_statement method should accept a list of substitution parameters like the Python connector

cur.execute("INSERT INTO test.accounts(first_name, last_name, email, amount) VALUES (?, ?, ?, ?)",
      (first_name, last_name, email, amount))

https://github.com/MariaDB/mariadb_kernel/blob/0dddfe5cd8c3c9ad80687fed05401cc5313e6d86/mariadb_kernel/mariadb_client.py#L50

Listening for the MariaDB [] prompt causes some queries to never finish or truncate the output. The Python connector will be more reliable as it is both officially supported and throughly tested.

image

LinuxJedi commented 1 year ago

Whatever the fix for this ends up being it will likely also fix #20. I'm going to mark it as an enhancement even though it is borderline between bug and enhancement.