GH-1389 wrap QDesktopServices and QProcess::startDetached

Essentially do not pass some environment variables to subprocesses:
* LD_PRELOAD
* LD_LIBRARY_PATH
* LD_DEBUG
* QT_PLUGIN_PATH
* QT_FONTPATH

application/GuiUtil.cpp

@@ -1,7 +1,6 @@
 #include "GuiUtil.h"
 
 #include <QClipboard>
-#include <QDesktopServices>
 #include <QApplication>
 #include <QFileDialog>
 
@@ -11,6 +10,7 @@
 
 #include "MultiMC.h"
 #include <settings/SettingsObject.h>
+#include <DesktopServices.h>
 #include <BuildConfig.h>
 
 void GuiUtil::uploadPaste(const QString &text, QWidget *parentWidget)
@@ -42,7 +42,7 @@ void GuiUtil::uploadPaste(const QString &text, QWidget *parentWidget)
 	{
 		const QString link = paste->pasteLink();
 		setClipboardText(link);
-		QDesktopServices::openUrl(link);
+		DesktopServices::openUrl(link);
 		CustomMessageBox::selectable(
 			parentWidget, QObject::tr("Upload finished"),
 			QObject::tr("The <a href=\"%1\">link to the uploaded log</a> has been opened in "

application/MainWindow.cpp

@@ -26,7 +26,6 @@
 #include <QtCore/QDir>
 #include <QtCore/QFileInfo>
 
-#include <QtGui/QDesktopServices>
 #include <QtGui/QKeyEvent>
 
 #include <QtWidgets/QAction>
@@ -69,6 +68,7 @@
 #include <tools/BaseProfiler.h>
 #include <updater/DownloadTask.h>
 #include <updater/UpdateChecker.h>
+#include <DesktopServices.h>
 
 #include "InstancePageProvider.h"
 #include "InstanceProxyModel.h"
@@ -1197,7 +1197,7 @@ void MainWindow::on_actionAddInstance_triggered()
 
 void MainWindow::on_actionREDDIT_triggered()
 {
-	openWebPage(QUrl("https://www.reddit.com/r/MultiMC/"));
+	DesktopServices::openUrl(QUrl("https://www.reddit.com/r/MultiMC/"));
 }
 
 void MainWindow::on_actionCopyInstance_triggered()
@@ -1312,7 +1312,7 @@ void MainWindow::on_actionChangeInstGroup_triggered()
 void MainWindow::on_actionViewInstanceFolder_triggered()
 {
 	QString str = MMC->settings()->get("InstanceDir").toString();
-	FS::openDirInDefaultProgram(str);
+	DesktopServices::openDirectory(str);
 }
 
 void MainWindow::on_actionRefresh_triggered()
@@ -1322,7 +1322,7 @@ void MainWindow::on_actionRefresh_triggered()
 
 void MainWindow::on_actionViewCentralModsFolder_triggered()
 {
-	FS::openDirInDefaultProgram(MMC->settings()->get("CentralModsDir").toString(), true);
+	DesktopServices::openDirectory(MMC->settings()->get("CentralModsDir").toString(), true);
 }
 
 void MainWindow::on_actionConfig_Folder_triggered()
@@ -1330,7 +1330,7 @@ void MainWindow::on_actionConfig_Folder_triggered()
 	if (m_selectedInstance)
 	{
 		QString str = m_selectedInstance->instanceConfigFolder();
-		FS::openDirInDefaultProgram(QDir(str).absolutePath());
+		DesktopServices::openDirectory(QDir(str).absolutePath());
 	}
 }
 
@@ -1384,26 +1384,30 @@ void MainWindow::on_actionManageAccounts_triggered()
 
 void MainWindow::on_actionReportBug_triggered()
 {
-	openWebPage(QUrl("https://github.com/MultiMC/MultiMC5/issues"));
+	DesktopServices::openUrl(QUrl("https://github.com/MultiMC/MultiMC5/issues"));
 }
 
 void MainWindow::on_actionPatreon_triggered()
 {
-	openWebPage(QUrl("http://www.patreon.com/multimc"));
+	DesktopServices::openUrl(QUrl("http://www.patreon.com/multimc"));
 }
 
 void MainWindow::on_actionMoreNews_triggered()
 {
-	openWebPage(QUrl("http://multimc.org/posts.html"));
+	DesktopServices::openUrl(QUrl("http://multimc.org/posts.html"));
 }
 
 void MainWindow::newsButtonClicked()
 {
 	QList<NewsEntryPtr> entries = m_newsChecker->getNewsEntries();
 	if (entries.count() > 0)
-		openWebPage(QUrl(entries[0]->link));
+	{
+		DesktopServices::openUrl(QUrl(entries[0]->link));
+	}
 	else
-		openWebPage(QUrl("http://multimc.org/posts.html"));
+	{
+		DesktopServices::openUrl(QUrl("http://multimc.org/posts.html"));
+	}
 }
 
 void MainWindow::on_actionAbout_triggered()
@@ -1469,7 +1473,7 @@ void MainWindow::on_actionViewSelectedInstFolder_triggered()
 	if (m_selectedInstance)
 	{
 		QString str = m_selectedInstance->instanceRoot();
-		FS::openDirInDefaultProgram(QDir(str).absolutePath());
+		DesktopServices::openDirectory(QDir(str).absolutePath());
 	}
 }
 
@@ -1538,12 +1542,6 @@ void MainWindow::startTask(Task *task)
 	task->start();
 }
 
