QFileSystemModel: don't crash with setIconProvider(nullptr)

The method takes a pointer, so the code shouldn't crash when passed a nullptr. QFileInfoGatherer::getInfo() still needs to generate a descriptive string for the file, so we refactor QAbstractFileIconProvider::type() to put the implementation into a reusable static function QAbstractFileIconProviderPrivate::getFileType(const QFileInfo &info). This unfortunately involves constructing a QMimeDatabase on the fly, but the docs say that is fine. Drive-by change: use nullptr instead of `0` for pointers. Pick-to: 6.5 6.6 6.7 Fixes: QTBUG-99178 Change-Id: Ia32ea0a26701d593e74fbecced7be8d9e0aa0f52 Reviewed-by: Ahmad Samir <a.samirh78@gmail.com> Reviewed-by: Shawn Rutledge <shawn.rutledge@qt.io>
qt · Feb 1, 2024 · 2a8b27b · 2a8b27b
1 parent 0211dfd
commit 2a8b27b
Show file tree

Hide file tree

Showing 9 changed files with 70 additions and 21 deletions.
diff --git a/src/gui/image/qabstractfileiconprovider.cpp b/src/gui/image/qabstractfileiconprovider.cpp
@@ -229,21 +229,16 @@ QIcon QAbstractFileIconProvider::icon(const QFileInfo &info) const
     return result.isNull() ? d->getPlatformThemeIcon(info) : result;
 }
 
-/*!
-  Returns the type of the file described by \a info.
-*/
 
-QString QAbstractFileIconProvider::type(const QFileInfo &info) const
+QString QAbstractFileIconProviderPrivate::getFileType(const QFileInfo &info)
 {
-    Q_D(const QAbstractFileIconProvider);
     if (QFileSystemEntry::isRootPath(info.absoluteFilePath()))
         return QGuiApplication::translate("QAbstractFileIconProvider", "Drive");
     if (info.isFile()) {
 #if QT_CONFIG(mimetype)
-        const QMimeType mimeType = d->mimeDatabase.mimeTypeForFile(info);
+        const QMimeType mimeType = QMimeDatabase().mimeTypeForFile(info);
         return mimeType.comment().isEmpty() ? mimeType.name() : mimeType.comment();
 #else
-        Q_UNUSED(d);
         return QGuiApplication::translate("QAbstractFileIconProvider", "File");
 #endif
     }
@@ -273,4 +268,13 @@ QString QAbstractFileIconProvider::type(const QFileInfo &info) const
     return QGuiApplication::translate("QAbstractFileIconProvider", "Unknown");
 }
 
+/*!
+  Returns the type of the file described by \a info.
+*/
+
+QString QAbstractFileIconProvider::type(const QFileInfo &info) const
+{
+    return QAbstractFileIconProviderPrivate::getFileType(info);
+}
+
 QT_END_NAMESPACE
diff --git a/src/gui/image/qabstractfileiconprovider_p.h b/src/gui/image/qabstractfileiconprovider_p.h
@@ -37,6 +37,7 @@ class Q_GUI_EXPORT QAbstractFileIconProviderPrivate
     QIcon getIconThemeIcon(const QFileInfo &info) const;
 
     static void clearIconTypeCache();
+    static QString getFileType(const QFileInfo &info);
 
     QAbstractFileIconProvider *q_ptr = nullptr;
     QAbstractFileIconProvider::Options options = {};

diff --git a/src/gui/itemmodels/qfileinfogatherer.cpp b/src/gui/itemmodels/qfileinfogatherer.cpp
@@ -5,6 +5,7 @@
 #include <qcoreapplication.h>
 #include <qdebug.h>
 #include <qdiriterator.h>
+#include <private/qabstractfileiconprovider_p.h>
 #include <private/qfileinfo_p.h>
 #ifndef Q_OS_WIN
 #  include <unistd.h>
