kotschak/qelectrotech-source-mirror
0
0
Fork 0
mirror of https://github.com/qelectrotech/qelectrotech-source-mirror.git synced 2026-09-29 14:14:15 +02:00
Code Issues Actions Packages Projects Releases Wiki Activity

Fix #1045: crash on selecting an element with the online-installer Qt

Browse Source 
Since #983, projectDataBase::newQuery() checked a query with
sqlite3_prepare_v2() and sqlite3_stmt_readonly() on the handle of the
QSQLITE driver. Those calls go to the libsqlite3 QElectroTech links. The
QSQLITE plugin of the Qt online installer does not use that library: it
carries its own copy of SQLite, so the handle belongs to another library
and the call crashes. #1021 then put newQuery() on every element
selection, which is where #1045 hits it.

The check now runs the query with PRAGMA query_only set, through the
driver. SQLite refuses a write itself, before touching a row, so the CTE
prefix #983 closed ("WITH x AS (SELECT 1) DELETE FROM element") stays
closed. A refused or failed query comes back empty, because several
callers call exec() again on what newQuery() returns, after query_only
is off.

QElectroTech no longer calls the SQLite C API anywhere.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
This commit is contained in:
ispyisail
2026-09-26 19:20:47 +12:00
parent 923367d2a8
commit 45aec7735b
6 changed files with 197 additions and 184 deletions
Download Patch File Download Diff File Expand all files Collapse all files
tests/qttest/tst_sqlreadonly.cpp
+118 -51
View File
@@ -17,7 +17,7 @@
*/
/*
QETSql::isSingleReadOnlyStatement() -- the read-only enforcement every
QETSql::execReadOnly() -- the read-only enforcement every
project-database query goes through.
The case that matters most here is the CTE prefix. SQLite has allowed
@@ -27,6 +27,10 @@
<query> is stored in the .qet and executed on load, so the text can
arrive from a file rather than from the person at the keyboard.
The connection is a QSQLITE one, as in QElectroTech, so this runs
through whichever SQLite the Qt driver carries -- the point of #1045,
where the previous check crashed because it did not.
This test owns its own in-memory database and links nothing of
QElectroTech but sqlreadonly.cpp, so it stays a fast, hermetic check of
the security property itself.
@@ -35,8 +39,8 @@
#include "dataBase/sqlreadonly.h"
#include <QtTest>
#include <sqlite3.h>
#include <QSqlDatabase>
#include <QSqlQuery>
class TstSqlReadOnly : public QObject
{
@@ -55,58 +59,85 @@ class TstSqlReadOnly : public QObject
void refusesTrailingStatement();
void refusesEmptyAndCommentOnly_data();
void refusesEmptyAndCommentOnly();
void refusesWithoutAConnection();
void reportsAReason();
void doesNotExecuteWhatItRefuses();
void refusedQueryCannotBeRunAgain();
void acceptedQueryRunAgainStillReads();
void leavesTheConnectionWritable();
private:
sqlite3 *m_db = nullptr;
QSqlDatabase m_db;
int rowCount();
bool isAccepted(const QString &query, QString *error = nullptr);
};
int TstSqlReadOnly::rowCount()
{
sqlite3_stmt *st = nullptr;
sqlite3_prepare_v2(m_db, "SELECT COUNT(*) FROM element", -1, &st, nullptr);
sqlite3_step(st);
const int n = sqlite3_column_int(st, 0);
sqlite3_finalize(st);
return n;
QSqlQuery q(m_db);
q.exec(QStringLiteral("SELECT COUNT(*) FROM element"));
q.next();
return q.value(0).toInt();
}
bool TstSqlReadOnly::isAccepted(const QString &query, QString *error)
{
QString reason;
const QSqlQuery q = QETSql::execReadOnly(m_db, query, &reason);
if (error) {
*error = reason;
}
// Accepted means both: no reason given, and a query that actually ran.
// A refusal must be both too, or a caller could act on either half.
const bool accepted = reason.isEmpty();
if (accepted != q.isActive()) {
qWarning() << "reason and query state disagree for" << query
<< reason << q.isActive();
return !accepted; // fails whichever way the test expected
}
return accepted;
}
void TstSqlReadOnly::initTestCase()
{
QCOMPARE(sqlite3_open(":memory:", &m_db), SQLITE_OK);
QCOMPARE(sqlite3_exec(m_db,
"CREATE TABLE element (uuid TEXT);"
"INSERT INTO element VALUES ('a'),('b');", nullptr, nullptr, nullptr),
SQLITE_OK);
m_db = QSqlDatabase::addDatabase(QStringLiteral("QSQLITE"),
QStringLiteral("tst_sqlreadonly"));
QVERIFY(m_db.open());
QSqlQuery q(m_db);
QVERIFY(q.exec(QStringLiteral("CREATE TABLE element (uuid TEXT)")));
QVERIFY(q.exec(QStringLiteral("INSERT INTO element VALUES ('a'),('b')")));
QCOMPARE(rowCount(), 2);
}
void TstSqlReadOnly::cleanupTestCase()
{
sqlite3_close(m_db);
m_db = nullptr;
m_db.close();
m_db = QSqlDatabase();
QSqlDatabase::removeDatabase(QStringLiteral("tst_sqlreadonly"));
}
void TstSqlReadOnly::acceptsOrdinaryReads()
{
QVERIFY(QETSql::isSingleReadOnlyStatement(m_db, "SELECT * FROM element"));
QVERIFY(QETSql::isSingleReadOnlyStatement(m_db, "SELECT uuid FROM element WHERE uuid = 'a'"));
// A semicolon inside a string literal is not a second statement. The
// textual check this replaced rejected exactly this.
QVERIFY(QETSql::isSingleReadOnlyStatement(m_db, "SELECT ';' AS semicolon"));
QVERIFY(isAccepted("SELECT * FROM element"));
QVERIFY(isAccepted("SELECT uuid FROM element WHERE uuid = 'a'"));
// A semicolon inside a string literal is not a second statement.
QVERIFY(isAccepted("SELECT ';' AS semicolon"));
// One trailing semicolon is ordinary punctuation, not a second statement.
QVERIFY(QETSql::isSingleReadOnlyStatement(m_db, "SELECT * FROM element;"));
QVERIFY(isAccepted("SELECT * FROM element;"));
// And the rows come back: the returned query is the one that ran.
QSqlQuery q = QETSql::execReadOnly(m_db, "SELECT uuid FROM element ORDER BY uuid");
QStringList uuids;
while (q.next()) {
uuids << q.value(0).toString();
}
QCOMPARE(uuids, QStringList({"a", "b"}));
}
void TstSqlReadOnly::acceptsLegitimateCommonTableExpression()
{
// WITH must keep working -- the fix is not "ban CTEs".
QVERIFY(QETSql::isSingleReadOnlyStatement(m_db,
"WITH x AS (SELECT 1 AS n) SELECT n FROM x"));
QVERIFY(QETSql::isSingleReadOnlyStatement(m_db,
QVERIFY(isAccepted("WITH x AS (SELECT 1 AS n) SELECT n FROM x"));
QVERIFY(isAccepted(
"WITH RECURSIVE c(n) AS (SELECT 1 UNION ALL SELECT n+1 FROM c) "
"SELECT n FROM c LIMIT 3"));
}
@@ -117,13 +148,15 @@ void TstSqlReadOnly::refusesCtePrefixedWrites_data()
QTest::newRow("delete") << "WITH x AS (SELECT 1) DELETE FROM element";
QTest::newRow("update") << "WITH x AS (SELECT 1) UPDATE element SET uuid = 'pwned'";
QTest::newRow("insert") << "WITH x AS (SELECT 1) INSERT INTO element VALUES ('injected')";
QTest::newRow("returning") << "WITH x AS (SELECT 1) DELETE FROM element RETURNING uuid";
}
void TstSqlReadOnly::refusesCtePrefixedWrites()
{
QFETCH(QString, query);
QVERIFY2(!QETSql::isSingleReadOnlyStatement(m_db, query),
QVERIFY2(!isAccepted(query),
qPrintable(QStringLiteral("accepted a write: %1").arg(query)));
QCOMPARE(rowCount(), 2);
}
void TstSqlReadOnly::refusesBareWrites_data()
@@ -133,18 +166,23 @@ void TstSqlReadOnly::refusesBareWrites_data()
QTest::newRow("update") << "UPDATE element SET uuid = 'pwned'";
QTest::newRow("insert") << "INSERT INTO element VALUES ('injected')";
QTest::newRow("drop") << "DROP TABLE element";
QTest::newRow("create") << "CREATE TABLE injected (x)";
QTest::newRow("temp") << "CREATE TEMP TABLE injected (x)";
}
void TstSqlReadOnly::refusesBareWrites()
{
QFETCH(QString, query);
QVERIFY(!QETSql::isSingleReadOnlyStatement(m_db, query));
QVERIFY2(!isAccepted(query),
qPrintable(QStringLiteral("accepted a write: %1").arg(query)));
QCOMPARE(rowCount(), 2);
}
void TstSqlReadOnly::refusesTrailingStatement()
{
QVERIFY(!QETSql::isSingleReadOnlyStatement(m_db, "SELECT 1; DROP TABLE element"));
QVERIFY(!QETSql::isSingleReadOnlyStatement(m_db, "SELECT 1; SELECT 2"));
QVERIFY(!isAccepted("SELECT 1; DROP TABLE element"));
QVERIFY(!isAccepted("SELECT 1; SELECT 2"));
QCOMPARE(rowCount(), 2);
}
void TstSqlReadOnly::refusesEmptyAndCommentOnly_data()
@@ -157,42 +195,71 @@ void TstSqlReadOnly::refusesEmptyAndCommentOnly_data()
void TstSqlReadOnly::refusesEmptyAndCommentOnly()
{
// sqlite3_prepare_v2() reports success and a null statement for these;
// sqlite3_stmt_readonly() must never be handed that.
QFETCH(QString, query);
QVERIFY(!QETSql::isSingleReadOnlyStatement(m_db, query));
}
void TstSqlReadOnly::refusesWithoutAConnection()
{
// Fails closed: with no connection there is nothing to ask, and
// guessing from the text is the weakness this replaced.
QVERIFY(!QETSql::isSingleReadOnlyStatement(nullptr, "SELECT * FROM element"));
QVERIFY(!isAccepted(query));
}
void TstSqlReadOnly::reportsAReason()
{
QString reason;
QVERIFY(!QETSql::isSingleReadOnlyStatement(
m_db, "WITH x AS (SELECT 1) DELETE FROM element", &reason));
QVERIFY(!isAccepted("WITH x AS (SELECT 1) DELETE FROM element", &reason));
QVERIFY2(!reason.isEmpty(), "a refusal must say why");
reason = QStringLiteral("stale");
QVERIFY(QETSql::isSingleReadOnlyStatement(m_db, "SELECT * FROM element", &reason));
QVERIFY(isAccepted("SELECT * FROM element", &reason));
QVERIFY2(reason.isEmpty(), "an accepted query must not leave a reason behind");
}
void TstSqlReadOnly::doesNotExecuteWhatItRefuses()
{
// The check compiles the statement to inspect it. Proving the table is
// untouched afterwards is what says it compiled without running it --
// and this same assertion goes red if the refusals above ever stop
// refusing, since then the caller would run the DELETE for real.
// Proving the table is untouched afterwards is what says the write was
// refused rather than run -- and this same assertion goes red if the
// refusals above ever stop refusing.
QCOMPARE(rowCount(), 2);
QVERIFY(!QETSql::isSingleReadOnlyStatement(m_db, "WITH x AS (SELECT 1) DELETE FROM element"));
QVERIFY(!QETSql::isSingleReadOnlyStatement(m_db, "DELETE FROM element"));
QVERIFY(!isAccepted("WITH x AS (SELECT 1) DELETE FROM element"));
QVERIFY(!isAccepted("DELETE FROM element"));
QCOMPARE(rowCount(), 2);
}
QTEST_APPLESS_MAIN(TstSqlReadOnly)
void TstSqlReadOnly::refusedQueryCannotBeRunAgain()
{
// Several callers of projectDataBase::newQuery() call exec() again on
// what it returns, and query_only is off by then. A refused query
// must therefore come back with nothing left to run.
QSqlQuery q = QETSql::execReadOnly(m_db, "WITH x AS (SELECT 1) DELETE FROM element");
QVERIFY(!q.exec());
QCOMPARE(rowCount(), 2);
}
void TstSqlReadOnly::acceptedQueryRunAgainStillReads()
{
QSqlQuery q = QETSql::execReadOnly(m_db, "SELECT uuid FROM element");
QVERIFY(q.exec());
int n = 0;
while (q.next()) {
++n;
}
QCOMPARE(n, 2);
}
void TstSqlReadOnly::leavesTheConnectionWritable()
{
// query_only must not outlive the call, whatever its outcome: the
// project database is rebuilt by writes on this same connection.
QETSql::execReadOnly(m_db, "SELECT * FROM element");
QETSql::execReadOnly(m_db, "DELETE FROM element");
QETSql::execReadOnly(m_db, "not even sql");
QSqlQuery q(m_db);
QVERIFY(q.exec(QStringLiteral("PRAGMA query_only")));
QVERIFY(q.next());
QCOMPARE(q.value(0).toInt(), 0);
QVERIFY(q.exec(QStringLiteral("INSERT INTO element VALUES ('c')")));
QCOMPARE(rowCount(), 3);
QVERIFY(q.exec(QStringLiteral("DELETE FROM element WHERE uuid = 'c'")));
QCOMPARE(rowCount(), 2);
}
QTEST_GUILESS_MAIN(TstSqlReadOnly)
#include "tst_sqlreadonly.moc"