-// BrowserDialog
-void MainWindow::openWebPage(QUrl url)
-{
-	QDesktopServices::openUrl(url);
-}
-
 void MainWindow::instanceChanged(const QModelIndex &current, const QModelIndex &previous)
 {
 	if (!current.isValid())

application/MainWindow.h

@@ -166,9 +166,6 @@ private:
 	void finalizeInstance(InstancePtr inst);
 	void launch(InstancePtr instance, bool online = true, BaseProfilerFactory *profiler = nullptr);
 
-	/// open a web page in the default browser
-	void openWebPage(QUrl url);
-
 private:
 	std::unique_ptr<Ui> ui;
 

application/MultiMC.cpp

@@ -17,7 +17,6 @@
 #include <QLibraryInfo>
 #include <QMessageBox>
 #include <QStringList>
-#include <QDesktopServices>
 #include <QDebug>
 
 #include "InstanceList.h"
@@ -54,6 +53,7 @@
 
 #include <Commandline.h>
 #include <FileSystem.h>
+#include <DesktopServices.h>
 
 using namespace Commandline;
 
@@ -932,12 +932,12 @@ bool MultiMC::openJsonEditor(const QString &filename)
 	const QString file = QDir::current().absoluteFilePath(filename);
 	if (m_settings->get("JsonEditor").toString().isEmpty())
 	{
-		return QDesktopServices::openUrl(QUrl::fromLocalFile(file));
+		return DesktopServices::openUrl(QUrl::fromLocalFile(file));
 	}
 	else
 	{
-		return QProcess::startDetached(m_settings->get("JsonEditor").toString(), QStringList()
-																					 << file);
+		//return DesktopServices::openFile(m_settings->get("JsonEditor").toString(), file);
+		return DesktopServices::run(m_settings->get("JsonEditor").toString(), {});
 	}
 }
 

application/dialogs/EditAccountDialog.cpp

@@ -15,7 +15,7 @@
 
 #include "EditAccountDialog.h"
 #include "ui_EditAccountDialog.h"
-#include <QDesktopServices>
+#include <DesktopServices.h>
 #include <QUrl>
 
 EditAccountDialog::EditAccountDialog(const QString &text, QWidget *parent, int flags)
@@ -37,7 +37,7 @@ EditAccountDialog::~EditAccountDialog()
 
 void EditAccountDialog::on_label_linkActivated(const QString &link)
 {
-	QDesktopServices::openUrl(QUrl(link));
+	DesktopServices::openUrl(QUrl(link));
 }
 
 void EditAccountDialog::setUsername(const QString & user) const

application/dialogs/ModEditDialogCommon.cpp

