SQLite: Handle identifiers correctly

This change can be described in the following 2 categories:
1. Support 3 ways to escape identifiers mentioned in SQLite Keywords
   In SQLite Keywords (https://sqlite.org/lang_keywords.html), it shows
   that there are 3 ways to escape identifiers, i.e., "", [], ``. So, I
   have overridden "bool isIdentifierEscaped(const QString &,
   IdentifierType)" to support it. In addition, there was a bug of
   _q_escapeIdentifier. If there is a field name called length [cm],
   which uses square brackets to show units, _q_escapeIdentifier will
   not escape it to "length [cm]".
2. Identify identifiers correctly if identifiers have been escaped
   There is a bug of QSQLiteDriver::record and
   QSQLiteDriver::primaryIndex.
   If we input escaped identifiers with separator, let's say
   "databaseName"."tableName", both will change the input into
   databaseName"."tableName, which is incorrect and causes
   qGetTableInfo cannot get the right results. In addition, I overrode
   stripDelimiters to strip "databaseName"."tableName" correctly.

There are still some assumptions for isIdentifierEscaped,
escapeIdentifier, and stripDelimiters, but I think this change it better
than what we have now.
1. For isIdentifierEscaped, if identifiers have a dot and the dot is a
   separator, it is the users' responsibility to escape the pair of
   schema and table name correctly. For example,
   "aSchemaName"."aTableName", not "aSchemaName".a"TableName". That's
   because we don't know whether the dot is just a dot of the name or a
   separator.
2. For escapeIdentifier, if identifiers have a dot and the parts before
   and after the dot are not escaped, escapeIdentifier will treat the
   dot as part of the table name or field name. The same as the item
   above, it is users' responsibility to do it right.
3. For stripDelimiters, the same as above, it is users' responsibility
   to do escape if users want to use format schemaName.tableName or
   tableName.fieldName.

Change-Id: I9d036a2a96180f8542436188f75a220a0fe58257
Reviewed-by: Po-Hao Su <supohaosu@gmail.com>
Reviewed-by: Qt CI Bot <qt_ci_bot@qt-project.org>
Reviewed-by: Christian Ehrlicher <ch.ehrlicher@gmx.de>

1 files changed, 105 insertions, 76 deletions

diff --git a/src/plugins/sqldrivers/sqlite/qsql_sqlite.cpp b/src/plugins/sqldrivers/sqlite/qsql_sqlite.cpp
index 92fee0abda..8f4bdaeb1b 100644
--- a/src/plugins/sqldrivers/sqlite/qsql_sqlite.cpp
+++ b/src/plugins/sqldrivers/sqlite/qsql_sqlite.cpp
@@ -40,22 +40,6 @@ QT_BEGIN_NAMESPACE
 
 using namespace Qt::StringLiterals;
 
-static QString _q_escapeIdentifier(const QString &identifier, QSqlDriver::IdentifierType type)
-{
-    QString res = identifier;
-    // If it contains [ and ] then we assume it to be escaped properly already as this indicates
-    // the syntax is exactly how it should be
-    if (identifier.contains(u'[') && identifier.contains(u']'))
-        return res;
-    if (!identifier.isEmpty() && !identifier.startsWith(u'"') && !identifier.endsWith(u'"')) {
-        res.replace(u'"', "\"\""_L1);
-        if (type == QSqlDriver::TableName)
-            res.replace(u'.', "\".\""_L1);
-        res = u'"' + res + u'"';
-    }
-    return res;
-}
-
 static int qGetColumnType(const QString &tpName)
 {
     const QString typeName = tpName.toLower();
@@ -114,11 +98,64 @@ class QSQLiteDriverPrivate : public QSqlDriverPrivate
 
 public:
     inline QSQLiteDriverPrivate() : QSqlDriverPrivate(QSqlDriver::SQLite) {}
+    bool isIdentifierEscaped(QStringView identifier) const;
+    QSqlIndex getTableInfo(QSqlQuery &query, const QString &tableName,
+                           bool onlyPIndex = false) const;
+
     sqlite3 *access = nullptr;
     QList<QSQLiteResult *> results;
     QStringList notificationid;
 };
 
