classification
Title: [sqlite3] Improve sqlite3_value_text() error handling
Type: behavior Stage: patch review
Components: Extension Modules Versions: Python 3.11, Python 3.10, Python 3.9
process
Status: open Resolution:
Dependencies: Superseder:
Assigned To: Nosy List: berker.peksag, erlendaasland, miss-islington, pablogsal, serhiy.storchaka
Priority: normal Keywords: patch

Created on 2021-04-15 10:00 by erlendaasland, last changed 2021-09-10 08:55 by erlendaasland.

Files
File name Uploaded Description Edit
patch.diff erlendaasland, 2021-04-15 10:00
Pull Requests
URL Status Linked Edit
PR 25422 merged erlendaasland, 2021-04-15 10:04
PR 26534 merged miss-islington, 2021-06-04 18:34
PR 27627 merged erlendaasland, 2021-08-06 12:39
PR 27642 open erlendaasland, 2021-08-06 19:43
PR 27922 merged erlendaasland, 2021-08-23 20:06
PR 27952 merged erlendaasland, 2021-08-25 19:13
PR 27953 merged erlendaasland, 2021-08-25 19:18
Messages (14)
msg391124 - (view) Author: Erlend E. Aasland (erlendaasland) * (Python triager) Date: 2021-04-15 10:00
Fix sqlite3_value_text() usage:
- Raise MemoryError if sqlite3_value_text() sets SQLITE_NOMEM
- Let PyUnicode_FromStringAndSize() errors propagate

Quoting the SQLite docs:
"As long as the input parameter is correct, these routines can only fail if an out-of-memory error occurs"


See also:
- bpo-43296
- https://sqlite.org/c3ref/value_blob.html
msg394267 - (view) Author: Erlend E. Aasland (erlendaasland) * (Python triager) Date: 2021-05-24 20:53
The affected branch is exercised by the following tests:
- test_aggr_check_param_blob
- test_aggr_check_param_float
- test_aggr_check_param_int
- test_aggr_check_param_none
- test_aggr_check_param_str
- test_aggr_check_params_int
- test_aggr_exception_in_finalize
- test_aggr_exception_in_step
- test_aggr_no_finalize
- test_param_string

I've expanded test_aggr_check_param_str and test_param_string to also check empty strings.
msg395109 - (view) Author: Pablo Galindo Salgado (pablogsal) * (Python committer) Date: 2021-06-04 18:34
New changeset 006fd869e4798b68e266f5de89c83ddb531a756b by Erlend Egeberg Aasland in branch 'main':
bpo-43853: Handle sqlite3_value_text() errors (GH-25422)
https://github.com/python/cpython/commit/006fd869e4798b68e266f5de89c83ddb531a756b
msg395112 - (view) Author: miss-islington (miss-islington) Date: 2021-06-04 18:54
New changeset 067d6d46575b5cf30bbf7c812defee1517106a34 by Miss Islington (bot) in branch '3.10':
bpo-43853: Handle sqlite3_value_text() errors (GH-25422)
https://github.com/python/cpython/commit/067d6d46575b5cf30bbf7c812defee1517106a34
msg399075 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2021-08-06 11:53
I think it is worth to backport it to 3.9.

It looks to me that it fixes also support of strings containing NUL. It would be nice to add corresponding tests for this. You can also add this in the NEWS entry for this issue.
msg399084 - (view) Author: Erlend E. Aasland (erlendaasland) * (Python triager) Date: 2021-08-06 12:58
I'll add extra tests in a separate PR, so we can easily backport it to 3.10 and 3.9.
msg399114 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2021-08-06 17:57
New changeset 8c07fef867707694c9f2fcee4d7a9563ad78ed14 by Erlend Egeberg Aasland in branch '3.9':
[3.9] bpo-43853: Handle sqlite3_value_text() errors (GH-25422). (GH-27627)
https://github.com/python/cpython/commit/8c07fef867707694c9f2fcee4d7a9563ad78ed14
msg399120 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2021-08-06 18:26
What about new tests and NEWS update?
msg400174 - (view) Author: Erlend E. Aasland (erlendaasland) * (Python triager) Date: 2021-08-23 20:09
> What about new tests and NEWS update?

