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
gh-79579: Improve DML query detection in sqlite3 #93623
Open
erlend-aasland
wants to merge
14
commits into
python:main
Choose a base branch
from
erlend-aasland:sqlite-rowcount/gh-79579-alt
base: main
Could not load branches
Branch not found: {{ refName }}
Could not load tags
Nothing to show
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
+49
−26
Open
Changes from all commits
Commits
Show all changes
14 commits
Select commit
Hold shift + click to select a range
c8bcf3b
Add tests
erlend-aasland d38e632
Modify pysqlite_check_remaining_sql to return char * iso. int
erlend-aasland 7fcca68
Rename pysqlite_check_remaining_sql to lstrip_sql
erlend-aasland 3061dc5
Use lstrip_sql() to strip comments and whitespace
erlend-aasland 631f24d
Adjust comment
erlend-aasland 8f29283
Explicit comparison
erlend-aasland a51cfe6
Add more tests
erlend-aasland bf0e150
Fix bug in lstrip parser
erlend-aasland 5918fbe
Adjust NEWS
erlend-aasland b45e5b5
Make test more messy
erlend-aasland b0a7fbc
Address Ma Lin's review
erlend-aasland 61a81eb
Reword comment
erlend-aasland e3fea31
Inline it
erlend-aasland d6ce75c
Further mess up messy test to increase code coverage
erlend-aasland File filter
Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
There are no files selected for viewing
This file contains 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 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
| @@ -0,0 +1,2 @@ | ||
| :mod:`sqlite3` now correctly detects DML queries with leading comments. | ||
| Patch by Erlend E. Aasland. |
This file contains 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
| @@ -26,7 +26,7 @@ | ||
| #include "util.h" | ||
|
|
||
| /* prototypes */ | ||
| static int pysqlite_check_remaining_sql(const char* tail); | ||
| static const char *lstrip_sql(const char *sql); | ||
|
|
||
| typedef enum { | ||
| LINECOMMENT_1, | ||
| @@ -73,7 +73,7 @@ pysqlite_statement_create(pysqlite_Connection *connection, PyObject *sql) | ||
| return NULL; | ||
| } | ||
|
|
||
| if (pysqlite_check_remaining_sql(tail)) { | ||
| if (lstrip_sql(tail) != NULL) { | ||
| PyErr_SetString(connection->ProgrammingError, | ||
| "You can only execute one statement at a time."); | ||
| goto error; | ||
| @@ -82,20 +82,12 @@ pysqlite_statement_create(pysqlite_Connection *connection, PyObject *sql) | ||
| /* Determine if the statement is a DML statement. | ||
| SELECT is the only exception. See #9924. */ | ||
| int is_dml = 0; | ||
| for (const char *p = sql_cstr; *p != 0; p++) { | ||
| switch (*p) { | ||
| case ' ': | ||
| case '\r': | ||
| case '\n': | ||
| case '\t': | ||
| continue; | ||
| } | ||
|
|
||
| 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); | ||
| break; | ||
| } | ||
|
|
||
| pysqlite_Statement *self = PyObject_GC_New(pysqlite_Statement, | ||
| @@ -139,23 +131,25 @@ stmt_traverse(pysqlite_Statement *self, visitproc visit, void *arg) | ||
| } | ||
|
|
||
| /* | ||
| * Checks if there is anything left in an SQL string after SQLite compiled it. | ||
| * This is used to check if somebody tried to execute more than one SQL command | ||
| * with one execute()/executemany() command, which the DB-API and we don't | ||
| * allow. | ||
| * Strip leading whitespace and comments from SQL string and return a | ||
| * pointer to the first non-whitespace, non-comment character. | ||
| * | ||
| * This is used to check if somebody tries to execute more than one SQL query | ||
| * with one execute()/executemany() command, which the DB-API don't allow. | ||
| * | ||
| * Returns 1 if there is more left than should be. 0 if ok. | ||
| * It is also used to harden DML query detection. | ||
| */ | ||
| static int pysqlite_check_remaining_sql(const char* tail) | ||
| static inline const char * | ||
| lstrip_sql(const char *sql) | ||
| { | ||
| const char* pos = tail; | ||
| const char *pos = sql; | ||
|
|
||
| parse_remaining_sql_state state = NORMAL; | ||
|
|
||
| for (;;) { | ||
| switch (*pos) { | ||
| case 0: | ||
| return 0; | ||
| return NULL; | ||
|
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. Note to self: this 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. |
||
| case '-': | ||
| if (state == NORMAL) { | ||
| state = LINECOMMENT_1; | ||
| @@ -165,9 +159,12 @@ static int pysqlite_check_remaining_sql(const char* tail) | ||
| break; | ||
| case ' ': | ||
| case '\t': | ||
| if (state == COMMENTEND_1) { | ||
| state = IN_COMMENT; | ||
| } | ||
| break; | ||
| case '\n': | ||
| case 13: | ||
| case '\r': | ||
| if (state == IN_LINECOMMENT) { | ||
| state = NORMAL; | ||
| } | ||
| @@ -178,14 +175,14 @@ static int pysqlite_check_remaining_sql(const char* tail) | ||
| } else if (state == COMMENTEND_1) { | ||
| state = NORMAL; | ||
| } else if (state == COMMENTSTART_1) { | ||
| return 1; | ||
| return pos; | ||
| } | ||
| break; | ||
| case '*': | ||
| if (state == NORMAL) { | ||
| return 1; | ||
| return pos; | ||
| } else if (state == LINECOMMENT_1) { | ||
| return 1; | ||
| return pos; | ||
| } else if (state == COMMENTSTART_1) { | ||
| state = IN_COMMENT; | ||
| } else if (state == IN_COMMENT) { | ||
| @@ -198,14 +195,14 @@ static int pysqlite_check_remaining_sql(const char* tail) | ||
| } else if (state == IN_LINECOMMENT) { | ||
| } else if (state == IN_COMMENT) { | ||
| } else { | ||
| return 1; | ||
| return pos; | ||
| } | ||
| } | ||
|
|
||
| pos++; | ||
| } | ||
|
|
||
| return 0; | ||
| return NULL; | ||
| } | ||
|
|
||
| static PyType_Slot stmt_slots[] = { | ||
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.


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.
Note this remark in the
sqlite3_changesdocs:This is a separate issue, out of scope for this PR.