1berry/qtbase
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

QDialogs: prevent crash in static get*() functions when parent gets deleted

Browse Source 
- As explained in
https://blogs.kde.org/2009/03/26/how-crash-almost-every-qtkde-application-and-how-fix-it-0
creating dialogs on the stack is a bad idea, if the
application or the dialog's parent window can be closed
by means other than user interaction (such as a timer or
an IPC call). Since we cannot know whether Qt is used to
build such an application, we must assume it is, create
the dialog on the heap, and monitor its lifetime with a
QPointer.

Long time ago instead of using manual resource management,
QAutoPointer was added to fix the crash problem for QInputDialog.

The same fix now is being applied for the rest of QDialog classes:
QColorDialog, QFileDialog, QFontDialog.

Documentation warnings about deleting the parent are not actual anymore. Deleted.

Pick-to: 6.5
Task-number: QTBUG-54693
Change-Id: Ib7cc0575ea25f392a295538e21de9015dc49ebe4
Reviewed-by: Ivan Solovev <ivan.solovev@qt.io>
(cherry picked from commit 81d55f772892445686862e49d0a6092668f8c725)
This commit is contained in:
Tatiana Borisova 2024-08-27 16:55:22 +02:00
parent 9ce6596394
commit ca3fa76a4e
7 changed files with 145 additions and 65 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

18
src/widgets/dialogs/qcolordialog.cpp
View File

