From 49efea26a5fae8c2275999c36c7c8d24cf4125de Mon Sep 17 00:00:00 2001
From: Florian Bruhin <me@the-compiler.org>
Date: Wed, 12 Sep 2018 10:04:39 +0200
Subject: sqlite: Fix QSqlError handling when opening/closing database

Both sqlite3_open_v2 and sqlite3_close are documented to return an error code:
https://www.sqlite.org/c3ref/open.html https://sqlite.org/c3ref/close.html

However, those were ignored (other than checking whether the operation
succeeded), causing QSqlError::nativeErrorCode() to always be "-1" when there
was an error while opening/closing the database.

Additionally, the error string needs to be read (via sqlite3_errmsg16) in
qMakeError *before* d->access is set to 0, or the databaseText() will always be
"out of memory" no matter what error actually happened.

Task-number: QTBUG-70506
Change-Id: I75cbf178c9711442e640afd26c4502214d20c598
Reviewed-by: Andy Shaw <andy.shaw@qt.io>
Reviewed-by: Simon Hausmann <simon.hausmann@qt.io>
Reviewed-by: Edward Welbourne <edward.welbourne@qt.io>
---
 src/plugins/sqldrivers/sqlite/qsql_sqlite.cpp         | 19 ++++++++++++-------
 .../auto/sql/kernel/qsqldatabase/tst_qsqldatabase.cpp | 18 ++++++++++++++++++
 2 files changed, 30 insertions(+), 7 deletions(-)

diff --git a/src/plugins/sqldrivers/sqlite/qsql_sqlite.cpp b/src/plugins/sqldrivers/sqlite/qsql_sqlite.cpp
index 2a770d0245..491d903137 100644
--- a/src/plugins/sqldrivers/sqlite/qsql_sqlite.cpp
+++ b/src/plugins/sqldrivers/sqlite/qsql_sqlite.cpp
@@ -109,7 +109,7 @@ static QVariant::Type qGetColumnType(const QString &tpName)
 }
 
 static QSqlError qMakeError(sqlite3 *access, const QString &descr, QSqlError::ErrorType type,
-                            int errorCode = -1)
+                            int errorCode)
 {
     return QSqlError(descr,
                      QString(reinterpret_cast<const QChar *>(sqlite3_errmsg16(access))),
@@ -803,7 +803,9 @@ bool QSQLiteDriver::open(const QString & db, const QString &, const QString &, c
 
     openMode |= SQLITE_OPEN_NOMUTEX;
 
-    if (sqlite3_open_v2(db.toUtf8().constData(), &d->access, openMode, NULL) == SQLITE_OK) {
+    const int res = sqlite3_open_v2(db.toUtf8().constData(), &d->access, openMode, NULL);
+
+    if (res == SQLITE_OK) {
         sqlite3_busy_timeout(d->access, timeOut);
         setOpen(true);
         setOpenError(false);
@@ -816,14 +818,15 @@ bool QSQLiteDriver::open(const QString & db, const QString &, const QString &, c
 #endif
         return true;
     } else {
+        setLastError(qMakeError(d->access, tr("Error opening database"),
+                     QSqlError::ConnectionError, res));
+        setOpenError(true);
+
         if (d->access) {
             sqlite3_close(d->access);
             d->access = 0;
         }
 
-        setLastError(qMakeError(d->access, tr("Error opening database"),
-                     QSqlError::ConnectionError));
-        setOpenError(true);
         return false;
     }
 }
@@ -840,8 +843,10 @@ void QSQLiteDriver::close()
             sqlite3_update_hook(d->access, NULL, NULL);
         }
 
-        if (sqlite3_close(d->access) != SQLITE_OK)
-            setLastError(qMakeError(d->access, tr("Error closing database"), QSqlError::ConnectionError));
+        const int res = sqlite3_close(d->access);
+
+        if (res != SQLITE_OK)
+            setLastError(qMakeError(d->access, tr("Error closing database"), QSqlError::ConnectionError, res));
         d->access = 0;
         setOpen(false);
         setOpenError(false);
diff --git a/tests/auto/sql/kernel/qsqldatabase/tst_qsqldatabase.cpp b/tests/auto/sql/kernel/qsqldatabase/tst_qsqldatabase.cpp
index 1f055e9c33..8cf43e243b 100644
--- a/tests/auto/sql/kernel/qsqldatabase/tst_qsqldatabase.cpp
+++ b/tests/auto/sql/kernel/qsqldatabase/tst_qsqldatabase.cpp
@@ -197,6 +197,8 @@ private slots:
     void sqlite_enableRegexp_data() { generic_data("QSQLITE"); }
     void sqlite_enableRegexp();
 
+    void sqlite_openError();
+
 private:
     void createTestTables(QSqlDatabase db);
     void dropTestTables(QSqlDatabase db);
@@ -2332,6 +2334,22 @@ void tst_QSqlDatabase::sqlite_enableRegexp()
     QFAIL_SQL(q, next());
 }
 
+void tst_QSqlDatabase::sqlite_openError()
+{
+    // see QTBUG-70506
+    if (!QSqlDatabase::drivers().contains("QSQLITE"))
+        QSKIP("Database driver QSQLITE not available");
+
+    QSqlDatabase db = QSqlDatabase::addDatabase("QSQLITE", "sqlite_openError");
+    db.setDatabaseName("/doesnotexist/foo.sqlite");
+    QVERIFY(db.isValid());
+
+    QVERIFY(!db.open());
+    QSqlError error = db.lastError();
+    QCOMPARE(error.nativeErrorCode(), "14");  // SQLITE_CANTOPEN
+    QCOMPARE(error.databaseText(), "unable to open database file");
+}
+
 void tst_QSqlDatabase::cloneDatabase()
 {
     QFETCH(QString, dbName);
-- 
cgit v1.2.3