+bool QSQLiteDriverPrivate::isIdentifierEscaped(QStringView identifier) const
+{
+    return identifier.size() > 2
+            && ((identifier.startsWith(u'"') && identifier.endsWith(u'"'))
+                || (identifier.startsWith(u'`') && identifier.endsWith(u'`'))
+                || (identifier.startsWith(u'[') && identifier.endsWith(u']')));
+}
+
+QSqlIndex QSQLiteDriverPrivate::getTableInfo(QSqlQuery &query, const QString &tableName,
+                                             bool onlyPIndex) const
+{
+    Q_Q(const QSQLiteDriver);
+    QString schema;
+    QString table = q->escapeIdentifier(tableName, QSqlDriver::TableName);
+    const auto indexOfSeparator = table.indexOf(u'.');
+    if (indexOfSeparator > -1) {
+        auto leftName = QStringView{table}.first(indexOfSeparator);
+        auto rightName = QStringView{table}.sliced(indexOfSeparator + 1);
+        if (isIdentifierEscaped(leftName) && isIdentifierEscaped(rightName)) {
+            schema = leftName.toString() + u'.';
+            table = rightName.toString();
+        }
+    }
+
+    query.exec("PRAGMA "_L1 + schema + "table_info ("_L1 + table + u')');
+    QSqlIndex ind;
+    while (query.next()) {
+        bool isPk = query.value(5).toInt();
+        if (onlyPIndex && !isPk)
+            continue;
+        QString typeName = query.value(2).toString().toLower();
+        QString defVal = query.value(4).toString();
+        if (!defVal.isEmpty() && defVal.at(0) == u'\'') {
+            const int end = defVal.lastIndexOf(u'\'');
+            if (end > 0)
+                defVal = defVal.mid(1, end - 1);
+        }
+
+        QSqlField fld(query.value(1).toString(), QMetaType(qGetColumnType(typeName)), tableName);
+        if (isPk && (typeName == "integer"_L1))
+            // INTEGER PRIMARY KEY fields are auto-generated in sqlite
+            // INT PRIMARY KEY is not the same as INTEGER PRIMARY KEY!
+            fld.setAutoValue(true);
+        fld.setRequired(query.value(3).toInt() != 0);
+        fld.setDefaultValue(defVal);
+        ind.append(fld);
+    }
+    return ind;
+}
 
 class QSQLiteResultPrivate : public QSqlCachedResultPrivate
 {
@@ -913,79 +950,26 @@ QStringList QSQLiteDriver::tables(QSql::TableType type) const
     return res;
 }
 
-static QSqlIndex qGetTableInfo(QSqlQuery &q, const QString &tableName, bool onlyPIndex = false)
-{
-    QString schema;
-    QString table(tableName);
-    const qsizetype indexOfSeparator = tableName.indexOf(u'.');
-    if (indexOfSeparator > -1) {
-        const qsizetype indexOfCloseBracket = tableName.indexOf(u']');
-        if (indexOfCloseBracket != tableName.size() - 1) {
-            // Handles a case like databaseName.tableName
-            schema = tableName.left(indexOfSeparator + 1);
-            table = tableName.mid(indexOfSeparator + 1);
-        } else {
-            const qsizetype indexOfOpenBracket = tableName.lastIndexOf(u'[', indexOfCloseBracket);
-            if (indexOfOpenBracket > 0) {
-                // Handles a case like databaseName.[tableName]
-                schema = tableName.left(indexOfOpenBracket);
-                table = tableName.mid(indexOfOpenBracket);
-            }
-        }
-    }
-    q.exec("PRAGMA "_L1 + schema + "table_info ("_L1 +
-           _q_escapeIdentifier(table, QSqlDriver::TableName) + u')');
-    QSqlIndex ind;
-    while (q.next()) {
-        bool isPk = q.value(5).toInt();
-        if (onlyPIndex && !isPk)
-            continue;
-        QString typeName = q.value(2).toString().toLower();
-        QString defVal = q.value(4).toString();
-        if (!defVal.isEmpty() && defVal.at(0) == u'\'') {
-            const int end = defVal.lastIndexOf(u'\'');
-            if (end > 0)
-                defVal = defVal.mid(1, end - 1);
-        }
-
-        QSqlField fld(q.value(1).toString(), QMetaType(qGetColumnType(typeName)), tableName);
-        if (isPk && (typeName == "integer"_L1))
-            // INTEGER PRIMARY KEY fields are auto-generated in sqlite
-            // INT PRIMARY KEY is not the same as INTEGER PRIMARY KEY!
-            fld.setAutoValue(true);
-        fld.setRequired(q.value(3).toInt() != 0);
-        fld.setDefaultValue(defVal);
-        ind.append(fld);
-    }
-    return ind;
-}
-
-QSqlIndex QSQLiteDriver::primaryIndex(const QString &tblname) const
+QSqlIndex QSQLiteDriver::primaryIndex(const QString &tablename) const
 {
+    Q_D(const QSQLiteDriver);
     if (!isOpen())
         return QSqlIndex();
 
-    QString table = tblname;
-    if (isIdentifierEscaped(table, QSqlDriver::TableName))
-        table = stripDelimiters(table, QSqlDriver::TableName);
-
     QSqlQuery q(createResult());
     q.setForwardOnly(true);
-    return qGetTableInfo(q, table, true);
+    return d->getTableInfo(q, tablename, true);
 }
 