@ -2197,13 +2197,19 @@ void QColorDialog::open(QObject *receiver, const char *member)
QColor QColorDialog::getColor(const QColor &initial, QWidget *parent, const QString &title,
ColorDialogOptions options)
{
QColorDialog dlg(parent);
QAutoPointer<QColorDialog> dlg(new QColorDialog(parent));
if (!title.isEmpty())
dlg.setWindowTitle(title);
dlg.setOptions(options);
dlg.setCurrentColor(initial);
dlg.exec();
return dlg.selectedColor();
dlg->setWindowTitle(title);
dlg->setOptions(options);
dlg->setCurrentColor(initial);
// If the dlg was deleted with a parent window,
// dlg == nullptr after leaving the exec().
dlg->exec();
if (bool(dlg))
return dlg->selectedColor();
else
return QColor();
}
/*!

66
src/widgets/dialogs/qfiledialog.cpp
View File

@ -2072,10 +2072,6 @@ QString QFileDialog::labelText(DialogLabel label) const
\a options includes DontResolveSymlinks, the file dialog treats
symlinks as regular directories.
\warning Do not delete \a parent during the execution of the dialog. If you
want to do this, you must create the dialog yourself using one of the
QFileDialog constructors.
\sa getOpenFileNames(), getSaveFileName(), getExistingDirectory()
*/
QString QFileDialog::getOpenFileName(QWidget *parent,
@ -2136,14 +2132,15 @@ QUrl QFileDialog::getOpenFileUrl(QWidget *parent,
args.mode = ExistingFile;
args.options = options;
QFileDialog dialog(args);
dialog.setSupportedSchemes(supportedSchemes);
QAutoPointer<QFileDialog> dialog(new QFileDialog(args));
dialog->setSupportedSchemes(supportedSchemes);
if (selectedFilter && !selectedFilter->isEmpty())
dialog.selectNameFilter(*selectedFilter);
if (dialog.exec() == QDialog::Accepted) {
dialog->selectNameFilter(*selectedFilter);
const int execResult = dialog->exec();
if (bool(dialog) && execResult == QDialog::Accepted) {
if (selectedFilter)
*selectedFilter = dialog.selectedNameFilter();
return dialog.selectedUrls().value(0);
*selectedFilter = dialog->selectedNameFilter();
return dialog->selectedUrls().value(0);
}
return QUrl();
}
@ -2185,10 +2182,6 @@ QUrl QFileDialog::getOpenFileUrl(QWidget *parent,
see the QFileDialog::Option enum for more information on the flags you can
pass.
\warning Do not delete \a parent during the execution of the dialog. If you
want to do this, you must create the dialog yourself using one of the
QFileDialog constructors.
\sa getOpenFileName(), getSaveFileName(), getExistingDirectory()
*/
QStringList QFileDialog::getOpenFileNames(QWidget *parent,
@ -2251,14 +2244,15 @@ QList<QUrl> QFileDialog::getOpenFileUrls(QWidget *parent,
args.mode = ExistingFiles;
args.options = options;
QFileDialog dialog(args);
dialog.setSupportedSchemes(supportedSchemes);
QAutoPointer<QFileDialog> dialog(new QFileDialog(args));
dialog->setSupportedSchemes(supportedSchemes);
if (selectedFilter && !selectedFilter->isEmpty())
dialog.selectNameFilter(*selectedFilter);
if (dialog.exec() == QDialog::Accepted) {
dialog->selectNameFilter(*selectedFilter);
const int execResult = dialog->exec();
if (bool(dialog) && execResult == QDialog::Accepted) {
if (selectedFilter)
*selectedFilter = dialog.selectedNameFilter();
return dialog.selectedUrls();
*selectedFilter = dialog->selectedNameFilter();
return dialog->selectedUrls();
}
return QList<QUrl>();
}
@ -2420,10 +2414,6 @@ void QFileDialog::saveFileContent(const QByteArray &fileContent, const QString &
\a options includes DontResolveSymlinks, the file dialog treats symlinks
as regular directories.
\warning Do not delete \a parent during the execution of the dialog. If you
want to do this, you must create the dialog yourself using one of the
QFileDialog constructors.
\sa getOpenFileName(), getOpenFileNames(), getExistingDirectory()
*/
QString QFileDialog::getSaveFileName(QWidget *parent,
@ -2484,15 +2474,16 @@ QUrl QFileDialog::getSaveFileUrl(QWidget *parent,
args.mode = AnyFile;
args.options = options;
QFileDialog dialog(args);
dialog.setSupportedSchemes(supportedSchemes);
dialog.setAcceptMode(AcceptSave);
QAutoPointer<QFileDialog> dialog(new QFileDialog(args));
dialog->setSupportedSchemes(supportedSchemes);
dialog->setAcceptMode(AcceptSave);
if (selectedFilter && !selectedFilter->isEmpty())
dialog.selectNameFilter(*selectedFilter);
if (dialog.exec() == QDialog::Accepted) {
dialog->selectNameFilter(*selectedFilter);
const int execResult = dialog->exec();
if (bool(dialog) && execResult == QDialog::Accepted) {
if (selectedFilter)
*selectedFilter = dialog.selectedNameFilter();
return dialog.selectedUrls().value(0);
*selectedFilter = dialog->selectedNameFilter();
return dialog->selectedUrls().value(0);
}
return QUrl();
}
@ -2535,10 +2526,6 @@ QUrl QFileDialog::getSaveFileUrl(QWidget *parent,
dispatch any QTimers, and if \a parent is not \nullptr then it positions
the dialog just below the parent's title bar.
\warning Do not delete \a parent during the execution of the dialog. If you
want to do this, you must create the dialog yourself using one of the
QFileDialog constructors.
\sa getOpenFileName(), getOpenFileNames(), getSaveFileName()
*/
QString QFileDialog::getExistingDirectory(QWidget *parent,
@ -2594,10 +2581,11 @@ QUrl QFileDialog::getExistingDirectoryUrl(QWidget *parent,
args.mode = Directory;
args.options = options;
QFileDialog dialog(args);
dialog.setSupportedSchemes(supportedSchemes);
if (dialog.exec() == QDialog::Accepted)
return dialog.selectedUrls().value(0);
QAutoPointer<QFileDialog> dialog(new QFileDialog(args));
dialog->setSupportedSchemes(supportedSchemes);
const int execResult = dialog->exec();
if (bool(dialog) && execResult == QDialog::Accepted)
return dialog->selectedUrls().value(0);
return QUrl();
}

22
src/widgets/dialogs/qfontdialog.cpp
View File

@ -326,10 +326,6 @@ QFontDialog::~QFontDialog()
\snippet code/src_gui_dialogs_qfontdialog.cpp 3
In this example, if the user clicks OK the font they chose will be
used, and if they click Cancel the original font is used.
\warning Do not delete \a parent during the execution of the dialog.
If you want to do this, you should create the dialog
yourself using one of the QFontDialog constructors.
*/
QFont QFontDialog::getFont(bool *ok, const QFont &initial, QWidget *parent, const QString &title,
FontDialogOptions options)
@ -352,10 +348,6 @@ QFont QFontDialog::getFont(bool *ok, const QFont &initial, QWidget *parent, cons
Example:
\snippet code/src_gui_dialogs_qfontdialog.cpp 4
\warning Do not delete \a parent during the execution of the dialog.
If you want to do this, you should create the dialog
yourself using one of the QFontDialog constructors.
*/
QFont QFontDialog::getFont(bool *ok, QWidget *parent)
{
@ -366,17 +358,17 @@ QFont QFontDialog::getFont(bool *ok, QWidget *parent)
QFont QFontDialogPrivate::getFont(bool *ok, const QFont &initial, QWidget *parent,
const QString &title, QFontDialog::FontDialogOptions options)
{
QFontDialog dlg(parent);
dlg.setOptions(options);
dlg.setCurrentFont(initial);
QAutoPointer<QFontDialog> dlg(new QFontDialog(parent));
dlg->setOptions(options);
dlg->setCurrentFont(initial);
if (!title.isEmpty())
dlg.setWindowTitle(title);
dlg->setWindowTitle(title);
int ret = (dlg.exec() || (options & QFontDialog::NoButtons));
int ret = (dlg->exec() || (options & QFontDialog::NoButtons));
if (ok)
*ok = !!ret;
if (ret) {
return dlg.selectedFont();
if (ret && bool(dlg)) {
return dlg->selectedFont();
} else {
return initial;
}

10
src/widgets/dialogs/qinputdialog.cpp
View File

@ -1149,7 +1149,7 @@ QString QInputDialog::getText(QWidget *parent, const QString &title, const QStri
const int ret = dialog->exec();
if (ok)
*ok = !!ret;
if (ret) {
if (bool(dialog) && ret) {
return dialog->textValue();
} else {
return QString();
@ -1197,7 +1197,7 @@ QString QInputDialog::getMultiLineText(QWidget *parent, const QString &title, co
const int ret = dialog->exec();
if (ok)
*ok = !!ret;
if (ret) {
if (bool(dialog) && ret) {
return dialog->textValue();
} else {
return QString();
@ -1242,7 +1242,7 @@ int QInputDialog::getInt(QWidget *parent, const QString &title, const QString &l
const int ret = dialog->exec();
if (ok)
*ok = !!ret;
if (ret) {
if (bool(dialog) && ret) {
return dialog->intValue();
} else {
return value;
@ -1291,7 +1291,7 @@ double QInputDialog::getDouble(QWidget *parent, const QString &title, const QStr
const int ret = dialog->exec();
if (ok)
*ok = !!ret;
if (ret) {
if (bool(dialog) && ret) {
return dialog->doubleValue();
} else {
return value;
@ -1344,7 +1344,7 @@ QString QInputDialog::getItem(QWidget *parent, const QString &title, const QStri
const int ret = dialog->exec();
if (ok)
*ok = !!ret;
if (ret) {
if (bool(dialog) && ret) {
return dialog->textValue();
} else {
return text;

13
tests/auto/widgets/dialogs/qcolordialog/tst_qcolordialog.cpp
View File

@ -27,6 +27,7 @@ private slots:
void native_activeModalWidget();
void task247349_alpha();
void QTBUG_43548_initialColor();
void noCrashWhenParentIsDeleted();
void hexColor_data();
void hexColor();
@ -132,6 +133,18 @@ void tst_QColorDialog::QTBUG_43548_initialColor()
QCOMPARE(a, dialog.currentColor());
}
void tst_QColorDialog::noCrashWhenParentIsDeleted()
{
QPointer<QWidget> mainWindow = new QWidget();
QTimer::singleShot(1000, mainWindow, [mainWindow]
{ if (mainWindow.get()) mainWindow->deleteLater(); });
const QColor color = QColorDialog::getColor(QColor::fromRgba(0xffffffff), mainWindow.get(),
QString(), QColorDialog::DontUseNativeDialog);
QVERIFY(!color.isValid());
QVERIFY(!mainWindow.get());
}
void tst_QColorDialog::hexColor_data()
{
QTest::addColumn<const QString>("colorString");

49
tests/auto/widgets/dialogs/qfiledialog2/tst_qfiledialog2.cpp
View File

@ -25,6 +25,7 @@
#include <qlayout.h>
#include <qmenu.h>
#include <qrandom.h>
#include <qpointer.h>
#include "../../../../../src/widgets/dialogs/qsidebar_p.h"
#include "../../../../../src/gui/itemmodels/qfilesystemmodel_p.h"
#include "../../../../../src/widgets/dialogs/qfiledialog_p.h"
@ -104,6 +105,7 @@ private slots:
void QTBUG4419_lineEditSelectAll();
void QTBUG6558_showDirsOnly();
void QTBUG4842_selectFilterWithHideNameFilterDetails();
void noCrashWhenParentIsDeleted();
void dontShowCompleterOnRoot();
void nameFilterParsing_data();
void nameFilterParsing();
@ -1333,6 +1335,53 @@ void tst_QFileDialog2::QTBUG4842_selectFilterWithHideNameFilterDetails()
}
void tst_QFileDialog2::noCrashWhenParentIsDeleted()
{
{
QPointer<QWidget> mainWindow = new QWidget();
QTimer::singleShot(1000, mainWindow, [mainWindow]
{ if (mainWindow.get()) mainWindow->deleteLater(); });
const QUrl url = QFileDialog::getOpenFileUrl(mainWindow.get(),
QStringLiteral("getOpenFileUrl"));
QVERIFY(url.isEmpty());
QVERIFY(!url.isValid());
QVERIFY(!mainWindow.get());
}
{
QPointer<QWidget> mainWindow = new QWidget();
QTimer::singleShot(1000, mainWindow, [mainWindow]
{ if (mainWindow.get()) mainWindow->deleteLater(); });
const QUrl url = QFileDialog::getSaveFileUrl(mainWindow.get(),
QStringLiteral("getSaveFileUrl"));
QVERIFY(url.isEmpty());
QVERIFY(!url.isValid());
QVERIFY(!mainWindow.get());
}
{
QPointer<QWidget> mainWindow = new QWidget();
QTimer::singleShot(1000, mainWindow, [mainWindow]
{ if (mainWindow.get()) mainWindow->deleteLater(); });
const QUrl url
= QFileDialog::getExistingDirectoryUrl(mainWindow.get(),
QStringLiteral("getExistingDirectoryUrl"));
QVERIFY(url.isEmpty());
QVERIFY(!url.isValid());
QVERIFY(!mainWindow.get());
}
{
QPointer<QWidget> mainWindow = new QWidget();
QTimer::singleShot(1000, mainWindow, [mainWindow]
{ if (mainWindow.get()) mainWindow->deleteLater(); });
const QList<QUrl> url = QFileDialog::getOpenFileUrls(mainWindow.get(),
QStringLiteral("getOpenFileUrls"));
QVERIFY(url.isEmpty());
QVERIFY(!mainWindow.get());
}
}
void tst_QFileDialog2::dontShowCompleterOnRoot()
{
if (!QGuiApplicationPrivate::platformIntegration()->hasCapability(QPlatformIntegration::WindowActivation))

32
tests/auto/widgets/dialogs/qfontdialog/tst_qfontdialog.cpp
View File

@ -44,6 +44,7 @@ private slots:
#ifndef QT_NO_STYLE_STYLESHEET
void qtbug_41513_stylesheetStyle();
#endif
void noCrashWhenParentIsDeleted();
void hideNativeByDestruction();
@ -211,6 +212,37 @@ void tst_QFontDialog::qtbug_41513_stylesheetStyle()
}
#endif // QT_NO_STYLE_STYLESHEET
void tst_QFontDialog::noCrashWhenParentIsDeleted()
{
{
QPointer<QWidget> mainWindow = new QWidget();
QTimer::singleShot(1000, mainWindow, [mainWindow]
{ if (mainWindow.get()) mainWindow->deleteLater(); });
bool accepted = false;
const QFont testFont = QFont(QStringLiteral("QtsSpecialTestFont1"));
QFontDialog::getFont(&accepted, testFont,
mainWindow.get(),
QLatin1String("QFontDialog - crash parent test"),
QFontDialog::DontUseNativeDialog);
QVERIFY(!accepted);
QVERIFY(!mainWindow.get());
}
{
QPointer<QWidget> mainWindow = new QWidget();
QTimer::singleShot(1000, mainWindow, [mainWindow]
{ if (mainWindow.get()) mainWindow->deleteLater(); });
bool accepted = false;
const QFont testFont = QFont(QStringLiteral("QtsSpecialTestFont2"));
QFontDialog::getFont(&accepted, testFont,
mainWindow.get(),
QLatin1String("QFontDialog - crash parent test"),
QFontDialog::NoButtons | QFontDialog::DontUseNativeDialog);
QVERIFY(accepted);
QVERIFY(!mainWindow.get());
}
}
void tst_QFontDialog::testNonStandardFontSize()
{
QList<int> standardSizesList = QFontDatabase::standardSizes();