fix: don't create native messaging manifest file unless browser directory exists (#6116)

Co-authored-by: Rasmus Karlsson <rasmus.karlsson@pajlada.com>

CHANGELOG.md

@@ -2,6 +2,7 @@
 
 ## Unversioned
 
+- Bugfix: Don't create native messaging manifest file if browser directory doesn't exist. (#6116)
 - Dev: Conan will no longer generate a `CMakeUserPresets.json` file. (#6117)
 
 ## 2.5.3

src/singletons/NativeMessaging.cpp

@@ -27,6 +27,7 @@
 
 namespace {
 
+using namespace chatterino::nm::detail;
 using namespace chatterino;
 using namespace chatterino::literals;
 
@@ -38,7 +39,8 @@ struct Config {
     QString fileName;
     QString registryKey;
 #else
-    QString directory;
+    QString browserDirectory;
+    QString nmDirectory;
 #endif
 };
 
@@ -48,10 +50,11 @@ const Config FIREFOX{
     .registryKey =
         u"HKCU\\Software\\Mozilla\\NativeMessagingHosts\\com.chatterino.chatterino"_s,
 #elif defined(Q_OS_MACOS)
-    .directory =
-        u"~/Library/Application Support/Mozilla/NativeMessagingHosts"_s,
+    .browserDirectory = u"~/Library/Application Support/Mozilla"_s,
+    .nmDirectory = u"NativeMessagingHosts"_s,
 #else
-    .directory = u"~/.mozilla/native-messaging-hosts"_s,
+    .browserDirectory = u"~/.mozilla"_s,
+    .nmDirectory = u"native-messaging-hosts"_s,
 #endif
 };
 
@@ -61,15 +64,36 @@ const Config CHROME{
     .registryKey =
         u"HKCU\\Software\\Google\\Chrome\\NativeMessagingHosts\\com.chatterino.chatterino"_s,
 #elif defined(Q_OS_MACOS)
-    .directory =
-        u"~/Library/Application Support/Google/Chrome/NativeMessagingHosts"_s,
+    .browserDirectory = u"~/Library/Application Support/Google/Chrome/"_s,
+    .nmDirectory = u"NativeMessagingHosts"_s,
 #else
-    .directory = u"~/.config/google-chrome/NativeMessagingHosts"_s,
+    .browserDirectory = u"~/.config/google-chrome"_s,
+    .nmDirectory = u"NativeMessagingHosts"_s,
 #endif
 };
 
-void writeManifestTo(QString directory, const QString &filename,
-                     const QJsonDocument &json)
+void registerNmManifest([[maybe_unused]] const Paths &paths,
+                        const Config &config, const QJsonDocument &document)
+{
+#ifdef Q_OS_WIN
+    writeManifestTo(paths.miscDirectory, u"."_s, config.fileName, document);
+
+    QSettings registry(config.registryKey, QSettings::NativeFormat);
+    registry.setValue("Default",
+                      QString(paths.miscDirectory % u'/' % config.fileName));
+#else
+    writeManifestTo(config.browserDirectory, config.nmDirectory,
+                    u"com.chatterino.chatterino.json"_s, document);
+#endif
+}
+
+}  // namespace
+
+namespace chatterino::nm::detail {
+
+nonstd::expected<void, WriteManifestError> writeManifestTo(
+    QString directory, const QString &nmDirectory, const QString &filename,
+    const QJsonDocument &json)
 {
     if (directory.startsWith('~'))
     {
@@ -77,40 +101,31 @@ void writeManifestTo(QString directory, const QString &filename,
     }
 
     QDir dir(directory);
-    if (!dir.mkpath(u"."_s))
+    if (!dir.exists(nmDirectory) && !dir.mkdir(nmDirectory))
     {
-        qCWarning(chatterinoNativeMessage) << "Failed to create" << directory;
-        return;
+        qCWarning(chatterinoNativeMessage)
+            << "Failed to create" << nmDirectory << "in" << directory;
+        return makeUnexpected(WriteManifestError::FailedToCreateDirectory);
     }
+    dir.cd(nmDirectory);
+
     QFile file(dir.filePath(filename));
     if (!file.open(QFile::WriteOnly | QFile::Truncate))
     {
         qCWarning(chatterinoNativeMessage)
             << "Failed to open" << filename << "in" << directory;
-        return;
+        return makeUnexpected(WriteManifestError::FailedToCreateFile);
     }
     file.write(json.toJson());
+
+    return {};
 }
 
-void registerNmManifest([[maybe_unused]] const Paths &paths,
-                        const Config &config, const QJsonDocument &document)
-{
-#ifdef Q_OS_WIN
-    writeManifestTo(paths.miscDirectory, config.fileName, document);
-
-    QSettings registry(config.registryKey, QSettings::NativeFormat);
-    registry.setValue("Default",
-                      QString(paths.miscDirectory % u'/' % config.fileName));
-#else
-    writeManifestTo(config.directory, u"com.chatterino.chatterino.json"_s,
-                    document);
-#endif
-}
-
-}  // namespace
+}  // namespace chatterino::nm::detail
 
 namespace chatterino {
 
+using namespace chatterino::nm::detail;
 using namespace literals;
 
 void registerNmHost(const Paths &paths)

src/singletons/NativeMessaging.hpp

@@ -1,6 +1,7 @@
 #pragma once
 
 #include "common/Atomic.hpp"
+#include "util/Expected.hpp"
 
 #include <QString>
 #include <QThread>
@@ -8,6 +9,19 @@
 #include <optional>
 #include <vector>
 
+namespace chatterino::nm::detail {
+
+enum class WriteManifestError : std::uint8_t {
+    FailedToCreateDirectory,
+    FailedToCreateFile,
+};
+
+nonstd::expected<void, WriteManifestError> writeManifestTo(
+    QString directory, const QString &nmDirectory, const QString &filename,
+    const QJsonDocument &json);
+
+}  // namespace chatterino::nm::detail
+
 namespace chatterino {
 
 class Application;

tests/CMakeLists.txt

@@ -54,6 +54,7 @@ set(test_SOURCES
     ${CMAKE_CURRENT_LIST_DIR}/src/OnceFlag.cpp
     ${CMAKE_CURRENT_LIST_DIR}/src/IncognitoBrowser.cpp
     ${CMAKE_CURRENT_LIST_DIR}/src/EventSubMessages.cpp
+    ${CMAKE_CURRENT_LIST_DIR}/src/NativeMessaging.cpp
 
     ${CMAKE_CURRENT_LIST_DIR}/src/lib/Snapshot.cpp
     ${CMAKE_CURRENT_LIST_DIR}/src/lib/Snapshot.hpp

tests/src/NativeMessaging.cpp

@@ -0,0 +1,62 @@
+#include "singletons/NativeMessaging.hpp"
+
+#include "Test.hpp"
+#include "util/CombinePath.hpp"
+
+#include <QDir>
+#include <QJsonDocument>
+#include <QTemporaryDir>
+
+using namespace chatterino;
+
+using namespace chatterino::nm::detail;
+
+class NativeMessagingFixture : public ::testing::Test
+{
+    QTemporaryDir tempDir;
+
+protected:
+    NativeMessagingFixture()
+        : dir(combinePath(this->tempDir.path(), QString("native-messaging")))
+    {
+    }
+
+    QDir dir;
+};
+
+TEST_F(NativeMessagingFixture, writeManifestToSubDirCreated)
+{
+    // writeManifestTo should succeed if the subdir directory is created
+    ASSERT_TRUE(this->dir.mkpath("."));
+
+    ASSERT_TRUE(writeManifestTo(this->dir.path(), "native-messaging-hosts",
+                                "test.json", QJsonDocument())
+                    .has_value());
+
+    QDir nmDir = combinePath(this->dir.path(), "native-messaging-hosts");
+
+    ASSERT_TRUE(nmDir.exists());
+
+    ASSERT_TRUE(QFile(combinePath(nmDir.path(), "test.json")).exists());
+}
+
+TEST_F(NativeMessagingFixture, writeManifestToSubDirNotCreated)
+{
+    // writeManifestTo should fail if the subdir is not created
+    ASSERT_EQ(writeManifestTo(this->dir.path(), "native-messaging-hosts",
+                              "test.json", QJsonDocument())
+                  .error(),
+              WriteManifestError::FailedToCreateDirectory);
+}
+
+TEST_F(NativeMessagingFixture, writeManifestToWindowsCurrentDir)
+{
+    // writeManifestTo should succeed if the subdir is created and the path to create is "."
+    ASSERT_TRUE(this->dir.mkpath("."));
+
+    ASSERT_TRUE(
+        writeManifestTo(this->dir.path(), ".", "test.json", QJsonDocument())
+            .has_value());
+
+    ASSERT_TRUE(QFile(combinePath(this->dir.path(), "test.json")).exists());
+}