From 9bbf761373f12dcdcc2a1bf56ddbe8b002fe950c Mon Sep 17 00:00:00 2001
From: Volker Hilsheimer <volker.hilsheimer@qt.io>
Date: Thu, 12 Jan 2023 16:25:31 +0100
Subject: [PATCH] QFormLayout: don't access out-of-bounds layout data

When rows are hidden (implicitly or explicitly), then their layout data
does not get fully updated. If rows get hidden after the layout data has
been calculated once, then we must make sure that their indices are
reset. Otherwise we might access array indices that are out of bounds
when the layout data structure gets resized to fit only visible rows.

For good measure, skip entirely over hidden rows when accessing the
layout data when arranging the widget.

Fixes: QTBUG-109237
Change-Id: I4d6943b6a110edb61f60ce78d31f0fc64b5cc03d
Reviewed-by: Axel Spoerl <axel.spoerl@qt.io>
(cherry picked from commit c3a5fe2fd7a1b8a6b6133c938ffe6b3f30181bf0)
Reviewed-by: Qt Cherry-pick Bot <cherrypick_bot@qt-project.org>
---
 src/widgets/kernel/qformlayout.cpp            | 18 +++++++-
 .../kernel/qformlayout/tst_qformlayout.cpp    | 44 +++++++++++++++++++
 2 files changed, 60 insertions(+), 2 deletions(-)

diff --git a/src/widgets/kernel/qformlayout.cpp b/src/widgets/kernel/qformlayout.cpp
index e924c719979..991b1429d7f 100644
--- a/src/widgets/kernel/qformlayout.cpp
+++ b/src/widgets/kernel/qformlayout.cpp
@@ -480,6 +480,7 @@ void QFormLayoutPrivate::recalcHFW(int w)
 
 void QFormLayoutPrivate::setupHfwLayoutData()
 {
+    Q_Q(QFormLayout);
     // setupVerticalLayoutData must be called before this
     // setupHorizontalLayoutData must also be called before this
     // copies non hfw data into hfw
@@ -504,6 +505,10 @@ void QFormLayoutPrivate::setupHfwLayoutData()
         QFormLayoutItem *label = m_matrix(i, 0);
         QFormLayoutItem *field = m_matrix(i, 1);
 
+        // ignore rows with only hidden items
+        if (!q->isRowVisible(i))
+            continue;
+
         if (label && label->vLayoutIndex > -1) {
             if (label->isHfw) {
                 // We don't check sideBySide here, since a label is only
@@ -681,9 +686,15 @@ void QFormLayoutPrivate::setupVerticalLayoutData(int width)
         QFormLayoutItem *label = m_matrix(i, 0);
         QFormLayoutItem *field = m_matrix(i, 1);
 
-        // Totally ignore empty rows or rows with only hidden items
-        if (!q->isRowVisible(i))
+        // Ignore empty rows or rows with only hidden items,
+        // and invalidate their position in the layout.
+        if (!q->isRowVisible(i)) {
+            if (label)
+                label->vLayoutIndex = -1;
+            if (field)
+                field->vLayoutIndex = -1;
             continue;
+        }
 
         QSize min1;
         QSize min2;
@@ -2190,6 +2201,9 @@ void QFormLayoutPrivate::arrangeWidgets(const QList<QLayoutStruct> &layouts, QRe
         QFormLayoutItem *label = m_matrix(i, 0);
         QFormLayoutItem *field = m_matrix(i, 1);
 
+        if (!q->isRowVisible(i))
+            continue;
+
         if (label && label->vLayoutIndex > -1) {
             int height = layouts.at(label->vLayoutIndex).size;
             if ((label->expandingDirections() & Qt::Vertical) == 0) {
diff --git a/tests/auto/widgets/kernel/qformlayout/tst_qformlayout.cpp b/tests/auto/widgets/kernel/qformlayout/tst_qformlayout.cpp
index 33e6bd64db0..44b716bb34a 100644
--- a/tests/auto/widgets/kernel/qformlayout/tst_qformlayout.cpp
+++ b/tests/auto/widgets/kernel/qformlayout/tst_qformlayout.cpp
@@ -112,6 +112,7 @@ private slots:
     void setLayout();
     void hideShowRow();
     void showWithHiddenRow();
+    void hiddenRowAndStretch();
 
 /*
     QLayoutItem *itemAt(int row, ItemRole role) const;
@@ -1253,6 +1254,49 @@ void tst_QFormLayout::showWithHiddenRow()
     topLevel.show();
 }
 
+/*
+    Test that hiding rows does not leave outdated layout data behind
+    in hidden items that results in out-of-bounds array access. See
+    QTBUG-109237.
+*/
+void tst_QFormLayout::hiddenRowAndStretch()
+{
+    QWidget topLevel;
+    QFormLayout layout;
+    layout.setRowWrapPolicy(QFormLayout::WrapAllRows);
+
+    // We need our own stretcher item so that QFormLayout doesn't insert
+    // it's own, as that would grow the size of the layout data array again.
+    QSpacerItem *stretch = new QSpacerItem(100, 100, QSizePolicy::Expanding, QSizePolicy::Expanding);
+    layout.setItem(0, QFormLayout::FieldRole, stretch);
+
+    QLabel *lastLabel = nullptr;
+    QLineEdit *lastField = nullptr;
+    for (int row = 1; row < 4; ++row) {
+        QLabel *label = new QLabel(QString("Label %1").arg(row));
+        label->setWordWrap(true);
+        QLineEdit *field = new QLineEdit;
+        layout.setWidget(row, QFormLayout::LabelRole, label);
+        layout.setWidget(row, QFormLayout::FieldRole, field);
+        if (row == 3) {
+            lastLabel = label;
+            lastField = field;
+        }
+    }
+
+    Q_ASSERT(lastLabel);
+    Q_ASSERT(lastField);
+
+    topLevel.setLayout(&layout);
+    topLevel.sizeHint();
+
+    lastLabel->setVisible(false);
+    lastField->setVisible(false);
+
+    // should not assert here
+    topLevel.show();
+}
+
 void tst_QFormLayout::itemAt()
 {
     QWidget topLevel;