Qt Designer: Fix Qt Creator crashes on reloading malformed UI files

Extract the functionality to read the DomUI with version check
to QFormBuilderExtra so that the reading can be split into DomUI
and widget creation.
In FormWindow, check the success of the DomUI creation before
modifying Qt Designer's internal state.

Task-number: QTCREATORBUG-20742
Change-Id: I095f80e90966eba67d8994cd68e83c3e11aa0ee9
Reviewed-by: Jarek Kobus <jaroslaw.kobus@qt.io>

6 files changed, 137 insertions, 87 deletions

diff --git a/src/designer/src/components/formeditor/formwindow.cpp b/src/designer/src/components/formeditor/formwindow.cpp
index b03be6c82..1f638ec8e 100644
--- a/src/designer/src/components/formeditor/formwindow.cpp
+++ b/src/designer/src/components/formeditor/formwindow.cpp
@@ -2140,6 +2140,14 @@ bool FormWindow::handleContextMenu(QWidget *, QWidget *managedWidget, QContextMe
 
 bool FormWindow::setContents(QIODevice *dev, QString *errorMessageIn /* = 0 */)
 {
+    QDesignerResource r(this);
+    QScopedPointer<DomUI> ui(r.readUi(dev));
+    if (ui.isNull()) {
+        if (errorMessageIn)
+            *errorMessageIn = r.errorString();
+        return false;
+    }
+
     UpdateBlocker ub(this);
     clearSelection();
     m_selection->clearSelectionPool();
@@ -2151,8 +2159,7 @@ bool FormWindow::setContents(QIODevice *dev, QString *errorMessageIn /* = 0 */)
     m_undoStack.clear();
     emit changed();
 
-    QDesignerResource r(this);
-    QWidget *w = r.load(dev, formContainer());
+    QWidget *w = r.loadUi(ui.data(), formContainer());
     if (w) {
         setMainContainer(w);
         emit changed();
diff --git a/src/designer/src/components/formeditor/qdesigner_resource.cpp b/src/designer/src/components/formeditor/qdesigner_resource.cpp
index b799e648c..caca4d497 100644
--- a/src/designer/src/components/formeditor/qdesigner_resource.cpp
+++ b/src/designer/src/components/formeditor/qdesigner_resource.cpp
@@ -461,6 +461,11 @@ QDesignerResource::~QDesignerResource()
 {
 }
 
+DomUI *QDesignerResource::readUi(QIODevice *dev)
+{
+    return d->readUi(dev);
+}
+
 static inline QString messageBoxTitle()
 {
     return QApplication::translate("Designer", "Qt Designer");
@@ -596,10 +601,19 @@ void QDesignerResource::saveDom(DomUI *ui, QWidget *widget)
 
 QWidget *QDesignerResource::load(QIODevice *dev, QWidget *parentWidget)
 {
-    QWidget *w = QEditorFormBuilder::load(dev, parentWidget);
-    if (w)  // Store the class name as 'reset' value for the main container's object name.
-        w->setProperty("_q_classname", w->objectName());
-    return w;
+    QScopedPointer<DomUI> ui(readUi(dev));
+    return ui.isNull() ? nullptr : loadUi(ui.data(), parentWidget);
+}
+
+QWidget *QDesignerResource::loadUi(DomUI *ui, QWidget *parentWidget)
+{
+    QWidget *widget = create(ui, parentWidget);
+    // Store the class name as 'reset' value for the main container's object name.
+    if (widget)
+        widget->setProperty("_q_classname", widget->objectName());
+    else if (d->m_errorString.isEmpty())
+        d->m_errorString = QFormBuilderExtra::msgInvalidUiFile();
+    return widget;
 }
 
 bool QDesignerResource::saveRelative() const
diff --git a/src/designer/src/components/formeditor/qdesigner_resource.h b/src/designer/src/components/formeditor/qdesigner_resource.h
index f444f6334..87620f600 100644
--- a/src/designer/src/components/formeditor/qdesigner_resource.h
+++ b/src/designer/src/components/formeditor/qdesigner_resource.h
@@ -78,6 +78,9 @@ public:
 
     QWidget *load(QIODevice *dev, QWidget *parentWidget) override;
 
+    DomUI *readUi(QIODevice *dev);
+    QWidget *loadUi(DomUI *ui, QWidget *parentWidget);
+
 protected:
     using QEditorFormBuilder::create;
     using QEditorFormBuilder::createDom;
diff --git a/src/designer/src/lib/uilib/abstractformbuilder.cpp b/src/designer/src/lib/uilib/abstractformbuilder.cpp
index af8b73b0d..f9275b852 100644
--- a/src/designer/src/lib/uilib/abstractformbuilder.cpp
+++ b/src/designer/src/lib/uilib/abstractformbuilder.cpp
@@ -93,7 +93,7 @@
 #  include <private/qlayout_p.h> // Compiling within Designer
 #endif
 
-#include <QtCore/QXmlStreamReader>
+#include <QtCore/QXmlStreamWriter>
 
 #include <QtCore/qdebug.h>
 
@@ -178,70 +178,6 @@ QAbstractFormBuilder::~QAbstractFormBuilder()
 {
 }
 
-// Return UI file version from attribute 'version="4.0"'
-static QPair<int, int> uiVersion(const QString &attr)
-{
-    const QVector<QStringRef> versions = attr.splitRef(QLatin1Char('.'), QString::SkipEmptyParts);
-    if (versions.size() >= 2) {
-        bool okMajor, okMinor;
-        const int majorVersion = versions.at(0).toInt(&okMajor);
-        const int minorVersion = versions.at(1).toInt(&okMinor);
-        if (okMajor &&  okMinor)
-            return QPair<int, int>(majorVersion, minorVersion);
-    }
-    return QPair<int, int>(-1, -1);
-}
-
-static inline QString msgXmlError(const QXmlStreamReader &reader)
-{
-    return QCoreApplication::translate("QAbstractFormBuilder", "An error has occurred while reading the UI file at line %1, column %2: %3")
-                                       .arg(reader.lineNumber()).arg(reader.columnNumber()).arg(reader.errorString());
-}
-
-// Read and check the  version and the (optional) language attribute
-// of an <ui> element and leave reader positioned at <ui>.
-static bool inline readUiAttributes(QXmlStreamReader &reader, const QString &language, QString *errorMessage)
-{
-    const QString uiElement = QStringLiteral("ui");
-    // Read up to first element
-    while (!reader.atEnd()) {
-        switch (reader.readNext()) {
-        case QXmlStreamReader::Invalid:
-            *errorMessage = msgXmlError(reader);
-            return false;
-        case QXmlStreamReader::StartElement:
-            if (reader.name().compare(uiElement, Qt::CaseInsensitive) == 0) {
-                const QString versionAttribute = QStringLiteral("version");
-                const QString languageAttribute = QStringLiteral("language");
-                const QXmlStreamAttributes attributes = reader.attributes();
-                if (attributes.hasAttribute(versionAttribute)) {
-                    const QString versionString = attributes.value(versionAttribute).toString();
-                    if (uiVersion(versionString).first < 4) {
-                        *errorMessage = QCoreApplication::translate("QAbstractFormBuilder", "This file was created using Designer from Qt-%1 and cannot be read.")
-                                .arg(versionString);
-                        return false;
-                    } // version error
-                }     // has version
-                if (attributes.hasAttribute(languageAttribute)) {
-                    // Check on optional language (Jambi)
-                    const QString formLanguage = attributes.value(languageAttribute).toString();
-                    if (!formLanguage.isEmpty() && formLanguage.compare(language, Qt::CaseInsensitive)) {
-                        *errorMessage = QCoreApplication::translate("QAbstractFormBuilder", "This file cannot be read because it was created using %1.").arg(formLanguage);
-                        return false;
-                    } // language error
-                }    // has language
-                return true;
-            }  // <ui> matched
-            break;
-        default:
-            break;
-        }
-    }
-    // No <ui> found.
-    *errorMessage = QCoreApplication::translate("QAbstractFormBuilder",  "Invalid UI file: The root element <ui> is missing.");
-    return false;
-}
-
 /*!
     \fn QWidget *QAbstractFormBuilder::load(QIODevice *device, QWidget *parent)
 
@@ -252,23 +188,12 @@ static bool inline readUiAttributes(QXmlStreamReader &reader, const QString &lan
 */
 QWidget *QAbstractFormBuilder::load(QIODevice *dev, QWidget *parentWidget)
 {
-    QXmlStreamReader reader(dev);
-    d->m_errorString.clear();
-    if (!readUiAttributes(reader, d->m_language, &d->m_errorString)) {
-        uiLibWarning(d->m_errorString);
-        return 0;
-    }
-    DomUI ui;
-    ui.read(reader);
-    if (reader.hasError()) {
-        d->m_errorString = msgXmlError(reader);
-        uiLibWarning(d->m_errorString);
-        return 0;
-    }
-
-    QWidget *widget = create(&ui, parentWidget);
+    QScopedPointer<DomUI> ui(d->readUi(dev));
+    if (ui.isNull())
+        return nullptr;
+    QWidget *widget = create(ui.data(), parentWidget);
     if (!widget && d->m_errorString.isEmpty())
-        d->m_errorString = QCoreApplication::translate("QAbstractFormBuilder", "Invalid UI file");
+        d->m_errorString = QFormBuilderExtra::msgInvalidUiFile();
     return widget;
 }
 
diff --git a/src/designer/src/lib/uilib/formbuilderextra.cpp b/src/designer/src/lib/uilib/formbuilderextra.cpp
index 79ca0dda7..23170a518 100644
--- a/src/designer/src/lib/uilib/formbuilderextra.cpp
+++ b/src/designer/src/lib/uilib/formbuilderextra.cpp
@@ -63,6 +63,7 @@
 #include <QtCore/QTextStream>
 #include <QtCore/QStringList>
 #include <QtCore/QCoreApplication>
+#include <QtCore/QXmlStreamReader>
 
 #include <limits.h>
 
@@ -114,6 +115,102 @@ void QFormBuilderExtra::clear()
     m_buttonGroups.clear();
 }
 
+// Return UI file version from attribute 'version="4.0"'
+static QPair<int, int> uiVersion(const QString &attr)
+{
+    const QVector<QStringRef> versions = attr.splitRef(QLatin1Char('.'), QString::SkipEmptyParts);
+    if (versions.size() >= 2) {
+        bool okMajor, okMinor;
+        const int majorVersion = versions.at(0).toInt(&okMajor);
+        const int minorVersion = versions.at(1).toInt(&okMinor);
+        if (okMajor &&  okMinor)
+            return QPair<int, int>(majorVersion, minorVersion);
+    }
+    return QPair<int, int>(-1, -1);
+}
+
+static inline QString msgXmlError(const QXmlStreamReader &reader)
+{
+    return QCoreApplication::translate("QAbstractFormBuilder",
+                                       "An error has occurred while reading the UI file at line %1, column %2: %3")
+                                       .arg(reader.lineNumber()).arg(reader.columnNumber())
+                                       .arg(reader.errorString());
+}
+
+// Read and check the  version and the (optional) language attribute
+// of an <ui> element and leave reader positioned at <ui>.
+static bool inline readUiAttributes(QXmlStreamReader &reader, const QString &language,
+                                    QString *errorMessage)
+{
+    const QString uiElement = QStringLiteral("ui");
+    // Read up to first element
+    while (!reader.atEnd()) {
+        switch (reader.readNext()) {
+        case QXmlStreamReader::Invalid:
+            *errorMessage = msgXmlError(reader);
+            return false;
+        case QXmlStreamReader::StartElement:
+            if (reader.name().compare(uiElement, Qt::CaseInsensitive) == 0) {
+                const QString versionAttribute = QStringLiteral("version");
+                const QString languageAttribute = QStringLiteral("language");
+                const QXmlStreamAttributes attributes = reader.attributes();
+                if (attributes.hasAttribute(versionAttribute)) {
+                    const QString versionString = attributes.value(versionAttribute).toString();
+                    if (uiVersion(versionString).first < 4) {
+                        *errorMessage =
+                            QCoreApplication::translate("QAbstractFormBuilder",
+                                                        "This file was created using Designer from Qt-%1 and cannot be read.")
+                                                        .arg(versionString);
+                        return false;
+                    } // version error
+                }     // has version
+                if (attributes.hasAttribute(languageAttribute)) {
+                    // Check on optional language (Jambi)
+                    const QString formLanguage = attributes.value(languageAttribute).toString();
+                    if (!formLanguage.isEmpty() && formLanguage.compare(language, Qt::CaseInsensitive)) {
+                        *errorMessage =
+                            QCoreApplication::translate("QAbstractFormBuilder",
+                                                        "This file cannot be read because it was created using %1.")
+                                                        .arg(formLanguage);
+                        return false;
+                    } // language error
+                }    // has language
+                return true;
+            }  // <ui> matched
+            break;
+        default:
+            break;
+        }
+    }
+    // No <ui> found.
+    *errorMessage = QCoreApplication::translate("QAbstractFormBuilder",
+                                                "Invalid UI file: The root element <ui> is missing.");
+    return false;
+}
+
+DomUI *QFormBuilderExtra::readUi(QIODevice *dev)
+{
+    QXmlStreamReader reader(dev);
+    m_errorString.clear();
+    if (!readUiAttributes(reader, m_language, &m_errorString)) {
+        uiLibWarning(m_errorString);
+        return nullptr;
+    }
+    DomUI *ui = new DomUI;
+    ui->read(reader);
+    if (reader.hasError()) {
+        m_errorString = msgXmlError(reader);
+        uiLibWarning(m_errorString);
+        delete ui;
+        return nullptr;
+    }
+    return ui;
+}
+
+QString QFormBuilderExtra::msgInvalidUiFile()
+{
+    return QCoreApplication::translate("QAbstractFormBuilder", "Invalid UI file");
+}
 
 bool QFormBuilderExtra::applyPropertyInternally(QObject *o, const QString &propertyName, const QVariant &value)
 {
diff --git a/src/designer/src/lib/uilib/formbuilderextra_p.h b/src/designer/src/lib/uilib/formbuilderextra_p.h
index 542ae47cd..df90c62b3 100644
--- a/src/designer/src/lib/uilib/formbuilderextra_p.h
+++ b/src/designer/src/lib/uilib/formbuilderextra_p.h
@@ -92,6 +92,7 @@ namespace QFormInternal
 class DomButtonGroups;
 class DomButtonGroup;
 class DomCustomWidget;
+class DomUI;
 
 class QAbstractFormBuilder;
 class QResourceBuilder;
@@ -115,6 +116,9 @@ public:
 
     void clear();
 
+    DomUI *readUi(QIODevice *dev);
+    static QString msgInvalidUiFile();
+
     bool applyPropertyInternally(QObject *o, const QString &propertyName, const QVariant &value);
 
     enum BuddyMode { BuddyApplyAll, BuddyApplyVisibleOnly };