@@ -342,8 +343,12 @@ void QFileInfoGatherer::run()
 QExtendedInformation QFileInfoGatherer::getInfo(const QFileInfo &fileInfo) const
 {
     QExtendedInformation info(fileInfo);
-    info.icon = m_iconProvider->icon(fileInfo);
-    info.displayType = m_iconProvider->type(fileInfo);
+    if (m_iconProvider) {
+        info.icon = m_iconProvider->icon(fileInfo);
+        info.displayType = m_iconProvider->type(fileInfo);
+    } else {
+        info.displayType = QAbstractFileIconProviderPrivate::getFileType(fileInfo);
+    }
 #if QT_CONFIG(filesystemwatcher)
     // ### Not ready to listen all modifications by default
     static const bool watchFiles = qEnvironmentVariableIsSet("QT_FILESYSTEMMODEL_WATCH_FILES");

diff --git a/src/gui/itemmodels/qfilesystemmodel.cpp b/src/gui/itemmodels/qfilesystemmodel.cpp
@@ -693,7 +693,9 @@ QVariant QFileSystemModel::myComputer(int role) const
         return QFileSystemModelPrivate::myComputer();
 #if QT_CONFIG(filesystemwatcher)
     case Qt::DecorationRole:
-        return d->fileInfoGatherer->iconProvider()->icon(QAbstractFileIconProvider::Computer);
+        if (auto *provider = d->fileInfoGatherer->iconProvider())
+            return provider->icon(QAbstractFileIconProvider::Computer);
+    break;
 #endif
     }
     return QVariant();
@@ -733,10 +735,9 @@ QVariant QFileSystemModel::data(const QModelIndex &index, int role) const
             QIcon icon = d->icon(index);
 #if QT_CONFIG(filesystemwatcher)
             if (icon.isNull()) {
-                if (d->node(index)->isDir())
-                    icon = d->fileInfoGatherer->iconProvider()->icon(QAbstractFileIconProvider::Folder);
-                else
-                    icon = d->fileInfoGatherer->iconProvider()->icon(QAbstractFileIconProvider::File);
+                using P = QAbstractFileIconProvider;
+                if (auto *provider = d->fileInfoGatherer->iconProvider())
+                    icon = provider->icon(d->node(index)->isDir() ? P::Folder: P::File);
             }
 #endif // filesystemwatcher
             return icon;
@@ -1569,7 +1570,7 @@ QAbstractFileIconProvider *QFileSystemModel::iconProvider() const
     Q_D(const QFileSystemModel);
     return d->fileInfoGatherer->iconProvider();
 #else
-    return 0;
+    return nullptr;
 #endif
 }
 

