From aae7e760c636e292e92015e8fb6c9147a64f4c0b Mon Sep 17 00:00:00 2001 From: huymobile Date: Sat, 3 Oct 2026 02:23:37 +0700 Subject: [PATCH] fix: reject SQL containing more than one statement sqlite3_prepare_v2 compiles only the first statement and reports the rest through its tail pointer, which prepareStatement ignored. execute, executeAsync, batch commands, loadFile lines and prepare therefore ran the first statement and silently dropped the rest. Prepare the tail and throw when it holds another statement. Whitespace, comments and extra semicolons still pass. This matches the better-sqlite3 Node mock, which already rejects multi-statement SQL. Fixes #3 Co-Authored-By: Claude Opus 5.5 --- .github/ci-paths.json | 1 + docs/content/docs/guides/load-sql-file.mdx | 2 +- .../docs/guides/parameters-and-results.mdx | 2 +- .../unit/specs/operations/execute.spec.ts | 27 ++++++++ .../cpp/NitroSQLiteOperations.cpp | 9 ++- .../cpp/NitroSQLiteStatementTail.hpp | 22 ++++++ .../tests/cpp/statementTail.test.cpp | 68 +++++++++++++++++++ scripts/test-cpp.sh | 15 ++++ 8 files changed, 143 insertions(+), 3 deletions(-) create mode 100644 packages/react-native-nitro-sqlite/cpp/NitroSQLiteStatementTail.hpp create mode 100644 packages/react-native-nitro-sqlite/tests/cpp/statementTail.test.cpp diff --git a/.github/ci-paths.json b/.github/ci-paths.json index 9ce172e1..1adc4891 100644 --- a/.github/ci-paths.json +++ b/.github/ci-paths.json @@ -56,6 +56,7 @@ "packages/react-native-nitro-sqlite/cpp/NitroSQLiteExecuteBatch.*", "packages/react-native-nitro-sqlite/cpp/NitroSQLiteOperations.*", "packages/react-native-nitro-sqlite/cpp/NitroSQLiteStatementGroup.hpp", + "packages/react-native-nitro-sqlite/cpp/NitroSQLiteStatementTail.hpp", "packages/react-native-nitro-sqlite/cpp/sqlite/sqlite3.*", "packages/react-native-nitro-sqlite/cpp/sqlite/sqlite3-symbol-prefix.h", "packages/react-native-nitro-sqlite-vec/cpp/**", diff --git a/docs/content/docs/guides/load-sql-file.mdx b/docs/content/docs/guides/load-sql-file.mdx index f04534f6..8d83cbfa 100644 --- a/docs/content/docs/guides/load-sql-file.mdx +++ b/docs/content/docs/guides/load-sql-file.mdx @@ -16,7 +16,7 @@ console.log(result.commands, result.rowsAffected) ## SQL file format -The file format is deliberately simple. Each nonempty line is sent to SQLite as one statement. Do not split a statement across lines, put a comment on its own line, or rely on a general SQL dump parser. For example: +The file format is deliberately simple. Each nonempty line is sent to SQLite as one statement, and a line containing more than one statement fails the import. Do not split a statement across lines, put a comment on its own line, or rely on a general SQL dump parser. For example: ```sql CREATE TABLE IF NOT EXISTS tags (id INTEGER PRIMARY KEY, name TEXT NOT NULL); diff --git a/docs/content/docs/guides/parameters-and-results.mdx b/docs/content/docs/guides/parameters-and-results.mdx index 1e1d763f..a0c2885e 100644 --- a/docs/content/docs/guides/parameters-and-results.mdx +++ b/docs/content/docs/guides/parameters-and-results.mdx @@ -48,7 +48,7 @@ Be especially careful with nullable columns and expressions when specifying a ro ## Errors -The connection's JavaScript helpers normalize database failures to `NitroSQLiteError`. Async methods reject; sync methods throw. An empty SQL string or one containing only comments also fails with the native category `SqlExecutionError`, because SQLite produces no statement to execute. Catch the error around the operation you can recover from: +The connection's JavaScript helpers normalize database failures to `NitroSQLiteError`. Async methods reject; sync methods throw. An empty SQL string or one containing only comments also fails with the native category `SqlExecutionError`, because SQLite produces no statement to execute. Each query runs one statement, so SQL that contains another statement after the first fails with the same category instead of running only the first one; trailing whitespace, comments, and semicolons are allowed. Use [batch operations](/docs/guides/batch-operations) to run several statements. Catch the error around the operation you can recover from: ```ts import { NitroSQLiteError } from 'react-native-nitro-sqlite' diff --git a/example/tests/unit/specs/operations/execute.spec.ts b/example/tests/unit/specs/operations/execute.spec.ts index c7f56039..fa66bc59 100644 --- a/example/tests/unit/specs/operations/execute.spec.ts +++ b/example/tests/unit/specs/operations/execute.spec.ts @@ -104,6 +104,33 @@ export default function registerExecuteUnitTests() { ) }) + it('rejects a query that contains more than one statement', async () => { + const query = + 'CREATE TABLE MultiStatementFirst (id INTEGER); CREATE TABLE MultiStatementSecond (id INTEGER)' + + for (const run of [ + () => testDb.execute(query), + () => testDb.executeAsync(query), + () => testDb.prepare(query), + ]) { + try { + await run() + throw new Error('Expected a multi-statement query to fail') + } catch (error) { + if (!isNitroSQLiteError(error)) throw error + expect(error.message).toContain( + 'Query contains more than one SQL statement', + ) + } + } + + expect( + testDb.execute( + "SELECT name FROM sqlite_schema WHERE name LIKE 'MultiStatement%'", + ).results, + ).toEqual([]) + }) + it('materializes native query results once', () => { const sourceRows = [ { id: 1, nullable: null }, diff --git a/packages/react-native-nitro-sqlite/cpp/NitroSQLiteOperations.cpp b/packages/react-native-nitro-sqlite/cpp/NitroSQLiteOperations.cpp index f012640d..7a6cb75d 100644 --- a/packages/react-native-nitro-sqlite/cpp/NitroSQLiteOperations.cpp +++ b/packages/react-native-nitro-sqlite/cpp/NitroSQLiteOperations.cpp @@ -2,6 +2,7 @@ #include "NitroSQLiteException.hpp" #include "NitroSQLiteLogs.hpp" #include "NitroSQLiteStatementGroup.hpp" +#include "NitroSQLiteStatementTail.hpp" #include "NitroSQLiteUtils.hpp" #include "hybridObjects/HybridNitroSQLiteQueryResult.hpp" #include "sqlite/sqlite3.h" @@ -164,7 +165,8 @@ namespace { SQLiteStatement prepareStatement(sqlite3* db, const std::string& query, const std::optional& params) { sqlite3_stmt* rawStatement = nullptr; - int statementStatus = sqlite3_prepare_v2(db, query.c_str(), -1, &rawStatement, nullptr); + const char* tail = nullptr; + int statementStatus = sqlite3_prepare_v2(db, query.c_str(), -1, &rawStatement, &tail); SQLiteStatement statement(rawStatement); if (statementStatus != SQLITE_OK) { @@ -177,6 +179,11 @@ namespace { throw NitroSQLiteException::SqlExecution("Query does not contain any SQL statement"); } + // Only the first statement would run, so reject the query instead of silently skipping the rest. + if (hasTrailingStatement(db, tail)) { + throw NitroSQLiteException::SqlExecution("Query contains more than one SQL statement"); + } + if (params) { bindStatement(statement.get(), *params); } diff --git a/packages/react-native-nitro-sqlite/cpp/NitroSQLiteStatementTail.hpp b/packages/react-native-nitro-sqlite/cpp/NitroSQLiteStatementTail.hpp new file mode 100644 index 00000000..1a793b79 --- /dev/null +++ b/packages/react-native-nitro-sqlite/cpp/NitroSQLiteStatementTail.hpp @@ -0,0 +1,22 @@ +#pragma once + +#include + +namespace margelo::nitro::rnnitrosqlite { + +// sqlite3_prepare_v2 compiles only the first statement of a query and points its tail at the rest. +// Preparing that tail lets SQLite decide whether it holds more than whitespace, comments, or semicolons: +// those prepare to a null statement, while another statement prepares or fails to prepare. +// Shared with the host test so the check runs against the bundled SQLite. +inline bool hasTrailingStatement(sqlite3* db, const char* tail) { + if (tail == nullptr || *tail == '\0') { + return false; + } + + sqlite3_stmt* statement = nullptr; + const int status = sqlite3_prepare_v2(db, tail, -1, &statement, nullptr); + sqlite3_finalize(statement); + return status != SQLITE_OK || statement != nullptr; +} + +} // namespace margelo::nitro::rnnitrosqlite diff --git a/packages/react-native-nitro-sqlite/tests/cpp/statementTail.test.cpp b/packages/react-native-nitro-sqlite/tests/cpp/statementTail.test.cpp new file mode 100644 index 00000000..c7d13b49 --- /dev/null +++ b/packages/react-native-nitro-sqlite/tests/cpp/statementTail.test.cpp @@ -0,0 +1,68 @@ +#include "NitroSQLiteStatementTail.hpp" +#include +#include +#include +#include + +using margelo::nitro::rnnitrosqlite::hasTrailingStatement; + +namespace { + +void expect(bool condition, const std::string& message) { + if (!condition) { + throw std::runtime_error(message); + } +} + +bool queryHasTrailingStatement(sqlite3* db, const std::string& query) { + sqlite3_stmt* raw = nullptr; + const char* tail = nullptr; + if (sqlite3_prepare_v2(db, query.c_str(), -1, &raw, &tail) != SQLITE_OK) { + throw std::runtime_error(sqlite3_errmsg(db)); + } + std::unique_ptr statement(raw, sqlite3_finalize); + expect(statement != nullptr, "query should contain a statement: " + query); + return hasTrailingStatement(db, tail); +} + +} // namespace + +int main() { + sqlite3* raw = nullptr; + if (sqlite3_open(":memory:", &raw) != SQLITE_OK) { + return 1; + } + std::unique_ptr db(raw, sqlite3_close); + try { + for (const std::string query : { + "SELECT 1", + "SELECT 1;", + "SELECT 1;\n ", + "SELECT 1;;;", + "SELECT 1; -- trailing comment", + "SELECT 1; /* trailing comment */", + "SELECT 1; /* unterminated comment", + }) { + expect(!queryHasTrailingStatement(db.get(), query), "single statement was rejected: " + query); + } + + for (const std::string query : { + "CREATE TABLE foo (id INTEGER); CREATE TABLE bar (id INTEGER);", + "SELECT 1; SELECT 2", + "SELECT 1; -- comment\nSELECT 2", + // The second statement cannot prepare before the first one runs. + "CREATE TABLE later (id INTEGER); INSERT INTO later VALUES (1)", + "SELECT 1; not valid SQL", + }) { + expect(queryHasTrailingStatement(db.get(), query), "trailing statement was not detected: " + query); + } + + expect(!hasTrailingStatement(db.get(), nullptr), "a missing tail should not be rejected"); + expect(sqlite3_next_stmt(db.get(), nullptr) == nullptr, "tail check leaked a prepared statement"); + std::cout << "[PASS] detects SQL after the first statement" << '\n'; + return 0; + } catch (const std::exception& error) { + std::cerr << "[FAIL] " << error.what() << '\n'; + return 1; + } +} diff --git a/scripts/test-cpp.sh b/scripts/test-cpp.sh index 58afcb12..66476754 100644 --- a/scripts/test-cpp.sh +++ b/scripts/test-cpp.sh @@ -60,6 +60,21 @@ clang++ \ -o /tmp/statementGroupTests /tmp/statementGroupTests +clang++ \ + -std=c++20 \ + -Wall \ + -Wextra \ + -Werror \ + -Ipackages/react-native-nitro-sqlite/cpp \ + -Ipackages/react-native-nitro-sqlite/cpp/sqlite \ + packages/react-native-nitro-sqlite/tests/cpp/statementTail.test.cpp \ + /tmp/sqlite3.o \ + -ldl \ + -lm \ + -pthread \ + -o /tmp/statementTailTests +/tmp/statementTailTests + clang \ -std=c11 \ -DSQLITE_THREADSAFE=0 \