@@ -28,7 +28,7 @@ void showWebsiteForMod(QWidget *parentDlg, Mod &m)
 		{
 			url = "http://" + url;
 		}
-		QDesktopServices::openUrl(url);
+		DesktopServices::openUrl(url);
 	}
 	else
 	{

application/dialogs/ModEditDialogCommon.h

@@ -1,6 +1,6 @@
 #pragma once
 #include <QModelIndex>
-#include <QDesktopServices>
+#include <DesktopServices.h>
 #include <QWidget>
 #include <minecraft/Mod.h>
 

application/pages/LegacyJarModPage.cpp

@@ -25,7 +25,7 @@
 #include "minecraft/ModList.h"
 #include "minecraft/LegacyInstance.h"
 #include "Env.h"
-#include <FileSystem.h>
+#include <DesktopServices.h>
 #include "MultiMC.h"
 #include <GuiUtil.h>
 
@@ -146,7 +146,7 @@ void LegacyJarModPage::on_rmJarBtn_clicked()
 
 void LegacyJarModPage::on_viewJarBtn_clicked()
 {
-	FS::openDirInDefaultProgram(m_inst->jarModsDir(), true);
+	DesktopServices::openDirectory(m_inst->jarModsDir(), true);
 }
 
 void LegacyJarModPage::jarCurrent(QModelIndex current, QModelIndex previous)

application/pages/ModFolderPage.cpp

@@ -19,7 +19,6 @@
 #include <QMessageBox>
 #include <QEvent>
 #include <QKeyEvent>
-#include <QDesktopServices>
 #include <QAbstractItemModel>
 
 #include "MultiMC.h"
@@ -29,7 +28,7 @@
 #include "minecraft/ModList.h"
 #include "minecraft/Mod.h"
 #include "minecraft/VersionFilterData.h"
-#include <FileSystem.h>
+#include <DesktopServices.h>
 
 ModFolderPage::ModFolderPage(BaseInstance *inst, std::shared_ptr<ModList> mods, QString id,
 							 QString iconName, QString displayName, QString helpPage,
@@ -162,7 +161,7 @@ void ModFolderPage::on_rmModBtn_clicked()
 
 void ModFolderPage::on_viewModBtn_clicked()
 {
-	FS::openDirInDefaultProgram(m_mods->dir().absolutePath(), true);
+	DesktopServices::openDirectory(m_mods->dir().absolutePath(), true);
 }
 
 void ModFolderPage::modCurrent(const QModelIndex &current, const QModelIndex &previous)

application/pages/ScreenshotsPage.cpp

@@ -12,7 +12,6 @@
 #include <QEvent>
 #include <QPainter>
 #include <QClipboard>
-#include <QDesktopServices>
 #include <QKeyEvent>
 
 #include <MultiMC.h>
@@ -26,6 +25,7 @@
 
 #include "RWStorage.h"
 #include <FileSystem.h>
+#include <DesktopServices.h>
 
 typedef RWStorage<QString, QIcon> SharedIconCache;
 typedef std::shared_ptr<SharedIconCache> SharedIconCachePtr;
@@ -271,12 +271,12 @@ void ScreenshotsPage::onItemActivated(QModelIndex index)
 		return;
 	auto info = m_model->fileInfo(index);
 	QString fileName = info.absoluteFilePath();
-	FS::openFileInDefaultProgram(info.absoluteFilePath());
+	DesktopServices::openFile(info.absoluteFilePath());
 }
 
 void ScreenshotsPage::on_viewFolderBtn_clicked()
 {
-	FS::openDirInDefaultProgram(m_folder, true);
+	DesktopServices::openDirectory(m_folder, true);
 }
 
 void ScreenshotsPage::on_uploadBtn_clicked()
@@ -312,7 +312,7 @@ void ScreenshotsPage::on_uploadBtn_clicked()
 		auto link = QString("https://imgur.com/a/%1").arg(imgurAlbum->id());
 		QClipboard *clipboard = QApplication::clipboard();
 		clipboard->setText(link);
-		QDesktopServices::openUrl(link);
+		DesktopServices::openUrl(link);
 		CustomMessageBox::selectable(
 			this, tr("Upload finished"),
 			tr("The <a href=\"%1\">link  to the uploaded album</a> has been opened in the "

application/pages/WorldListPage.cpp

@@ -16,7 +16,7 @@
 #include "WorldListPage.h"
 #include "ui_WorldListPage.h"
 #include "minecraft/WorldList.h"
-#include <FileSystem.h>
+#include <DesktopServices.h>
 #include "dialogs/ModEditDialogCommon.h"
 #include <QEvent>
 #include <QKeyEvent>
@@ -123,7 +123,7 @@ void WorldListPage::on_rmWorldBtn_clicked()
 
 void WorldListPage::on_viewFolderBtn_clicked()
 {
-	FS::openDirInDefaultProgram(m_worlds->dir().absolutePath(), true);
+	DesktopServices::openDirectory(m_worlds->dir().absolutePath(), true);
 }
 
 QModelIndex WorldListPage::getSelectedWorld()
@@ -193,7 +193,8 @@ void WorldListPage::on_mcEditBtn_clicked()
 	if(program.size())
 	{
 		qint64 pid = 0;
-		QProcess::startDetached(program, QStringList() << fullPath, mceditPath, &pid);
+
+		DesktopServices::openFile(program, fullPath, mceditPath, &pid);
 		if(pid == 0)
 		{
 			QMessageBox::warning(this->parentWidget(), tr("MCEdit failed to start!"), tr("MCEdit failed to start.\nIt may be necessary to reinstall it."));

application/widgets/PageContainer.cpp

@@ -25,13 +25,13 @@
 #include <QLabel>
 #include <QDialogButtonBox>
 #include <QGridLayout>
-#include <QDesktopServices>
 
 #include "MultiMC.h"
 #include "settings/SettingsObject.h"
 #include "widgets/IconLabel.h"
 #include "PageContainer_p.h"
 #include <MultiMC.h>
+#include <DesktopServices.h>
 
 class PageEntryFilterModel : public QSortFilterProxyModel
 {
@@ -195,7 +195,7 @@ void PageContainer::help()
 		QString pageId = m_currentPage->helpPage();
 		if (pageId.isEmpty())
 			return;
-		QDesktopServices::openUrl(QUrl("https://github.com/MultiMC/MultiMC5/wiki/" + pageId));
+		DesktopServices::openUrl(QUrl("https://github.com/MultiMC/MultiMC5/wiki/" + pageId));
 	}
 }
 

application/widgets/ServerStatus.cpp

@@ -2,6 +2,7 @@
 #include "LineSeparator.h"
 #include "IconLabel.h"
 #include "status/StatusChecker.h"
+#include <DesktopServices.h>
 
 #include "MultiMC.h"
 
@@ -11,7 +12,6 @@
 #include <QMap>
 #include <QToolButton>
 #include <QAction>
-#include <QDesktopServices>
 
 class ClickableLabel : public QLabel
 {
@@ -127,7 +127,7 @@ void ServerStatus::addStatus(QString key, QString name)
 
 void ServerStatus::clicked()
 {
-	QDesktopServices::openUrl(QUrl("https://help.mojang.com/"));
+	DesktopServices::openUrl(QUrl("https://help.mojang.com/"));
 }
 
 void ServerStatus::setStatus(QString key, int value)

logic/CMakeLists.txt

@@ -32,8 +32,12 @@ set(LOGIC_SOURCES
 	# JSON parsing helpers
 	Json.h
 	Json.cpp
+
 	FileSystem.h
 	FileSystem.cpp
+	DesktopServices.h
+	DesktopServices.cpp
+
 	Exception.h
 
 	# RW lock protected map

logic/DesktopServices.cpp

@@ -0,0 +1,149 @@
+#include "DesktopServices.h"
+#include <QDir>
+#include <QDesktopServices>
+#include <QProcess>
+#include <QDebug>
+
+/**
+ * This shouldn't exist, but until QTBUG-9328 and other unreported bugs are fixed, it needs to be a thing.
+ */
+#if defined(Q_OS_LINUX)
+
+#include <unistd.h>
+#include <errno.h>
+#include <sys/types.h>
+#include <sys/wait.h>
+
+template <typename T>
+bool IndirectOpen(T callable, qint64 *pid_forked = nullptr)
+{
+	auto pid = fork();
+	if(pid_forked)
+	{
+		if(pid > 0)
+			*pid_forked = pid;
+		else
+			*pid_forked = 0;
+	}
+	if(pid == -1)
+	{
+		qWarning() << "IndirectOpen failed to fork: " << errno;
+		return false;
+	}
+	// child - do the stuff
+	if(pid == 0)
+	{
+		// unset all this garbage so it doesn't get passed to the child process
+		qunsetenv("LD_PRELOAD");
+		qunsetenv("LD_LIBRARY_PATH");
+		qunsetenv("LD_DEBUG");
+		qunsetenv("QT_PLUGIN_PATH");
+		qunsetenv("QT_FONTPATH");
+
+		// open the URL
+		auto status = callable();
+
+		// detach from the parent process group.
+		setsid();
+
+		// die. now. do not clean up anything, it would just hang forever.
+		_exit(status ? 0 : 1);
+	}
+	else
+	{
+		//parent - assume it worked.
+		int status;
+		while (waitpid(pid, &status, 0))
+		{
+			if(WIFEXITED(status))
+			{
+				return WEXITSTATUS(status) == 0;
+			}
+			if(WIFSIGNALED(status))
+			{
+				return false;
+			}
+		}
+		return true;
+	}
+}
+#endif
+
+namespace DesktopServices {
+bool openDirectory(const QString &path, bool ensureExists)
+{
+	qDebug() << "Opening directory" << path;
+	QDir parentPath;
+	QDir dir(path);
+	if (!dir.exists())
+	{
+		parentPath.mkpath(dir.absolutePath());
+	}
+	auto f = [&]()
+	{
+		return QDesktopServices::openUrl(QUrl::fromLocalFile(dir.absolutePath()));
+	};
+#if defined(Q_OS_LINUX)
+	return IndirectOpen(f);
+#else
+	return f();
+#endif
+}
+
+bool openFile(const QString &path)
+{
+	qDebug() << "Opening file" << path;
+	auto f = [&]()
+	{
+		return QDesktopServices::openUrl(QUrl::fromLocalFile(path));
+	};
+#if defined(Q_OS_LINUX)
+	return IndirectOpen(f);
+#else
+	return f();
+#endif
+}
+
+bool openFile(const QString &application, const QString &path, const QString &workingDirectory, qint64 *pid)
+{
+	qDebug() << "Opening file" << path << "using" << application;
+#if defined(Q_OS_LINUX)
+	// FIXME: the pid here is fake. So if something depends on it, it will likely misbehave
+	return IndirectOpen([&]()
+	{
+		return QProcess::startDetached(application, QStringList() << path, workingDirectory);
+	}, pid);
+#else
+	return QProcess::startDetached(application, QStringList() << path, workingDirectory, pid);
+#endif
+}
+
+bool run(const QString &application, const QStringList &args, const QString &workingDirectory, qint64 *pid)
+{
+	qDebug() << "Running" << application << "with args" << args.join(' ');
+#if defined(Q_OS_LINUX)
+	// FIXME: the pid here is fake. So if something depends on it, it will likely misbehave
+	return IndirectOpen([&]()
+	{
+		return QProcess::startDetached(application, args, workingDirectory);
+	}, pid);
+#else
+	return QProcess::startDetached(application, args, workingDirectory, pid);
+#endif
+}
+
+bool openUrl(const QUrl &url)
+{
+	qDebug() << "Opening URL" << url.toString();
+	auto f = [&]()
+	{
+		return QDesktopServices::openUrl(url);
+	};
+#if defined(Q_OS_LINUX)
+	return IndirectOpen(f);
+#else
+	return f();
+#endif
+}
+
+}

logic/DesktopServices.h

@@ -0,0 +1,37 @@
+#pragma once
+
+#include <QUrl>
+#include <QString>
+#include "multimc_logic_export.h"
+
+/**
+ * This wraps around QDesktopServices and adds workarounds where needed
+ * Use this instead of QDesktopServices!
+ */
+namespace DesktopServices
+{
+	/**
+	 * Open a file in whatever application is applicable
+	 */
+	MULTIMC_LOGIC_EXPORT bool openFile(const QString &path);
+
+	/**
+	 * Open a file in the specified application
+	 */
+	MULTIMC_LOGIC_EXPORT bool openFile(const QString &application, const QString &path, const QString & workingDirectory = QString(), qint64 *pid = 0);
+
+	/**
+	 * Run an application
+	 */
+	MULTIMC_LOGIC_EXPORT bool run(const QString &application,const QStringList &args, const QString & workingDirectory = QString(), qint64 *pid = 0);
+
+	/**
+	 * Open a directory
+	 */
+	MULTIMC_LOGIC_EXPORT bool openDirectory(const QString &path, bool ensureExists = false);
+
+	/**
+	 * Open the URL, most likely in a browser. Maybe.
+	 */
+	MULTIMC_LOGIC_EXPORT bool openUrl(const QUrl &url);
+};

logic/FileSystem.cpp

@@ -6,8 +6,8 @@
 #include <QSaveFile>
 #include <QFileInfo>
 #include <QDebug>
-#include <QDesktopServices>
 #include <QUrl>
+#include <QStandardPaths>
 
 namespace FS {
 
@@ -304,22 +304,6 @@ QString DirNameFromString(QString string, QString inDir)
 	return dirName;
 }
 
-void openDirInDefaultProgram(QString path, bool ensureExists)
-{
-	QDir parentPath;
-	QDir dir(path);
-	if (!dir.exists())
-	{
-		parentPath.mkpath(dir.absolutePath());
-	}
-	QDesktopServices::openUrl(QUrl::fromLocalFile(dir.absolutePath()));
-}
-
-void openFileInDefaultProgram(QString filename)
-{
-	QDesktopServices::openUrl(QUrl::fromLocalFile(filename));
-}
-
 // Does the directory path contain any '!'? If yes, return true, otherwise false.
 // (This is a problem for Java)
 bool checkProblemticPathJava(QDir folder)
@@ -449,4 +433,4 @@ bool createShortCut(QString location, QString dest, QStringList args, QString na
 	return false;
 #endif
 }
-}
+}

logic/FileSystem.h

@@ -110,12 +110,6 @@ MULTIMC_LOGIC_EXPORT QString RemoveInvalidFilenameChars(QString string, QChar re
 
 MULTIMC_LOGIC_EXPORT QString DirNameFromString(QString string, QString inDir = ".");
 
-/// Opens the given file in the default application.
-MULTIMC_LOGIC_EXPORT void openFileInDefaultProgram(QString filename);
-
-/// Opens the given directory in the default application.
-MULTIMC_LOGIC_EXPORT void openDirInDefaultProgram(QString dirpath, bool ensureExists = false);
-
 /// Checks if the a given Path contains "!"
 MULTIMC_LOGIC_EXPORT bool checkProblemticPathJava(QDir folder);
 

logic/tools/MCEditTool.cpp

@@ -2,7 +2,6 @@
 
 #include <QDir>
 #include <QProcess>
-#include <QDesktopServices>
 #include <QUrl>
 // FIXME: mixing logic and UI!!!!
 #include <QInputDialog>
@@ -11,6 +10,7 @@
 #include "settings/SettingsObject.h"
 #include "BaseInstance.h"
 #include "minecraft/MinecraftInstance.h"
+#include <DesktopServices.h>
 
 MCEditTool::MCEditTool(SettingsObjectPtr settings, InstancePtr instance, QObject *parent)
 	: BaseDetachedTool(settings, instance, parent)
@@ -84,7 +84,7 @@ void MCEditTool::runImpl()
 	#endif
 	if(program.size())
 	{
-		QProcess::startDetached(program, QStringList() << save, mceditPath);
+		DesktopServices::openFile(program, save, mceditPath);
 	}
 #endif
 }