QAbstractItemView: do not access invalid model indices (3/N)

The slots connected to rowsAboutToBeRemoved and columnsAboutToBeRemoved
attempt to find a new current index by scanning the model around the
existing current index -- in case that index is indeed about to be
removed.

The problem is that the scanning was done without any sorts of bounds
checking; instead it was relying on the model's index() to return an
invalid index for an out of bounds request.

Fix that by adding bounds checking. Since models are not supposed
to return invalid indices for in-bounds index() calls, added some
warnings if that happens.

For some reason, the code handling rows and columns isn't perfectly
symmetrical. Rows are searched both forwards and backwards, while
columns only backwards, and the related code is slightly different.
Filed QTBUG-100273 to try and understand what's going on.

Change-Id: I7452d8c521e74daa4408e6cc969ce5a6059f53ea
Pick-to: 5.15 6.2 6.3
Reviewed-by: David Faure <david.faure@kdab.com>
Reviewed-by: Qt CI Bot <qt_ci_bot@qt-project.org>
Reviewed-by: Richard Moe Gustavsen <richard.gustavsen@qt.io>
This commit is contained in:
Giuseppe D'Angelo 2022-01-27 09:47:59 +01:00
parent 63937ffe6e
commit 453f469216

View File

@ -3477,15 +3477,39 @@ void QAbstractItemView::rowsAboutToBeRemoved(const QModelIndex &parent, int star
} else {
int row = end + 1;
QModelIndex next;
do { // find the next visible and enabled item
const int rowCount = d->model->rowCount(parent);
bool found = false;
// find the next visible and enabled item
while (row < rowCount && !found) {
next = d->model->index(row++, current.column(), current.parent());
} while (next.isValid() && (isIndexHidden(next) || !d->isIndexEnabled(next)));
if (row > d->model->rowCount(parent)) {
row = start - 1;
do { // find the previous visible and enabled item
next = d->model->index(row--, current.column(), current.parent());
} while (next.isValid() && (isIndexHidden(next) || !d->isIndexEnabled(next)));
#ifdef QT_DEBUG
if (!next.isValid()) {
qWarning("Model unexpectedly returned an invalid index");
break;
}
#endif
if (!isIndexHidden(next) && d->isIndexEnabled(next)) {
found = true;
break;
}
}
if (!found) {
row = start - 1;
// find the previous visible and enabled item
while (row >= 0) {
next = d->model->index(row--, current.column(), current.parent());
#ifdef QT_DEBUG
if (!next.isValid()) {
qWarning("Model unexpectedly returned an invalid index");
break;
}
#endif
if (!isIndexHidden(next) && d->isIndexEnabled(next))
break;
}
}
setCurrentIndex(next);
}
}
@ -3563,9 +3587,19 @@ void QAbstractItemViewPrivate::_q_columnsAboutToBeRemoved(const QModelIndex &par
} else {
int column = end;
QModelIndex next;
do { // find the next visible and enabled item
const int columnCount = model->columnCount(current.parent());
// find the next visible and enabled item
while (column < columnCount) {
next = model->index(current.row(), column++, current.parent());
} while (next.isValid() && (q->isIndexHidden(next) || !isIndexEnabled(next)));
#ifdef QT_DEBUG
if (!next.isValid()) {
qWarning("Model unexpectedly returned an invalid index");
break;
}
#endif
if (!q->isIndexHidden(next) && isIndexEnabled(next))
break;
}
q->setCurrentIndex(next);
}
}