Skip to content

bpo-43296: Handle sqlite3_value_blob() errors #24674

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 5 commits into from
Apr 14, 2021
Merged

bpo-43296: Handle sqlite3_value_blob() errors #24674

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
51f1886
bpo-43296: sqlite3_value_bytes() must come after type conversion
Feb 22, 2021
f1eceaf
Add empty blob test
Feb 28, 2021
4900d9f
Handle sqlite3_value_blob() errors
Feb 22, 2021
edf5a5a
Add NEWS
Mar 1, 2021
32fcc41
Simplify
Mar 2, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions Lib/sqlite3/test/userfunctions.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -276,6 +276,10 @@ def test_any_arguments(self):
val = cur.fetchone()[0]
self.assertEqual(val, 2)

def test_empty_blob(self):
cur = self.con.execute("select isblob(x'')")
self.assertTrue(cur.fetchone()[0])

# Regarding deterministic functions:
#
# Between 3.8.3 and 3.15.0, deterministic functions were only used to
Expand Down
3 changes: 3 additions & 0 deletions Misc/NEWS.d/next/Library/2021-03-01-13-01-33.bpo-43296.p_gU6T.rst
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
Improve :mod:`sqlite3` error handling: ``sqlite3_value_blob()`` errors that
set ``SQLITE_NOMEM`` now raise :exc:`MemoryError`. Patch by Erlend E.
Aasland.
24 changes: 17 additions & 7 deletions Modules/_sqlite/connection.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -546,7 +546,6 @@ _pysqlite_build_py_params(sqlite3_context *context, int argc,
sqlite3_value* cur_value;
PyObject* cur_py_value;
const char* val_str;
Py_ssize_t buflen;

args = PyTuple_New(argc);
if (!args) {
Expand All @@ -571,26 +570,37 @@ _pysqlite_build_py_params(sqlite3_context *context, int argc,
cur_py_value = Py_NewRef(Py_None);
}
break;
case SQLITE_BLOB:
buflen = sqlite3_value_bytes(cur_value);
cur_py_value = PyBytes_FromStringAndSize(
sqlite3_value_blob(cur_value), buflen);
case SQLITE_BLOB: {
sqlite3 *db = sqlite3_context_db_handle(context);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe it may make sense to see what other core devs think about declaring new variables inside case.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Well, there's plenty of other such cases in the code base: grep -A1 -rE "\<case\>.*:.*{" Modules

const void *blob = sqlite3_value_blob(cur_value);

if (blob == NULL && sqlite3_errcode(db) == SQLITE_NOMEM) {
PyErr_NoMemory();
goto error;
}

Py_ssize_t size = sqlite3_value_bytes(cur_value);
cur_py_value = PyBytes_FromStringAndSize(blob, size);
break;
}
case SQLITE_NULL:
default:
cur_py_value = Py_NewRef(Py_None);
}

if (!cur_py_value) {
Py_DECREF(args);
return NULL;
goto error;
}

PyTuple_SetItem(args, i, cur_py_value);

}

return args;

error:
Py_DECREF(args);
return NULL;
}

static void
Expand Down