Skip to content

bpo-43853: Handle sqlite3_value_text() errors #25422

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.

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 3 commits into from
Jun 4, 2021

Conversation

erlend-aasland
Copy link
Contributor

@erlend-aasland erlend-aasland commented Apr 15, 2021

  • raise MemoryError if sqlite3_value_text() sets SQLITE_NOMEM
  • use sqlite3_value_bytes() to find string length; must be called after sqlite3_value_text()
  • replace PyUnicode_FromString with PyUnicode_FromStringAndSize() and let errors propagate

https://bugs.python.org/issue43853

@erlend-aasland erlend-aasland requested a review from pablogsal June 4, 2021 10:47
@erlend-aasland
Copy link
Contributor Author

@pablogsal, see top comment for a description of the changes.

Relevant SQLite docs: https://sqlite.org/c3ref/value_blob.html

  • "As long as the input parameter is correct, these routines can only fail if an out-of-memory error occurs during a format conversion."
  • "If an out-of-memory error occurs, then the return value from these routines is the same as if the column had contained an SQL NULL value. Valid SQL NULL returns can be distinguished from out-of-memory errors by invoking the sqlite3_errcode() immediately after the suspect return value is obtained and before any other SQLite interface is called on the same database connection."

We call the API's in the following order:

  1. sqlite3_value_type() (the switch statement in line 556)
  2. sqlite3_value_text() (case SQLITE_TEXT in line 563)
  3. sqlite3_errcode()
  4. sqlite3_value_bytes()

@pablogsal pablogsal merged commit 006fd86 into python:main Jun 4, 2021
@erlend-aasland erlend-aasland deleted the sqlite-value-text branch June 4, 2021 18:34
@pablogsal pablogsal added the needs backport to 3.10 only security fixes label Jun 4, 2021
@miss-islington
Copy link
Contributor

Thanks @erlend-aasland for the PR, and @pablogsal for merging it 🌮🎉.. I'm working now to backport this PR to: 3.10.
🐍🍒⛏🤖

@erlend-aasland
Copy link
Contributor Author

Thanks for reviewing, Pablo!

miss-islington pushed a commit to miss-islington/cpython that referenced this pull request Jun 4, 2021
(cherry picked from commit 006fd86)

Co-authored-by: Erlend Egeberg Aasland <erlend.aasland@innova.no>
@bedevere-bot
Copy link

GH-26534 is a backport of this pull request to the 3.10 branch.

@bedevere-bot bedevere-bot removed the needs backport to 3.10 only security fixes label Jun 4, 2021
miss-islington added a commit that referenced this pull request Jun 4, 2021
(cherry picked from commit 006fd86)

Co-authored-by: Erlend Egeberg Aasland <erlend.aasland@innova.no>
@serhiy-storchaka serhiy-storchaka added the needs backport to 3.9 only security fixes label Aug 6, 2021
@miss-islington
Copy link
Contributor

Thanks @erlend-aasland for the PR, and @pablogsal for merging it 🌮🎉.. I'm working now to backport this PR to: 3.9.
🐍🍒⛏🤖

@miss-islington
Copy link
Contributor

Sorry, @erlend-aasland and @pablogsal, I could not cleanly backport this to 3.9 due to a conflict.
Please backport using cherry_picker on command line.
cherry_picker 006fd869e4798b68e266f5de89c83ddb531a756b 3.9

@erlend-aasland
Copy link
Contributor Author

I'll fix the backport.

@bedevere-bot
Copy link

GH-27627 is a backport of this pull request to the 3.9 branch.

@bedevere-bot bedevere-bot removed the needs backport to 3.9 only security fixes label Aug 6, 2021
erlend-aasland pushed a commit to erlend-aasland/cpython that referenced this pull request Aug 6, 2021
(cherry picked from commit 006fd86)

Co-authored-by: Erlend Egeberg Aasland <erlend.aasland@innova.no>
serhiy-storchaka pushed a commit that referenced this pull request Aug 6, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants