SQL/QSqlRelationTableModel: don't crash with more than one relation
When more than one relation is used, an internal container might be resized which can lead to a reallocation. Since the internal data structure holds a pointer to an element of this container, this pointer is invalidated after the reallocation. Therefore store a QSharedPointer instead. Pick-to: 6.7 6.5 Fixes: QTBUG-60674 Change-Id: I18c6157c7328be201f8b89a7ca12f423a86d9b71 Reviewed-by: Axel Spoerl <axel.spoerl@qt.io> (cherry picked from commit c0aabbd8a0c5a01c2048bcaf36525570a8e0bb35) Reviewed-by: Qt Cherry-pick Bot <cherrypick_bot@qt-project.org>
This commit is contained in:
parent
c342496831
commit
4ab6fc7215
@ -103,7 +103,8 @@ class QRelatedTableModel;
|
||||
struct QRelation
|
||||
{
|
||||
public:
|
||||
QRelation(): model(nullptr), m_parent(nullptr), m_dictInitialized(false) {}
|
||||
Q_DISABLE_COPY(QRelation) // QRelatedTableModel stores a pointer to this class
|
||||
QRelation() = default;
|
||||
void init(QSqlRelationalTableModel *parent, const QSqlRelation &relation);
|
||||
|
||||
void populateModel();
|
||||
@ -116,18 +117,18 @@ struct QRelation
|
||||
bool isValid() const;
|
||||
|
||||
QSqlRelation rel;
|
||||
QRelatedTableModel *model;
|
||||
QRelatedTableModel *model = nullptr;
|
||||
QHash<QString, QVariant> dictionary;//maps keys to display values
|
||||
|
||||
private:
|
||||
QSqlRelationalTableModel *m_parent;
|
||||
bool m_dictInitialized;
|
||||
QSqlRelationalTableModel *m_parent = nullptr;
|
||||
bool m_dictInitialized = false;
|
||||
};
|
||||
|
||||
class QRelatedTableModel : public QSqlTableModel
|
||||
{
|
||||
public:
|
||||
QRelatedTableModel(QRelation *rel, QObject *parent = nullptr, const QSqlDatabase &db = QSqlDatabase());
|
||||
QRelatedTableModel(QRelation *rel, QObject *parent, const QSqlDatabase &db);
|
||||
bool select() override;
|
||||
private:
|
||||
bool firstSelect;
|
||||
@ -241,7 +242,7 @@ public:
|
||||
QString fullyQualifiedFieldName(const QString &tableName, const QString &fieldName) const;
|
||||
|
||||
int nameToIndex(const QString &name) const override;
|
||||
mutable QList<QRelation> relations;
|
||||
QList<QSharedPointer<QRelation>> relations;
|
||||
QSqlRecord baseRec; // the record without relations
|
||||
void clearChanges();
|
||||
void clearCache() override;
|
||||
@ -254,7 +255,7 @@ public:
|
||||
void QSqlRelationalTableModelPrivate::clearChanges()
|
||||
{
|
||||
for (auto &rel : relations)
|
||||
rel.clear();
|
||||
rel->clear();
|
||||
}
|
||||
|
||||
void QSqlRelationalTableModelPrivate::revertCachedRow(int row)
|
||||
@ -276,7 +277,7 @@ int QSqlRelationalTableModelPrivate::nameToIndex(const QString &name) const
|
||||
void QSqlRelationalTableModelPrivate::clearCache()
|
||||
{
|
||||
for (auto &rel : relations)
|
||||
rel.clearDictionary();
|
||||
rel->clearDictionary();
|
||||
|
||||
QSqlTableModelPrivate::clearCache();
|
||||
}
|
||||
@ -394,10 +395,10 @@ QVariant QSqlRelationalTableModel::data(const QModelIndex &index, int role) cons
|
||||
Q_D(const QSqlRelationalTableModel);
|
||||
|
||||
if (role == Qt::DisplayRole && index.column() >= 0 && index.column() < d->relations.size() &&
|
||||
d->relations.value(index.column()).isValid()) {
|
||||
QRelation &relation = d->relations[index.column()];
|
||||
if (!relation.isDictionaryInitialized())
|
||||
relation.populateDictionary();
|
||||
d->relations.at(index.column())->isValid()) {
|
||||
auto relation = d->relations.at(index.column());
|
||||
if (!relation->isDictionaryInitialized())
|
||||
relation->populateDictionary();
|
||||
|
||||
//only perform a dictionary lookup for the display value
|
||||
//when the value at index has been changed or added.
|
||||
@ -409,7 +410,7 @@ QVariant QSqlRelationalTableModel::data(const QModelIndex &index, int role) cons
|
||||
if (d->strategy == OnManualSubmit || row.op() != QSqlTableModelPrivate::Delete) {
|
||||
QVariant v = row.rec().value(index.column());
|
||||
if (v.isValid())
|
||||
return relation.dictionary[v.toString()];
|
||||
return relation->dictionary[v.toString()];
|
||||
}
|
||||
}
|
||||
}
|
||||
@ -437,11 +438,11 @@ bool QSqlRelationalTableModel::setData(const QModelIndex &index, const QVariant
|
||||
{
|
||||
Q_D(QSqlRelationalTableModel);
|
||||
if ( role == Qt::EditRole && index.column() > 0 && index.column() < d->relations.size()
|
||||
&& d->relations.value(index.column()).isValid()) {
|
||||
QRelation &relation = d->relations[index.column()];
|
||||
if (!relation.isDictionaryInitialized())
|
||||
relation.populateDictionary();
|
||||
if (!relation.dictionary.contains(value.toString()))
|
||||
&& d->relations.at(index.column())->isValid()) {
|
||||
auto relation = d->relations.at(index.column());
|
||||
if (!relation->isDictionaryInitialized())
|
||||
relation->populateDictionary();
|
||||
if (!relation->dictionary.contains(value.toString()))
|
||||
return false;
|
||||
}
|
||||
return QSqlTableModel::setData(index, value, role);
|
||||
@ -470,9 +471,13 @@ void QSqlRelationalTableModel::setRelation(int column, const QSqlRelation &relat
|
||||
Q_D(QSqlRelationalTableModel);
|
||||
if (column < 0)
|
||||
return;
|
||||
if (d->relations.size() <= column)
|
||||
if (d->relations.size() <= column) {
|
||||
const auto oldSize = d->relations.size();
|
||||
d->relations.resize(column + 1);
|
||||
d->relations[column].init(this, relation);
|
||||
for (auto i = oldSize; i < d->relations.size(); ++i)
|
||||
d->relations[i] = QSharedPointer<QRelation>::create();
|
||||
}
|
||||
d->relations.at(column)->init(this, relation);
|
||||
}
|
||||
|
||||
/*!
|
||||
@ -484,7 +489,7 @@ void QSqlRelationalTableModel::setRelation(int column, const QSqlRelation &relat
|
||||
QSqlRelation QSqlRelationalTableModel::relation(int column) const
|
||||
{
|
||||
Q_D(const QSqlRelationalTableModel);
|
||||
return d->relations.value(column).rel;
|
||||
return d->relations.value(column) ? d->relations.at(column)->rel : QSqlRelation();
|
||||
}
|
||||
|
||||
QString QSqlRelationalTableModelPrivate::fullyQualifiedFieldName(const QString &tableName,
|
||||
@ -513,7 +518,7 @@ QString QSqlRelationalTableModel::selectStatement() const
|
||||
QHash<QString, int> fieldNames;
|
||||
QStringList fieldList;
|
||||
for (int i = 0; i < d->baseRec.count(); ++i) {
|
||||
QSqlRelation relation = d->relations.value(i).rel;
|
||||
QSqlRelation relation = d->relations.value(i) ? d->relations.at(i)->rel : QSqlRelation();
|
||||
QString name;
|
||||
if (relation.isValid()) {
|
||||
// Count the display column name, not the original foreign key
|
||||
@ -540,7 +545,7 @@ QString QSqlRelationalTableModel::selectStatement() const
|
||||
QString conditions;
|
||||
QString from = SqlrTm::from(tableName());
|
||||
for (int i = 0; i < d->baseRec.count(); ++i) {
|
||||
QSqlRelation relation = d->relations.value(i).rel;
|
||||
QSqlRelation relation = d->relations.value(i) ? d->relations.at(i)->rel : QSqlRelation();
|
||||
const QString tableField = d->fullyQualifiedFieldName(tableName(), d->db.driver()->escapeIdentifier(d->baseRec.fieldName(i), QSqlDriver::FieldName));
|
||||
if (relation.isValid()) {
|
||||
const QString relTableAlias = SqlrTm::relTablePrefix(i);
|
||||
@ -605,13 +610,13 @@ QSqlTableModel *QSqlRelationalTableModel::relationModel(int column) const
|
||||
if (column < 0 || column >= d->relations.size())
|
||||
return nullptr;
|
||||
|
||||
QRelation &relation = const_cast<QSqlRelationalTableModelPrivate *>(d)->relations[column];
|
||||
if (!relation.isValid())
|
||||
auto relation = d->relations.at(column);
|
||||
if (!relation || !relation->isValid())
|
||||
return nullptr;
|
||||
|
||||
if (!relation.model)
|
||||
relation.populateModel();
|
||||
return relation.model;
|
||||
if (!relation->model)
|
||||
relation->populateModel();
|
||||
return relation->model;
|
||||
}
|
||||
|
||||
/*!
|
||||
@ -682,7 +687,7 @@ void QSqlRelationalTableModel::setTable(const QString &table)
|
||||
void QSqlRelationalTableModelPrivate::translateFieldNames(QSqlRecord &values) const
|
||||
{
|
||||
for (int i = 0; i < values.count(); ++i) {
|
||||
if (relations.value(i).isValid()) {
|
||||
if (relations.value(i) && relations.at(i)->isValid()) {
|
||||
QVariant v = values.value(i);
|
||||
bool gen = values.isGenerated(i);
|
||||
values.replace(i, baseRec.field(i));
|
||||
@ -725,7 +730,7 @@ QString QSqlRelationalTableModel::orderByClause() const
|
||||
{
|
||||
Q_D(const QSqlRelationalTableModel);
|
||||
|
||||
const QSqlRelation rel = d->relations.value(d->sortColumn).rel;
|
||||
const QSqlRelation rel = d->relations.value(d->sortColumn) ? d->relations.at(d->sortColumn)->rel : QSqlRelation();
|
||||
if (!rel.isValid())
|
||||
return QSqlTableModel::orderByClause();
|
||||
|
||||
|
@ -45,6 +45,7 @@ private slots:
|
||||
void selectAfterUpdate();
|
||||
void relationOnFirstColumn();
|
||||
void setRelation();
|
||||
void setMultipleRelations();
|
||||
|
||||
private:
|
||||
void fixupTableNamesForDb(const QSqlDatabase &db);
|
||||
@ -1540,5 +1541,24 @@ void tst_QSqlRelationalTableModel::setRelation()
|
||||
QCOMPARE(model.data(model.index(0, 2)), QVariant(1));
|
||||
}
|
||||
|
||||
void tst_QSqlRelationalTableModel::setMultipleRelations()
|
||||
{
|
||||
QFETCH_GLOBAL(QString, dbName);
|
||||
QSqlDatabase db = QSqlDatabase::database(dbName);
|
||||
CHECK_DATABASE(db);
|
||||
recreateTestTables(db);
|
||||
|
||||
QSqlRelationalTableModel model(0, db);
|
||||
model.setTable(reltest1);
|
||||
QVERIFY_SQL(model, select());
|
||||
model.setRelation(2, QSqlRelation(reltest2, "id", "title"));
|
||||
model.data(model.index(0, 2)); // initialize model for QSqlRelation above
|
||||
// id must be big enough that the internal QList needs to be reallocated
|
||||
model.setRelation(100, QSqlRelation(reltest2, "id", "title"));
|
||||
QSqlTableModel *relationModel = model.relationModel(2);
|
||||
QVERIFY(relationModel);
|
||||
QVERIFY(relationModel->select());
|
||||
}
|
||||
|
||||
QTEST_MAIN(tst_QSqlRelationalTableModel)
|
||||
#include "tst_qsqlrelationaltablemodel.moc"
|
||||
|
Loading…
x
Reference in New Issue
Block a user