diff --git a/src/gui/itemmodels/qfilesystemmodel_p.h b/src/gui/itemmodels/qfilesystemmodel_p.h
@@ -150,8 +150,12 @@ class Q_GUI_EXPORT QFileSystemModelPrivate : public QAbstractItemModelPrivate
             return visibleChildren.indexOf(childName);
         }
         void updateIcon(QAbstractFileIconProvider *iconProvider, const QString &path) {
+            if (!iconProvider)
+                return;
+
             if (info)
                 info->icon = iconProvider->icon(QFileInfo(path));
+
             for (QFileSystemNode *child : std::as_const(children)) {
                 //On windows the root (My computer) has no path so we don't want to add a / for nothing (e.g. /C:/)
                 if (!path.isEmpty()) {
@@ -165,6 +169,9 @@ class Q_GUI_EXPORT QFileSystemModelPrivate : public QAbstractItemModelPrivate
         }
 
         void retranslateStrings(QAbstractFileIconProvider *iconProvider, const QString &path) {
+            if (!iconProvider)
+                return;
+
             if (info)
                 info->displayType = iconProvider->type(QFileInfo(path));
             for (QFileSystemNode *child : std::as_const(children)) {

diff --git a/src/widgets/dialogs/qsidebar.cpp b/src/widgets/dialogs/qsidebar.cpp
@@ -183,11 +183,14 @@ void QUrlModel::setUrl(const QModelIndex &index, const QUrl &url, const QModelIn
             setData(index, true, EnabledRole);
         }
 
+        // newIcon could be null if fileSystemModel->iconProvider() returns null
+        if (!newIcon.isNull()) {
         // Make sure that we have at least 32x32 images
-        const QSize size = newIcon.actualSize(QSize(32,32));
-        if (size.width() < 32) {
-            QPixmap smallPixmap = newIcon.pixmap(QSize(32, 32));
-            newIcon.addPixmap(smallPixmap.scaledToWidth(32, Qt::SmoothTransformation));
+            const QSize size = newIcon.actualSize(QSize(32,32));
+            if (size.width() < 32) {
+                QPixmap smallPixmap = newIcon.pixmap(QSize(32, 32));
+                newIcon.addPixmap(smallPixmap.scaledToWidth(32, Qt::SmoothTransformation));
+            }
         }
 
         if (index.data().toString() != newName)

diff --git a/tests/auto/gui/itemmodels/qfilesystemmodel/tst_qfilesystemmodel.cpp b/tests/auto/gui/itemmodels/qfilesystemmodel/tst_qfilesystemmodel.cpp
@@ -65,6 +65,7 @@ private slots:
     void rootPath();
     void readOnly();
     void iconProvider();
+    void nullIconProvider();
 
     void rowCount();
 
@@ -311,6 +312,19 @@ void tst_QFileSystemModel::iconProvider()
     QCOMPARE(myModel->fileIcon(myModel->index(QDir::homePath())).pixmap(50, 50), mb);
 }
 
+void tst_QFileSystemModel::nullIconProvider()
+{
+    QFileSystemModel model;
+    QAbstractItemModelTester tester(&model);
+    tester.setUseFetchMore(false);
+    QVERIFY(model.iconProvider());
+    // No crash when setIconProvider(nullptr) is used
+    model.setIconProvider(nullptr);
+    const auto documentPaths = QStandardPaths::standardLocations(QStandardPaths::DocumentsLocation);
+    QVERIFY(!documentPaths.isEmpty());
+    model.setRootPath(documentPaths.constFirst());
+}
+
 bool tst_QFileSystemModel::createFiles(QFileSystemModel *model, const QString &test_path,
                                        const QStringList &initial_files, int existingFileCount,
                                        const QStringList &initial_dirs)

diff --git a/tests/manual/dialogs/filedialogpanel.cpp b/tests/manual/dialogs/filedialogpanel.cpp
@@ -86,6 +86,7 @@ FileDialogPanel::FileDialogPanel(QWidget *parent)
     , m_resolveSymLinks(new QCheckBox(tr("Resolve symlinks")))
     , m_native(new QCheckBox(tr("Use native dialog")))
     , m_customDirIcons(new QCheckBox(tr("Don't use custom directory icons")))
+    , m_noIconProvider(new QCheckBox(tr("Null icon provider")))
     , m_acceptMode(createCombo(this, acceptModeComboData, sizeof(acceptModeComboData)/sizeof(FlagData)))
     , m_fileMode(createCombo(this, fileModeComboData, sizeof(fileModeComboData)/sizeof(FlagData)))
     , m_viewMode(createCombo(this, viewModeComboData, sizeof(viewModeComboData)/sizeof(FlagData)))
@@ -113,6 +114,7 @@ FileDialogPanel::FileDialogPanel(QWidget *parent)
     optionsLayout->addRow(m_resolveSymLinks);
     optionsLayout->addRow(m_readOnly);
     optionsLayout->addRow(m_customDirIcons);
+    optionsLayout->addRow(m_noIconProvider);
 
     // Files
     QGroupBox *filesGroupBox = new QGroupBox(tr("Files / Filters"));
@@ -417,12 +419,19 @@ void FileDialogPanel::restoreDefaults()
         l->restoreDefault(&d);
 }
 
-void FileDialogPanel::applySettings(QFileDialog *d) const
+void FileDialogPanel::applySettings(QFileDialog *d)
 {
     d->setAcceptMode(comboBoxValue<QFileDialog::AcceptMode>(m_acceptMode));
     d->setViewMode(comboBoxValue<QFileDialog::ViewMode>(m_viewMode));
     d->setFileMode(comboBoxValue<QFileDialog::FileMode>(m_fileMode));
     d->setOptions(options());
+    if (m_noIconProvider->isChecked()) {
+        m_origIconProvider = d->iconProvider();
+        d->setIconProvider(nullptr);
+    } else if (m_origIconProvider) {
+        d->setIconProvider(m_origIconProvider);
+    }
+
     d->setDefaultSuffix(m_defaultSuffix->text().trimmed());
     const QString directory = m_directory->text().trimmed();
     if (!directory.isEmpty())

diff --git a/tests/manual/dialogs/filedialogpanel.h b/tests/manual/dialogs/filedialogpanel.h
@@ -9,6 +9,8 @@
 #include <QPointer>
 
 QT_BEGIN_NAMESPACE
+
+class QAbstractFileIconProvider;
 class QPushButton;
 class QCheckBox;
 class QComboBox;
@@ -52,7 +54,7 @@ private slots:
     QString filterString() const;
     QFileDialog::Options options() const;
     QStringList allowedSchemes() const;
-    void applySettings(QFileDialog *d) const;
+    void applySettings(QFileDialog *d);
 
     QFormLayout *filesLayout;
     QCheckBox *m_showDirsOnly;
@@ -62,6 +64,9 @@ private slots:
     QCheckBox *m_resolveSymLinks;
     QCheckBox *m_native;
     QCheckBox *m_customDirIcons;
+    QCheckBox *m_noIconProvider = nullptr;
+    QAbstractFileIconProvider *m_origIconProvider = nullptr;
+
     QComboBox *m_acceptMode;
     QComboBox *m_fileMode;
     QComboBox *m_viewMode;