- Notifications
You must be signed in to change notification settings - Fork 311
Description
After rebasing our local branch onto mainline for the first time in way too long, our SQL output bots started crashing when inserting data into the database, with the following backtrace:
Traceback (most recent call last): File "/usr/local/lib/python3.6/site-packages/intelmq/lib/bot.py", line 319, in start self.process() File "/usr/local/lib/python3.6/site-packages/intelmq/bots/outputs/sql/output.py", line 56, in process if self.execute(query, values, rollback=True): File "/usr/local/lib/python3.6/site-packages/intelmq/lib/mixins/sql.py", line 118, in execute self.cur.execute(query, values) File "src/pymssql/_pymssql.pyx", line 460, in pymssql._pymssql.Cursor.execute File "src/pymssql/_mssql.pyx", line 1104, in pymssql._mssql.MSSQLConnection.execute_query File "src/pymssql/_mssql.pyx", line 1135, in pymssql._mssql.MSSQLConnection.execute_query File "src/pymssql/_mssql.pyx", line 1252, in pymssql._mssql.MSSQLConnection.format_and_run_query File "src/pymssql/_mssql.pyx", line 1274, in pymssql._mssql.MSSQLConnection.format_sql_command File "src/pymssql/_mssql.pyx", line 2038, in pymssql._mssql._substitute_params ValueError: 'params' arg (<class 'list'>) can be only a tuple or a dictionary. This seems to be because after #2223 all values going to the database now pass through the function prepare_values() in the SQL output bot:
intelmq/intelmq/bots/outputs/sql/output.py
Lines 60 to 67 in e411d4f
| def prepare_values(self, values): | |
| if self._engine_name == self.POSTGRESQL: | |
| # escape JSON-encoded NULL characters. JSON escapes them once, but we need to escape them twice, | |
| # so that Postgres does not encounter a NULL char while decoding it | |
| # https://github.com/certtools/intelmq/issues/2203 | |
| return [value.replace('\\u0000', '\\\\u0000') if isinstance(value, str) else value for value in values] | |
| else: | |
| return list(values) |
prepare_values() takes parameters in the form of a tuple but returns them transformed in the form of a list, which is then passed directly as the parameters argument to the database API's Cursor.execute() method.
PEP-0249 is not explicit on the type of parameters, but pymssql accepts an atom or tuple, psycopg2 accepts a tuple or dictionary and sqlite3 is also not explicit, but defaults to a empty tuple.
Notably, none of the three database APIs supported by IntelMQ are documented to accept the list that prepare_values() returns, but all are documented to accept tuples.
I changed the existing
return list(values) to simply
return valueswhich is a tuple and works for me.
But I am unclear on why prepare_values() changes the data type of its input from a tuple to a list. Am I missing something? Do your tests pass with lists?
Also, I have only changed the else branch and tested with MSSQL. I don't have a PostgreSQL instance handy, and thus can't test that branch.