-QSqlRecord QSQLiteDriver::record(const QString &tbl) const
+QSqlRecord QSQLiteDriver::record(const QString &tablename) const
 {
+    Q_D(const QSQLiteDriver);
     if (!isOpen())
         return QSqlRecord();
 
-    QString table = tbl;
-    if (isIdentifierEscaped(table, QSqlDriver::TableName))
-        table = stripDelimiters(table, QSqlDriver::TableName);
-
     QSqlQuery q(createResult());
     q.setForwardOnly(true);
-    return qGetTableInfo(q, table);
+    return d->getTableInfo(q, tablename);
 }
 
 QVariant QSQLiteDriver::handle() const
@@ -996,7 +980,52 @@ QVariant QSQLiteDriver::handle() const
 
 QString QSQLiteDriver::escapeIdentifier(const QString &identifier, IdentifierType type) const
 {
-    return _q_escapeIdentifier(identifier, type);
+    Q_D(const QSQLiteDriver);
+    if (identifier.isEmpty() || isIdentifierEscaped(identifier, type))
+        return identifier;
+
+    const auto indexOfSeparator = identifier.indexOf(u'.');
+    if (indexOfSeparator > -1) {
+        auto leftName = QStringView{identifier}.first(indexOfSeparator);
+        auto rightName = QStringView{identifier}.sliced(indexOfSeparator + 1);
+        const QStringView leftEnclose = d->isIdentifierEscaped(leftName) ? u"" : u"\"";
+        const QStringView rightEnclose = d->isIdentifierEscaped(rightName) ? u"" : u"\"";
+        if (leftEnclose.isEmpty() || rightEnclose.isEmpty())
+            return (leftEnclose + leftName + leftEnclose + u'.' + rightEnclose + rightName
+                    + rightEnclose);
+    }
+    return u'"' + identifier + u'"';
+}
+
+bool QSQLiteDriver::isIdentifierEscaped(const QString &identifier, IdentifierType type) const
+{
+    Q_D(const QSQLiteDriver);
+    Q_UNUSED(type);
+    return d->isIdentifierEscaped(QStringView{identifier});
+}
+
+QString QSQLiteDriver::stripDelimiters(const QString &identifier, IdentifierType type) const
+{
+    Q_D(const QSQLiteDriver);
+    const auto indexOfSeparator = identifier.indexOf(u'.');
+    if (indexOfSeparator > -1) {
+        auto leftName = QStringView{identifier}.first(indexOfSeparator);
+        auto rightName = QStringView{identifier}.sliced(indexOfSeparator + 1);
+        const auto leftEscaped = d->isIdentifierEscaped(leftName);
+        const auto rightEscaped = d->isIdentifierEscaped(rightName);
+        if (leftEscaped || rightEscaped) {
+            if (leftEscaped)
+                leftName = leftName.sliced(1).chopped(1);
+            if (rightEscaped)
+                rightName = rightName.sliced(1).chopped(1);
+            return leftName + u'.' + rightName;
+        }
+    }
+
+    if (isIdentifierEscaped(identifier, type))
+        return identifier.mid(1, identifier.size() - 2);
+
+    return identifier;
 }
 
 static void handle_sqlite_callback(void *qobj,int aoperation, char const *adbname, char const *atablename,