Repository navigation
Add a mode that refuses a query string with more than one statement - #12
Merged
Merged
Conversation
CovenantSQL checks each query before its miners run it, but SQLite splits a string into statements by its own lexical rules, so a second statement that the checker parses differently, for example inside what it takes for a comment, runs unchecked. With SQLiteDriver.SingleStatement set, prepare checks the rest of the string after the first statement byte by byte, following the tokenizer of the bundled SQLite, and refuses anything but whitespace, comments and semicolons with ErrMultipleStatements. The rest is never prepared on the connection, and the C fast path of Exec without arguments, which steps each statement before Go sees the rest, is not used. Exec, Query, Prepare, transactions and the statement cache all go through prepare. The default keeps running batches.
SQLite skips leading empty statements itself and stops after the first statement, so one check of the rest after the first prepare is enough; prepare no longer prepares the rest after an empty statement. The block comment scan searched the whole rest of the string for a NUL at every comment, which is quadratic in the number of comments. It now searches the comment only. The README no longer says nothing of a refused string runs: a first statement that acts when it is prepared still does. The tests check that Query and Prepare refuse the string themselves, and cover a line comment followed by a second statement, CR in a line comment, a semicolon in a string literal and a NUL.
The accepted strings were only run with an argument, so Exec without arguments, which would otherwise take the C fast path, was not tested with them. Run each also without arguments. Cancel the context during and after a call, and check that nothing beyond the first statement ran. Note in the README that unkeyed SQLiteDriver literals must switch to field names, as the struct has a new field.
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
CovenantSQL checks each query before its miners run it, but SQLite splits a query string into statements by its own lexical rules, so a second statement that the checker reads differently, for example as part of a comment, runs unchecked. Part of CovenantSQL/CovenantSQL#443.
With the new
SQLiteDriver.SingleStatementset,preparechecks the rest of the string after the first statement byte by byte, following the tokenizer of the bundled SQLite (whitespace,--and/* */comments,;, a UTF-8 BOM), and refuses anything else with the newErrMultipleStatements. The rest is never prepared on the connection, and the C fast path ofExecwithout arguments, which steps before Go sees the rest, is skipped. A first statement that acts when it is prepared, such asPRAGMA case_sensitive_like, still acts; the doc comment says so. The default mode is unchanged. UnkeyedSQLiteDriverliterals, such asSQLiteDriver{nil, nil}, must switch to field names.Testing
linux/amd64, Docker, 4 CPUs, Go 1.27.3 and 1.26.10, with the four tag sets of
.github/workflows/go.yamland CovenantSQL's miner tags:go test -race -count=20 -run '^TestSingleStatement' .passes. It runs refused and accepted strings, the accepted ones with and without arguments, through Exec, Query and Prepare with plain and cancellable contexts and through a transaction, with the statement cache on and off, and cancels a context during and after a call.Execfast path in this mode, preparing the rest of the string, letting block comments nest, reading a final/*as a comment, and starting a space token with\v.go test -race .passes under every tag set.