See PR 27642 for the expanded tests, and PR 27922 for amending news entries. I didn't combine the two, as I figured you might have more comments on the former PR.
msg400288 - (view) Author: Pablo Galindo Salgado (pablogsal) * (Python committer) Date: 2021-08-25 19:03
New changeset 7903a1096343d8018e889029f025d39bdd077170 by Erlend Egeberg Aasland in branch 'main':
bpo-43853: Amend NEWS entry for latest changes in sqlite3 (GH-27922)
https://github.com/python/cpython/commit/7903a1096343d8018e889029f025d39bdd077170
msg400292 - (view) Author: Pablo Galindo Salgado (pablogsal) * (Python committer) Date: 2021-08-25 19:26
New changeset 0ec17a2494412ea7d76f7d0567b73c1fffa88c18 by Erlend Egeberg Aasland in branch '3.9':
[3.9] bpo-43853: Amend NEWS entry for latest changes in sqlite3 (GH-27922) (GH-27952)
https://github.com/python/cpython/commit/0ec17a2494412ea7d76f7d0567b73c1fffa88c18
msg400296 - (view) Author: Pablo Galindo Salgado (pablogsal) * (Python committer) Date: 2021-08-25 19:50
New changeset b34ca7e0515e6726cf5e938f7239de41df710cfd by Erlend Egeberg Aasland in branch '3.10':
[3.10] bpo-43853: Amend NEWS entry for latest changes in sqlite3 (GH-27922). (GH-27953)
https://github.com/python/cpython/commit/b34ca7e0515e6726cf5e938f7239de41df710cfd
msg400319 - (view) Author: Erlend E. Aasland (erlendaasland) * (Python triager) Date: 2021-08-26 07:08
Thanks for merging the NEWS amendments, Pablo. We can close this issue after landing PR 27642.
msg401565 - (view) Author: Erlend E. Aasland (erlendaasland) * (Python triager) Date: 2021-09-10 08:55
Serhiy, I believe I've addressed your review remarks on PR 27642. Would you mind taking a look at it again if you have time?
History
Date User Action Args
2021-09-10 08:55:22erlendaaslandsetmessages: + msg401565
2021-08-26 07:08:04erlendaaslandsetmessages: + msg400319
2021-08-25 19:50:28pablogsalsetmessages: + msg400296
2021-08-25 19:26:37pablogsalsetmessages: + msg400292
2021-08-25 19:18:39erlendaaslandsetpull_requests: + pull_request26400
2021-08-25 19:13:35erlendaaslandsetpull_requests: + pull_request26399
2021-08-25 19:03:15pablogsalsetmessages: + msg400288
2021-08-23 20:09:35erlendaaslandsetmessages: + msg400174
2021-08-23 20:07:16erlendaaslandsetmessages: - msg399133
2021-08-23 20:06:34erlendaaslandsetpull_requests: + pull_request26374
2021-08-06 19:43:18erlendaaslandsetstage: resolved -> patch review
pull_requests: + pull_request26136
2021-08-06 19:24:15erlendaaslandsetmessages: + msg399133
2021-08-06 18:28:27erlendaaslandsetstatus: closed -> open
resolution: fixed ->
2021-08-06 18:26:57serhiy.storchakasetmessages: + msg399120
2021-08-06 18:07:16erlendaaslandsetstatus: open -> closed
stage: patch review -> resolved
2021-08-06 17:57:46serhiy.storchakasetmessages: + msg399114
2021-08-06 12:58:27erlendaaslandsetmessages: + msg399084
2021-08-06 12:39:05erlendaaslandsetstage: resolved -> patch review
pull_requests: + pull_request26121
2021-08-06 11:53:34serhiy.storchakasetstatus: closed -> open

messages: + msg399075
versions: + Python 3.9, Python 3.11
2021-06-04 18:54:57miss-islingtonsetmessages: + msg395112
2021-06-04 18:39:12pablogsalsetstatus: open -> closed
resolution: fixed
stage: patch review -> resolved
2021-06-04 18:34:27miss-islingtonsetnosy: + miss-islington
pull_requests: + pull_request25125
2021-06-04 18:34:07pablogsalsetnosy: + pablogsal
messages: + msg395109
2021-05-24 20:53:17erlendaaslandsetmessages: + msg394267
2021-05-09 17:05:28erlendaaslandsettitle: [sqlite3] Fix sqlite3_value_text() usage -> [sqlite3] Improve sqlite3_value_text() error handling
2021-04-15 10:04:35erlendaaslandsettitle: [sqlite] Fix sqlite3_value_text() usage -> [sqlite3] Fix sqlite3_value_text() usage
2021-04-15 10:04:10erlendaaslandsetstage: patch review
pull_requests: + pull_request24154
2021-04-15 10:00:09erlendaaslandcreate