From a26119b92067130d1dccd9b2834d75ceee5ade48 Mon Sep 17 00:00:00 2001 From: Eike Ziller Date: Mon, 30 Sep 2024 14:51:21 +0200 Subject: [PATCH] Crashpad: Fix issues with reports path macOS - if the settings path did not exist yet (first start), it would fail to create it - the path that was created and used was directly `.../QtProject/crashpad_reports`, which is not specific to the application (QtC or QDS), and wasn't consistent with the path returned by ICore::crashReportsPath, which was `.../QtProject//crashpad_reports` (which is a better path) other - always use the user resource directory as the base for crash reports, the installation directory might not be writable, and it might not belong to the user, which means that crash reports that potentially contain private data could be exposed to other users The patch simplifies the synchronization between main.cpp and ICore by putting the corresponding data into AppInfo. That implies that the user resource path is now created early enough, because it is created when setting up the AppInfo. Change-Id: Ic1293e515279aa971b02c54bc03abd98633b58cb Reviewed-by: Tim Jenssen Reviewed-by: Cristian Adam --- src/app/main.cpp | 69 ++++++++----------- src/libs/utils/appinfo.h | 3 + src/plugins/coreplugin/icore.cpp | 5 +- src/plugins/coreplugin/systemsettings.cpp | 43 ++++++------ src/tools/qml2puppet/qml2puppet/qmlpuppet.cpp | 8 +-- 5 files changed, 54 insertions(+), 74 deletions(-) diff --git a/src/app/main.cpp b/src/app/main.cpp index a4c011663db..9da6030f5e5 100644 --- a/src/app/main.cpp +++ b/src/app/main.cpp @@ -409,26 +409,14 @@ QStringList lastSessionArgument() return hasProjectExplorer ? QStringList({"-lastsession"}) : QStringList(); } -// should be in sync with src/plugins/coreplugin/icore.cpp -> FilePath ICore::crashReportsPath() -// and src\tools\qml2puppet\qml2puppet\qmlpuppet.cpp -> QString crashReportsPath() -QString crashReportsPath() -{ - std::unique_ptr settings(createUserSettings()); - if (HostOsInfo::isMacHost()) - return QFileInfo(settings->fileName()).path() + "/crashpad_reports"; - else - return QCoreApplication::applicationDirPath() - + '/' + RELATIVE_LIBEXEC_PATH + "crashpad_reports"; -} - #ifdef ENABLE_CRASHPAD -bool startCrashpad(const QString &libexecPath, bool crashReportingEnabled) +bool startCrashpad(const AppInfo &appInfo, bool crashReportingEnabled) { using namespace crashpad; // Cache directory that will store crashpad information and minidumps - QString databasePath = QDir::cleanPath(crashReportsPath()); - QString handlerPath = QDir::cleanPath(libexecPath + "/crashpad_handler"); + const QString databasePath = appInfo.crashReports.path(); + const QString handlerPath = (appInfo.libexec / "crashpad_handler").path(); #ifdef Q_OS_WIN handlerPath += ".exe"; base::FilePath database(databasePath.toStdWString()); @@ -716,17 +704,35 @@ int main(int argc, char **argv) const int threadCount = QThreadPool::globalInstance()->maxThreadCount(); QThreadPool::globalInstance()->setMaxThreadCount(qMax(4, 2 * threadCount)); - const QString libexecPath = QCoreApplication::applicationDirPath() - + '/' + RELATIVE_LIBEXEC_PATH; + using namespace Core; + const FilePath appDirPath = FilePath::fromUserInput(QApplication::applicationDirPath()); + AppInfo info; + info.author = Constants::IDE_AUTHOR; + info.copyright = Constants::IDE_COPYRIGHT; + info.displayVersion = Constants::IDE_VERSION_DISPLAY; + info.id = Constants::IDE_ID; + info.revision = Constants::IDE_REVISION_STR; + info.revisionUrl = Constants::IDE_REVISION_URL; + info.userFileExtension = Constants::IDE_PROJECT_USER_FILE_EXTENSION; + info.plugins = (appDirPath / RELATIVE_PLUGIN_PATH).cleanPath(); + info.userPluginsRoot = userPluginsRoot(); + info.resources = (appDirPath / RELATIVE_DATA_PATH).cleanPath(); + info.userResources = userResourcePath(settings->fileName(), Constants::IDE_ID); + info.libexec = (appDirPath / RELATIVE_LIBEXEC_PATH).cleanPath(); + // sync with src\tools\qml2puppet\qml2puppet\qmlpuppet.cpp -> QString crashReportsPath() + info.crashReports = info.userResources / "crashpad_reports"; + info.luaPlugins = info.resources / "lua-plugins"; + info.userLuaPlugins = info.userResources / "lua-plugins"; + Utils::Internal::setAppInfo(info); // Display a backtrace once a serious signal is delivered (Linux only). - CrashHandlerSetup setupCrashHandler(Core::Constants::IDE_DISPLAY_NAME, - CrashHandlerSetup::EnableRestart, - libexecPath); + CrashHandlerSetup setupCrashHandler( + Core::Constants::IDE_DISPLAY_NAME, CrashHandlerSetup::EnableRestart, info.libexec.path()); #ifdef ENABLE_CRASHPAD + // depends on AppInfo and QApplication being created bool crashReportingEnabled = settings->value("CrashReportingEnabled", false).toBool(); - startCrashpad(libexecPath, crashReportingEnabled); + startCrashpad(info, crashReportingEnabled); #endif PluginManager pluginManager; @@ -737,27 +743,6 @@ int main(int argc, char **argv) BaseAspect::setQtcSettings(settings); - using namespace Core; - AppInfo info; - info.author = Constants::IDE_AUTHOR; - info.copyright = Constants::IDE_COPYRIGHT; - info.displayVersion = Constants::IDE_VERSION_DISPLAY; - info.id = Constants::IDE_ID; - info.revision = Constants::IDE_REVISION_STR; - info.revisionUrl = Constants::IDE_REVISION_URL; - info.userFileExtension = Constants::IDE_PROJECT_USER_FILE_EXTENSION; - - const FilePath appDirPath = FilePath::fromUserInput(QApplication::applicationDirPath()); - - info.plugins = (appDirPath / RELATIVE_PLUGIN_PATH).cleanPath(); - info.userPluginsRoot = userPluginsRoot(); - info.resources = (appDirPath / RELATIVE_DATA_PATH).cleanPath(); - info.userResources = userResourcePath(settings->fileName(), Constants::IDE_ID); - info.luaPlugins = info.resources / "lua-plugins"; - info.userLuaPlugins = info.userResources / "lua-plugins"; - - Utils::Internal::setAppInfo(info); - QTranslator translator; QTranslator qtTranslator; QStringList uiLanguages = QLocale::system().uiLanguages(); diff --git a/src/libs/utils/appinfo.h b/src/libs/utils/appinfo.h index 268e26c8804..128092531ba 100644 --- a/src/libs/utils/appinfo.h +++ b/src/libs/utils/appinfo.h @@ -37,6 +37,9 @@ public: FilePath resources; FilePath userResources; + FilePath crashReports; + + FilePath libexec; }; QTCREATOR_UTILS_EXPORT const AppInfo &appInfo(); diff --git a/src/plugins/coreplugin/icore.cpp b/src/plugins/coreplugin/icore.cpp index 53bec274faa..3b1535201ef 100644 --- a/src/plugins/coreplugin/icore.cpp +++ b/src/plugins/coreplugin/icore.cpp @@ -660,10 +660,7 @@ FilePath ICore::libexecPath(const QString &rel) FilePath ICore::crashReportsPath() { - if (Utils::HostOsInfo::isMacHost()) - return Core::ICore::userResourcePath("crashpad_reports/completed"); - else - return libexecPath("crashpad_reports/reports"); + return appInfo().crashReports; } static QString clangIncludePath(const QString &clangVersion) diff --git a/src/plugins/coreplugin/systemsettings.cpp b/src/plugins/coreplugin/systemsettings.cpp index d928eeeec6f..6ac1fd9697b 100644 --- a/src/plugins/coreplugin/systemsettings.cpp +++ b/src/plugins/coreplugin/systemsettings.cpp @@ -277,13 +277,28 @@ public: }); connect(&s.enableCrashReporting, &BaseAspect::changed, this, &SystemSettingsWidget::apply); - updateClearCrashWidgets(); - connect(m_clearCrashReportsButton, &QPushButton::clicked, this, [&] { - const FilePaths &crashFiles = ICore::crashReportsPath().dirEntries(QDir::Files); + const FilePath reportsPath = ICore::crashReportsPath() + / QLatin1String( + HostOsInfo::isMacHost() ? "completed" : "reports"); + const auto updateClearCrashWidgets = [this, reportsPath] { + qint64 size = 0; + const FilePaths crashFiles = reportsPath.dirEntries(QDir::Files); for (const FilePath &file : crashFiles) - file.removeFile(); - updateClearCrashWidgets(); - }); + size += file.fileSize(); + m_clearCrashReportsButton->setEnabled(!crashFiles.isEmpty()); + m_crashReportsSizeText->setText(formatSize(size)); + }; + updateClearCrashWidgets(); + connect( + m_clearCrashReportsButton, + &QPushButton::clicked, + this, + [updateClearCrashWidgets, reportsPath] { + const FilePaths &crashFiles = reportsPath.dirEntries(QDir::Files); + for (const FilePath &file : crashFiles) + file.removeFile(); + updateClearCrashWidgets(); + }); #endif if (HostOsInfo::isAnyUnixHost()) { @@ -356,8 +371,6 @@ private: void updateTerminalUi(const Utils::TerminalCommand &term); void updatePath(); void updateEnvironmentChangesLabel(); - void updateClearCrashWidgets(); - void showHelpDialog(const QString &title, const QString &helpText); QComboBox *m_fileSystemCaseSensitivityChooser; @@ -459,20 +472,6 @@ void SystemSettingsWidget::showHelpDialog(const QString &title, const QString &h mb->show(); } -#ifdef ENABLE_CRASHPAD -void SystemSettingsWidget::updateClearCrashWidgets() -{ - QDir crashReportsDir(ICore::crashReportsPath().path()); - crashReportsDir.setFilter(QDir::Files); - qint64 size = 0; - const FilePaths crashFiles = ICore::crashReportsPath().dirEntries(QDir::Files); - for (const FilePath &file : crashFiles) - size += file.fileSize(); - m_clearCrashReportsButton->setEnabled(!crashFiles.isEmpty()); - m_crashReportsSizeText->setText(formatSize(size)); -} -#endif - void SystemSettingsWidget::showHelpForFileBrowser() { if (HostOsInfo::isAnyUnixHost() && !HostOsInfo::isMacHost()) diff --git a/src/tools/qml2puppet/qml2puppet/qmlpuppet.cpp b/src/tools/qml2puppet/qml2puppet/qmlpuppet.cpp index 345e2c4cf74..7be35fa0487 100644 --- a/src/tools/qml2puppet/qml2puppet/qmlpuppet.cpp +++ b/src/tools/qml2puppet/qml2puppet/qmlpuppet.cpp @@ -84,12 +84,8 @@ QString crashReportsPath() QLatin1String(Core::Constants::IDE_SETTINGSVARIANT_STR), QLatin1String(Core::Constants::IDE_CASED_ID)); -#if defined(Q_OS_MACOS) - return QFileInfo(settings.fileName()).path() + "/crashpad_reports"; -#else - return QCoreApplication::applicationDirPath() - + '/' + RELATIVE_LIBEXEC_PATH + "crashpad_reports"; -#endif + return QFileInfo(settings.fileName()).path() + "/" + Core::Constants::IDE_ID + + "/crashpad_reports"; } void QmlPuppet::initQmlRunner()