sqlite: reject statement-less SQLTagStore queries - #65152
Conversation
Sorry, something went wrong.
Signed-off-by: Lazizbek Ergashev <lazerg2@gmail.com>
|
Review requested:
|
Sorry, something went wrong.
Codecov Report✅ All modified and coverable lines are covered by tests. @@ Coverage Diff @@
## main #65152 +/- ##
=======================================
Coverage 90.32% 90.32%
=======================================
Files 759 759
Lines 248325 248345 +20
Branches 46861 46875 +14
=======================================
+ Hits 224303 224322 +19
+ Misses 15467 15462 -5
- Partials 8555 8561 +6
... and 31 files with indirect coverage changes 🚀 New features to boost your workflow:
|
Sorry, something went wrong.
Renegade334
left a comment
There was a problem hiding this comment.
#65157 is the correct approach here, there's no point in allowing a null statement to be constructed and added to the tag store cache in the first place. We should reject these at the point of preparation.
That being said, this should also be the case for db.prepare()
Sorry, something went wrong.
Signed-off-by: Lazizbek Ergashev <lazerg2@gmail.com>
|
Closing in favour of #65157. I would recommend not opening PRs within seconds of an issue being opened. There is often discussion to take place, even if the issue seems straightforward as in this case. |
Sorry, something went wrong.
Fixes: #65149
A query that holds no statement, such as sql.run`-- comment`, segfaults every SQLTagStore method. sqlite3_prepare_v2() returns SQLITE_OK with a null sqlite3_stmt* for input that is only a comment or whitespace, and ResetAndBindStatement() passes that null pointer straight to sqlite3_clear_bindings().
StatementSync already models a null statement as finalized, so this checks for it and throws ERR_INVALID_STATE, the same error db.prepare('-- comment').run() gives today.