gh-79579: Improve DML query detection in sqlite3 #93623
Conversation
|
cc. @animalize |
|
If the |
Misc/NEWS.d/next/Library/2022-06-06-12-58-27.gh-issue-79579.e8rB-M.rst
Outdated
Show resolved
Hide resolved
erlend-aasland
added
the
|
If you want to schedule another build, you need to add the " |
bedevere-bot
removed
the
Please merge after a few days, maybe there is something not currently thought of.
Misc/NEWS.d/next/Library/2022-06-06-12-58-27.gh-issue-79579.e8rB-M.rst
Outdated
Show resolved
Hide resolved
- normalise switch cases - improve NEWS entry accuracy
| } | ||
|
|
||
| const char *p = lstrip_sql(sql_cstr); | ||
| if (p != NULL) { | ||
| is_dml = (PyOS_strnicmp(p, "insert", 6) == 0) | ||
| || (PyOS_strnicmp(p, "update", 6) == 0) | ||
| || (PyOS_strnicmp(p, "delete", 6) == 0) | ||
| || (PyOS_strnicmp(p, "replace", 7) == 0); |
There was a problem hiding this comment.
Note this remark in the sqlite3_changes docs:
auxiliary changes caused by triggers, foreign key actions or REPLACE constraint resolution are not counted.
This is a separate issue, out of scope for this PR.
|
Thanks for reviewing, Ma Lin. Highly appreciated 🙏🏻 The buildbot run for 5918fbe completed without failures. I'll let this PR sit around for some days to give Serhiy a chance to review. I'll merge sometime next week. |
|
As I said before, I'm not a deep user of SQL. So when in very complex situations, there may be things that I can't think of. |
|
|
||
| parse_remaining_sql_state state = NORMAL; | ||
|
|
||
| for (;;) { | ||
| switch (*pos) { | ||
| case 0: | ||
| return 0; | ||
| return NULL; |
There was a problem hiding this comment.
Note to self: this case makes the return NULL at the end of the function unreachable. This can be easily fixed with a tiny refactor, where case 0 is removed:
lstrip_sql(const char *sql)
{
parse_remaining_sql_state state = NORMAL;
for (const char *pos = sql; *pos; pos++) {
...
}
return NULL;I'll add that in a separate PR.
|
In current code, this code can't be processed correctly. -
- INSERT INTO test(income) VALUES(?)Maybe there are some corner cases that cannot be handled correctly as well. IMHO, SQLite's code is simple and robust: It uses a loop to skip comment, we can use this method instead of the state machine. |
Strip whitespace and comments from queries in order to harden DML query
detection.
Resolves #79579