From 0c8a88d4e30b4bb0423ab98715ab43520d5231f1 Mon Sep 17 00:00:00 2001 From: "Erlend E. Aasland" <erlend.aasland@innova.no> Date: Mon, 9 Aug 2021 23:15:20 +0200 Subject: [PATCH 1/9] bpo-44859: Raise InterfaceError iso. ProgrammingError for SQLITE_MISUSE If SQLITE_MISUSE is raised, it is a sqlite3 module bug. Users of the sqlite3 module are not responsible of using the SQLite C API correctly. --- Modules/_sqlite/util.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Modules/_sqlite/util.c b/Modules/_sqlite/util.c index 24cefc626b66e2..bbea10d5c1c509 100644 --- a/Modules/_sqlite/util.c +++ b/Modules/_sqlite/util.c @@ -83,7 +83,7 @@ _pysqlite_seterror(pysqlite_state *state, sqlite3 *db) PyErr_SetString(state->IntegrityError, sqlite3_errmsg(db)); break; case SQLITE_MISUSE: - PyErr_SetString(state->ProgrammingError, sqlite3_errmsg(db)); + PyErr_SetString(state->InterfaceError, sqlite3_errmsg(db)); break; default: PyErr_SetString(state->DatabaseError, sqlite3_errmsg(db)); From aa755d23e3516e842ab99de08db36af4b267a168 Mon Sep 17 00:00:00 2001 From: "Erlend E. Aasland" <erlend.aasland@innova.no> Date: Mon, 9 Aug 2021 23:37:49 +0200 Subject: [PATCH 2/9] Don't overwrite BufferErrors when converting to BLOB --- Modules/_sqlite/connection.c | 2 -- Modules/_sqlite/statement.c | 1 - 2 files changed, 3 deletions(-) diff --git a/Modules/_sqlite/connection.c b/Modules/_sqlite/connection.c index 67160c4c449aa1..838d6a98333100 100644 --- a/Modules/_sqlite/connection.c +++ b/Modules/_sqlite/connection.c @@ -533,8 +533,6 @@ _pysqlite_set_result(sqlite3_context* context, PyObject* py_val) } else if (PyObject_CheckBuffer(py_val)) { Py_buffer view; if (PyObject_GetBuffer(py_val, &view, PyBUF_SIMPLE) != 0) { - PyErr_SetString(PyExc_ValueError, - "could not convert BLOB to buffer"); return -1; } if (view.len > INT_MAX) { diff --git a/Modules/_sqlite/statement.c b/Modules/_sqlite/statement.c index 2d5c72d13b7edb..a5b450d55e8466 100644 --- a/Modules/_sqlite/statement.c +++ b/Modules/_sqlite/statement.c @@ -184,7 +184,6 @@ int pysqlite_statement_bind_parameter(pysqlite_Statement* self, int pos, PyObjec case TYPE_BUFFER: { Py_buffer view; if (PyObject_GetBuffer(parameter, &view, PyBUF_SIMPLE) != 0) { - PyErr_SetString(PyExc_ValueError, "could not convert BLOB to buffer"); return -1; } if (view.len > INT_MAX) { From e51d6c14f144aaf05f0d9a8dc743ea49411db430 Mon Sep 17 00:00:00 2001 From: "Erlend E. Aasland" <erlend.aasland@innova.no> Date: Mon, 9 Aug 2021 23:44:37 +0200 Subject: [PATCH 3/9] Raise ProgrammingError if user tries to execute() more than one statement --- Lib/sqlite3/test/dbapi.py | 5 +++-- Modules/_sqlite/statement.c | 2 +- 2 files changed, 4 insertions(+), 3 deletions(-) diff --git a/Lib/sqlite3/test/dbapi.py b/Lib/sqlite3/test/dbapi.py index 5d7e5bba05bc45..c23d507f64ecef 100644 --- a/Lib/sqlite3/test/dbapi.py +++ b/Lib/sqlite3/test/dbapi.py @@ -312,8 +312,9 @@ def test_execute_illegal_sql(self): self.cu.execute("select asdf") def test_execute_too_much_sql(self): - with self.assertRaises(sqlite.Warning): - self.cu.execute("select 5+4; select 4+5") + self.assertRaisesRegex(sqlite.ProgrammingError, + "You can only execute one statement at a time", + self.cu.execute, "select 5+4; select 4+5") def test_execute_too_much_sql2(self): self.cu.execute("select 5+4; -- foo bar") diff --git a/Modules/_sqlite/statement.c b/Modules/_sqlite/statement.c index a5b450d55e8466..5a73b7e4afae03 100644 --- a/Modules/_sqlite/statement.c +++ b/Modules/_sqlite/statement.c @@ -85,7 +85,7 @@ pysqlite_statement_create(pysqlite_Connection *connection, PyObject *sql) } if (pysqlite_check_remaining_sql(tail)) { - PyErr_SetString(connection->Warning, + PyErr_SetString(connection->ProgrammingError, "You can only execute one statement at a time."); goto error; } From 452ce400a124fb6b5c7f1e52b94c534f179fc672 Mon Sep 17 00:00:00 2001 From: "Erlend E. Aasland" <erlend.aasland@innova.no> Date: Mon, 9 Aug 2021 23:46:29 +0200 Subject: [PATCH 4/9] Raise DataError if query contains null chars --- Lib/sqlite3/test/regression.py | 16 ++++++++++------ Modules/_sqlite/statement.c | 2 +- 2 files changed, 11 insertions(+), 7 deletions(-) diff --git a/Lib/sqlite3/test/regression.py b/Lib/sqlite3/test/regression.py index ddf36e71819445..15d9a6eecc71d1 100644 --- a/Lib/sqlite3/test/regression.py +++ b/Lib/sqlite3/test/regression.py @@ -338,12 +338,16 @@ def test_invalid_isolation_level_type(self): def test_null_character(self): # Issue #21147 - con = sqlite.connect(":memory:") - self.assertRaises(ValueError, con, "\0select 1") - self.assertRaises(ValueError, con, "select 1\0") - cur = con.cursor() - self.assertRaises(ValueError, cur.execute, " \0select 2") - self.assertRaises(ValueError, cur.execute, "select 2\0") + cur = self.con.cursor() + queries = ["\0select 1", "select 1\0"] + msg = "the query contains a null character" + for query in queries: + with self.subTest(query=query): + self.assertRaisesRegex(sqlite.DataError, msg, + self.con.execute, query) + with self.subTest(query=query): + self.assertRaisesRegex(sqlite.DataError, msg, + cur.execute, query) def test_surrogates(self): con = sqlite.connect(":memory:") diff --git a/Modules/_sqlite/statement.c b/Modules/_sqlite/statement.c index 5a73b7e4afae03..c309c6fda272f9 100644 --- a/Modules/_sqlite/statement.c +++ b/Modules/_sqlite/statement.c @@ -67,7 +67,7 @@ pysqlite_statement_create(pysqlite_Connection *connection, PyObject *sql) return NULL; } if (strlen(sql_cstr) != (size_t)size) { - PyErr_SetString(PyExc_ValueError, + PyErr_SetString(connection->DataError, "the query contains a null character"); return NULL; } From f9a5dc61c1b86790c712302b97e4d7257448e0a0 Mon Sep 17 00:00:00 2001 From: "Erlend E. Aasland" <erlend.aasland@innova.no> Date: Tue, 10 Aug 2021 00:06:01 +0200 Subject: [PATCH 5/9] Add NEWS --- .../next/Library/2021-08-10-00-05-53.bpo-44859.9e9_3V.rst | 7 +++++++ 1 file changed, 7 insertions(+) create mode 100644 Misc/NEWS.d/next/Library/2021-08-10-00-05-53.bpo-44859.9e9_3V.rst diff --git a/Misc/NEWS.d/next/Library/2021-08-10-00-05-53.bpo-44859.9e9_3V.rst b/Misc/NEWS.d/next/Library/2021-08-10-00-05-53.bpo-44859.9e9_3V.rst new file mode 100644 index 00000000000000..accfae1b07f4f1 --- /dev/null +++ b/Misc/NEWS.d/next/Library/2021-08-10-00-05-53.bpo-44859.9e9_3V.rst @@ -0,0 +1,7 @@ +Raise more accurate exceptions in :mod:`sqlite3`. + +* Raise InterfaceError iso. ProgrammingError for SQLITE_MISUSE +* Don't overwrite BufferError with ValueError when converting to BLOB +* Raise ProgrammingError iso. Warning if user tries to execute() more than one + statement +* Raise DataError iso. ValueError if query contains NULL characters From f2300bdc541e480580a63e061af872181d97d78e Mon Sep 17 00:00:00 2001 From: "Erlend E. Aasland" <erlend.aasland@innova.no> Date: Tue, 21 Sep 2021 01:49:44 +0200 Subject: [PATCH 6/9] _pysqlite_set_result() MUST set an exception if it returns -1 Failure to do so, will result in an assertion failure / segfault. --- Lib/sqlite3/test/test_userfunctions.py | 6 ++++++ Modules/_sqlite/connection.c | 4 ++++ 2 files changed, 10 insertions(+) diff --git a/Lib/sqlite3/test/test_userfunctions.py b/Lib/sqlite3/test/test_userfunctions.py index ad408475b73af7..c3183293f9a456 100644 --- a/Lib/sqlite3/test/test_userfunctions.py +++ b/Lib/sqlite3/test/test_userfunctions.py @@ -452,6 +452,12 @@ def test_func_return_too_large_blob(self, size): with self.assertRaises(sqlite.DataError): cur.execute("select largeblob()") + def test_func_return_illegal_value(self): + self.con.create_function("badreturn", 0, lambda: self) + msg = "user-defined function raised exception" + self.assertRaisesRegex(sqlite.OperationalError, msg, + self.con.execute, "select badreturn()") + class AggregateTests(unittest.TestCase): def setUp(self): diff --git a/Modules/_sqlite/connection.c b/Modules/_sqlite/connection.c index eea88aa9f132f7..542611ed11a707 100644 --- a/Modules/_sqlite/connection.c +++ b/Modules/_sqlite/connection.c @@ -560,6 +560,10 @@ _pysqlite_set_result(sqlite3_context* context, PyObject* py_val) sqlite3_result_blob(context, view.buf, (int)view.len, SQLITE_TRANSIENT); PyBuffer_Release(&view); } else { + callback_context *ctx = (callback_context *)sqlite3_user_data(context); + PyErr_Format(ctx->state->ProgrammingError, + "UDF's cannot return '%s' values to SQLite", + Py_TYPE(py_val)->tp_name); return -1; } return 0; From d3186be74fe98e7361ef7b0b228cce4b5369078b Mon Sep 17 00:00:00 2001 From: "Erlend E. Aasland" <erlend.aasland@innova.no> Date: Sat, 26 Feb 2022 23:17:59 +0100 Subject: [PATCH 7/9] Adapt GH-27642 tests --- Lib/test/test_sqlite3/test_userfunctions.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/Lib/test/test_sqlite3/test_userfunctions.py b/Lib/test/test_sqlite3/test_userfunctions.py index 84b59f5e2f41cb..418e9401909f17 100644 --- a/Lib/test/test_sqlite3/test_userfunctions.py +++ b/Lib/test/test_sqlite3/test_userfunctions.py @@ -340,7 +340,8 @@ def test_too_large_int(self): "select spam(?)", (1 << 65,)) def test_non_contiguous_blob(self): - self.assertRaisesRegex(ValueError, "could not convert BLOB to buffer", + self.assertRaisesRegex(BufferError, + "underlying buffer is not C-contiguous", self.con.execute, "select spam(?)", (memoryview(b"blob")[::2],)) From b4a95bdb096d379bc1698778a1506537224ea660 Mon Sep 17 00:00:00 2001 From: "Erlend E. Aasland" <erlend.aasland@innova.no> Date: Sat, 26 Feb 2022 23:27:08 +0100 Subject: [PATCH 8/9] Address review: add tests for PyObject_GetBuffer failures --- Lib/test/test_sqlite3/test_userfunctions.py | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/Lib/test/test_sqlite3/test_userfunctions.py b/Lib/test/test_sqlite3/test_userfunctions.py index 418e9401909f17..5c12461c424604 100644 --- a/Lib/test/test_sqlite3/test_userfunctions.py +++ b/Lib/test/test_sqlite3/test_userfunctions.py @@ -196,6 +196,8 @@ def setUp(self): self.con.create_function("returnlonglong", 0, func_returnlonglong) self.con.create_function("returnnan", 0, lambda: float("nan")) self.con.create_function("returntoolargeint", 0, lambda: 1 << 65) + self.con.create_function("return_noncont_blob", 0, + lambda: memoryview(b"blob")[::2]) self.con.create_function("raiseexception", 0, func_raiseexception) self.con.create_function("memoryerror", 0, func_memoryerror) self.con.create_function("overflowerror", 0, func_overflowerror) @@ -345,6 +347,12 @@ def test_non_contiguous_blob(self): self.con.execute, "select spam(?)", (memoryview(b"blob")[::2],)) + @with_tracebacks(BufferError, regex="buffer.*contiguous") + def test_return_non_contiguous_blob(self): + with self.assertRaises(sqlite.OperationalError): + cur = self.con.execute("select return_noncont_blob()") + cur.fetchone() + def test_param_surrogates(self): self.assertRaisesRegex(UnicodeEncodeError, "surrogates not allowed", self.con.execute, "select spam(?)", From 3792cf0deb82d3aee60d9e2d691cf8539fd37e62 Mon Sep 17 00:00:00 2001 From: "Erlend E. Aasland" <erlend.aasland@innova.no> Date: Tue, 8 Mar 2022 09:59:05 +0100 Subject: [PATCH 9/9] Address reviews --- Lib/test/test_sqlite3/test_regression.py | 5 ++--- .../2021-08-10-00-05-53.bpo-44859.9e9_3V.rst | 15 +++++++++------ Modules/_sqlite/connection.c | 3 ++- Modules/_sqlite/statement.c | 2 +- 4 files changed, 14 insertions(+), 11 deletions(-) diff --git a/Lib/test/test_sqlite3/test_regression.py b/Lib/test/test_sqlite3/test_regression.py index 3491f1cfb1d4ef..aebea59b9e5bbd 100644 --- a/Lib/test/test_sqlite3/test_regression.py +++ b/Lib/test/test_sqlite3/test_regression.py @@ -321,13 +321,12 @@ def test_null_character(self): # Issue #21147 cur = self.con.cursor() queries = ["\0select 1", "select 1\0"] - msg = "the query contains a null character" for query in queries: with self.subTest(query=query): - self.assertRaisesRegex(sqlite.DataError, msg, + self.assertRaisesRegex(sqlite.ProgrammingError, "null char", self.con.execute, query) with self.subTest(query=query): - self.assertRaisesRegex(sqlite.DataError, msg, + self.assertRaisesRegex(sqlite.ProgrammingError, "null char", cur.execute, query) def test_surrogates(self): diff --git a/Misc/NEWS.d/next/Library/2021-08-10-00-05-53.bpo-44859.9e9_3V.rst b/Misc/NEWS.d/next/Library/2021-08-10-00-05-53.bpo-44859.9e9_3V.rst index accfae1b07f4f1..07d7eb0bafb62b 100644 --- a/Misc/NEWS.d/next/Library/2021-08-10-00-05-53.bpo-44859.9e9_3V.rst +++ b/Misc/NEWS.d/next/Library/2021-08-10-00-05-53.bpo-44859.9e9_3V.rst @@ -1,7 +1,10 @@ -Raise more accurate exceptions in :mod:`sqlite3`. +Raise more accurate and :pep:`249` compatible exceptions in :mod:`sqlite3`. -* Raise InterfaceError iso. ProgrammingError for SQLITE_MISUSE -* Don't overwrite BufferError with ValueError when converting to BLOB -* Raise ProgrammingError iso. Warning if user tries to execute() more than one - statement -* Raise DataError iso. ValueError if query contains NULL characters +* Raise :exc:`~sqlite3.InterfaceError` instead of + :exc:`~sqlite3.ProgrammingError` for ``SQLITE_MISUSE`` errors. +* Don't overwrite :exc:`BufferError` with :exc:`ValueError` when conversion to + BLOB fails. +* Raise :exc:`~sqlite3.ProgrammingError` instead of :exc:`~sqlite3.Warning` if + user tries to :meth:`~sqlite3.Cursor.execute()` more than one SQL statement. +* Raise :exc:`~sqlite3.ProgrammingError` instead of :exc:`ValueError` if an SQL + query contains null characters. diff --git a/Modules/_sqlite/connection.c b/Modules/_sqlite/connection.c index 72677db8ea8821..6b60fc7c58217d 100644 --- a/Modules/_sqlite/connection.c +++ b/Modules/_sqlite/connection.c @@ -591,7 +591,8 @@ _pysqlite_set_result(sqlite3_context* context, PyObject* py_val) } else { callback_context *ctx = (callback_context *)sqlite3_user_data(context); PyErr_Format(ctx->state->ProgrammingError, - "UDF's cannot return '%s' values to SQLite", + "User-defined functions cannot return '%s' values to " + "SQLite", Py_TYPE(py_val)->tp_name); return -1; } diff --git a/Modules/_sqlite/statement.c b/Modules/_sqlite/statement.c index ca62664cf4b87a..baa1b71a8daa41 100644 --- a/Modules/_sqlite/statement.c +++ b/Modules/_sqlite/statement.c @@ -67,7 +67,7 @@ pysqlite_statement_create(pysqlite_Connection *connection, PyObject *sql) return NULL; } if (strlen(sql_cstr) != (size_t)size) { - PyErr_SetString(connection->DataError, + PyErr_SetString(connection->ProgrammingError, "the query contains a null character"); return NULL; }