-
Notifications
You must be signed in to change notification settings - Fork 1.5k
sqlite3: pass None for NULL authorizer args instead of crashing #8534
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
youknowone
merged 2 commits into
RustPython:main
from
ever0de:fix/sqlite-authorizer-null-args
Aug 15, 2026
+21
−11
Merged
Changes from all commits
Commits
Show all changes
2 commits
Select commit
Hold shift + click to select a range
File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -587,10 +587,10 @@ mod _sqlite3 { | |
| ) -> c_int { | ||
| let (callable, vm) = unsafe { (*data.cast::<Self>()).retrieve() }; | ||
| let f = || -> PyResult<c_int> { | ||
| let arg1 = ptr_to_str(arg1, vm)?; | ||
| let arg2 = ptr_to_str(arg2, vm)?; | ||
| let db_name = ptr_to_str(db_name, vm)?; | ||
| let access = ptr_to_str(access, vm)?; | ||
| let arg1 = ptr_to_str_or_none(arg1, vm)?; | ||
| let arg2 = ptr_to_str_or_none(arg2, vm)?; | ||
| let db_name = ptr_to_str_or_none(db_name, vm)?; | ||
| let access = ptr_to_str_or_none(access, vm)?; | ||
|
|
||
| let val = callable.call((action, arg1, arg2, db_name, access), vm)?; | ||
| let Some(val) = val.downcast_ref::<PyInt>() else { | ||
|
|
@@ -2798,12 +2798,14 @@ mod _sqlite3 { | |
| } | ||
| let sql_cstr = sql.to_cstring(vm)?; | ||
|
|
||
| let db = connection.db_lock(vm)?; | ||
|
|
||
| db.sql_limit(sql.byte_len(), vm)?; | ||
| let raw = { | ||
| let db = connection.db_lock(vm)?; | ||
| db.sql_limit(sql.byte_len(), vm)?; | ||
| **db | ||
| }; | ||
|
|
||
| let mut tail = null(); | ||
| let st = db.prepare(sql_cstr.as_ptr(), &mut tail, vm)?; | ||
| let st = raw.prepare(sql_cstr.as_ptr(), &mut tail, vm)?; | ||
|
|
||
| let Some(st) = st else { | ||
| return Ok(None); | ||
|
|
@@ -3480,7 +3482,17 @@ mod _sqlite3 { | |
| return Err(vm.new_memory_error("string pointer is null")); | ||
| } | ||
| unsafe { CStr::from_ptr(p).to_str() } | ||
| .map_err(|_| vm.new_value_error("Invalid UIF-8 codepoint")) | ||
| .map_err(|_| vm.new_value_error("Invalid UTF-8 codepoint")) | ||
|
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think this was a typo, so I fixed it. |
||
| } | ||
|
|
||
| fn ptr_to_str_or_none(p: *const libc::c_char, vm: &VirtualMachine) -> PyResult<PyObjectRef> { | ||
| if p.is_null() { | ||
| return Ok(vm.ctx.none()); | ||
| } | ||
| let s = unsafe { CStr::from_ptr(p) } | ||
| .to_str() | ||
| .map_err(|_| vm.new_value_error("Invalid UTF-8 codepoint".to_owned()))?; | ||
| Ok(vm.ctx.new_str(s).into()) | ||
| } | ||
|
|
||
| fn ptr_to_string( | ||
|
|
||
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🗄️ Data Integrity & Integration | 🟠 Major | 🏗️ Heavy lift
🧩 Analysis chain
🏁 Script executed:
Repository: RustPython/RustPython
Length of output: 159
🏁 Script executed:
Repository: RustPython/RustPython
Length of output: 290
🏁 Script executed:
Repository: RustPython/RustPython
Length of output: 299
🏁 Script executed:
Repository: RustPython/RustPython
Length of output: 159
🏁 Script executed:
Repository: RustPython/RustPython
Length of output: 39700
🏁 Script executed:
Repository: RustPython/RustPython
Length of output: 2603
🏁 Script executed:
Repository: RustPython/RustPython
Length of output: 41904
🏁 Script executed:
Repository: RustPython/RustPython
Length of output: 9974
🌐 Web query:
SQLite sqlite3_set_authorizer callback must not modify connection sqlite3_prepare_v2 connection modification documentation💡 Result:
The official SQLite documentation explicitly states that an authorizer callback must not perform any actions that modify the database connection that invoked the callback [1][2][3]. Specifically, the documentation notes that both sqlite3_prepare_v2 and sqlite3_step are considered operations that modify the database connection [1][4]. Because an authorizer callback is invoked during the compilation of a SQL statement (which occurs within sqlite3_prepare_v2 or its variants), attempting to call sqlite3_prepare_v2 or sqlite3_step from within that callback constitutes a prohibited modification of the connection state [1][5]. Doing so can lead to undefined behavior, as the internal state of the connection being prepared is already in a sensitive, intermediate phase [1]. Furthermore, if a statement requires re-preparation during sqlite3_step due to a schema change, the authorizer callback might be triggered again; therefore, the prohibition applies to these nested operations as well [1][4]. In summary, you must avoid any SQLite API calls within your authorizer callback that would result in calling functions like sqlite3_prepare_v2 or sqlite3_step on the same database connection [1][5].
Citations:
🏁 Script executed:
Repository: RustPython/RustPython
Length of output: 9933
Reject connection mutations during authorizer callbacks.
Statement::newcopies the rawsqlite3*, releasesconnection.db, and then callssqlite3_prepare_v2. The authorizer can re-enter a capturedConnectionduring this call and invokeclose,set_authorizer,setlimit, or another statement operation. SQLite forbids these mutations during authorizer callbacks. Reject or defer them, or restrict and document supported callback operations.🤖 Prompt for AI Agents
Source: MCP tools
🩺 Stability & Availability | 🟠 Major | 🏗️ Heavy lift
🧩 Analysis chain
🏁 Script executed:
Repository: RustPython/RustPython
Length of output: 159
🏁 Script executed:
Repository: RustPython/RustPython
Length of output: 5721
🏁 Script executed:
Repository: RustPython/RustPython
Length of output: 8917
🏁 Script executed:
Repository: RustPython/RustPython
Length of output: 13584
🏁 Script executed:
Repository: RustPython/RustPython
Length of output: 201
🏁 Script executed:
Repository: RustPython/RustPython
Length of output: 1063
🏁 Script executed:
Repository: RustPython/RustPython
Length of output: 43389
🏁 Script executed:
Repository: RustPython/RustPython
Length of output: 44637
🏁 Script executed:
Repository: RustPython/RustPython
Length of output: 9443
🌐 Web query:
SQLite sqlite3_close_v2 concurrent sqlite3_prepare_v2 connection lifetime active operation documentation💡 Result:
When managing SQLite database connections, the behavior of sqlite3_close_v2 differs significantly from the standard sqlite3_close function regarding active operations [1][2]. 1. Connection Lifetime and Active Operations: Ideally, all applications should finalize prepared statements (via sqlite3_finalize), close BLOB handles, and finish backup objects before attempting to close a database connection [1][2]. - The sqlite3_close function will return SQLITE_BUSY if any such resources remain unfinalized, leaving the database connection open [1][2]. - The sqlite3_close_v2 function is designed for environments (such as garbage-collected languages) where the order of destructor calls is arbitrary or difficult to control [1][2]. If called with active resources, it returns SQLITE_OK and marks the database connection as an unusable zombie [1][2]. The connection and its associated resources are then automatically deallocated only once all pending operations (prepared statements, BLOB handles, backups) are finalized or finished [1][2]. 2. Concurrency and Safety: - Multi-threading: In multi-threaded mode, SQLite requires that no single database connection or object derived from it (such as a prepared statement) be used in two or more threads simultaneously [3][4]. Serialized mode provides automatic mutex protection, but even then, it is generally considered unsafe to perform operations on a connection that is simultaneously being closed [3][4]. - Zombie State: Once sqlite3_close_v2 is called, the connection is effectively dead; you should not attempt to use it or its derived objects for further database operations, as the connection is strictly in a zombie state awaiting total cleanup [5][2]. - POSIX Considerations: On POSIX systems, calling close on a file descriptor while other file descriptors to the same file are open can lead to unintended side effects due to how POSIX advisory locks function [6]. While SQLite includes internal safeguards, it is best practice to avoid closing connections while other operations on the same database are still active within the process [6]. In summary, while sqlite3_close_v2 prevents the immediate failure returned by sqlite3_close when resources are still active, it does not absolve the developer of the responsibility to ensure all resources are eventually finalized [7][8]. Using a connection after it has been marked as a zombie, or attempting to close a connection while another thread is actively using it, is not supported and can lead to unpredictable behavior [5][3][4].
Citations:
🏁 Script executed:
Repository: RustPython/RustPython
Length of output: 50377
Keep the database owner alive during preparation.
rawcopies a pointer, then releases thedbguard beforesqlite3_prepare_v2runs.Connection::closecan drop the owner during this gap, especially whencheck_same_threadis disabled. Use an in-flight guard or retain the owner until preparation returns.🤖 Prompt for AI Agents
Sources: Coding guidelines, MCP tools