diff --git a/Quotient/settings.cpp b/Quotient/settings.cpp index 40e547db0..4a4ce0e01 100644 --- a/Quotient/settings.cpp +++ b/Quotient/settings.cpp @@ -3,6 +3,8 @@ #include "settings.h" +#include "ranges_extras.h" + #include using namespace Quotient; @@ -17,6 +19,20 @@ void Settings::setLegacyNames(const QString& organizationName, legacyApplicationName = applicationName; } +QString Settings::escapedForSettings(QString key) +{ + key.replace(u'/', u"%2F"_s); + key.replace(u'\\', u"%5C"_s); + return key; +} + +QString Settings::unescapedFromSettings(QString key) +{ + key.replace(u"%2F"_s, u"/"_s); + key.replace(u"%5C"_s, u"\\"_s); + return key; +} + Settings::Settings(QObject* parent) : QSettings(parent) {} @@ -57,6 +73,8 @@ QStringList Settings::childGroups() const for (const auto& g: legacyGroups) if (!groups.contains(g)) groups.push_back(g); + if (group() == u"Accounts") + std::ranges::for_each(groups, [](QString& g) { g = unescapedFromSettings(g); }); // See #842 return groups; } @@ -78,7 +96,7 @@ QStringList SettingsGroup::childGroups() const { const_cast(this)->beginGroup(groupPath); const_cast(legacySettings).beginGroup(groupPath); - QStringList l = Settings::childGroups(); + auto l = Settings::childGroups(); const_cast(this)->endGroup(); const_cast(legacySettings).endGroup(); return l; @@ -88,7 +106,7 @@ void SettingsGroup::remove(const QString& key) { QString fullKey { groupPath }; if (!key.isEmpty()) - fullKey += u'/' + key; + fullKey += u'/' + escapedForSettings(key); Settings::remove(fullKey); } diff --git a/Quotient/settings.h b/Quotient/settings.h index 3b7c95305..b35e06a47 100644 --- a/Quotient/settings.h +++ b/Quotient/settings.h @@ -69,6 +69,12 @@ class QUOTIENT_API Settings : public QSettings { Q_INVOKABLE bool contains(const QString& key) const; Q_INVOKABLE QStringList childGroups() const; + //! Escape forward- and backslashes in keys because QSettings doesn't (see #842) + static QString escapedForSettings(QString key); + + //! Unescape `\` and `/` in keys stored with escapedForSettings() + static QString unescapedFromSettings(QString key); + private: static QString legacyOrganizationName; static QString legacyApplicationName; @@ -128,7 +134,7 @@ class QUOTIENT_API AccountSettings : public SettingsGroup { WRITE setEncryptionAccountPickle) public: explicit AccountSettings(const QString& accountId, QObject* parent = nullptr) - : SettingsGroup("Accounts/"_L1 + accountId, parent) + : SettingsGroup("Accounts/"_L1 + escapedForSettings(accountId), parent) {} QString userId() const; diff --git a/autotests/CMakeLists.txt b/autotests/CMakeLists.txt index a8023a351..28540d2c1 100644 --- a/autotests/CMakeLists.txt +++ b/autotests/CMakeLists.txt @@ -29,4 +29,5 @@ quotient_add_test(NAME testcryptoutils) quotient_add_test(NAME testkeyverification) quotient_add_test(NAME testcrosssigning) quotient_add_test(NAME testkeyimport) +quotient_add_test(NAME testsettings) quotient_add_test(NAME testthread) diff --git a/autotests/testsettings.cpp b/autotests/testsettings.cpp new file mode 100644 index 000000000..0fd77b95d --- /dev/null +++ b/autotests/testsettings.cpp @@ -0,0 +1,40 @@ +// SPDX-FileCopyrightText: The Quotient Project Contributors +// SPDX-License-Identifier: LGPL-3.0-or-later + +#include + +#include + +class TestSettings : public QObject { + Q_OBJECT +private slots: + void accountSettings(); +}; + +using namespace Qt::Literals; +using Quotient::Settings, Quotient::SettingsGroup, Quotient::AccountSettings; + +void TestSettings::accountSettings() +{ + static const auto AccountsGroupName = u"Accounts"_s; + + QSettings qSettings{}; + qSettings.remove(AccountsGroupName); + const auto mxId = u"@user/with\\slashes:example.org"_s; // Test #842 + const auto escapedMxId = Settings::escapedForSettings(mxId); + AccountSettings(mxId).setHomeserver(QUrl(u"https://example.org"_s)); + qSettings.sync(); + qSettings.beginGroup(AccountsGroupName); + // NB: QSettings::contains() doesn't work on groups, only on leaf keys; hence childGroups below + auto childGroups = qSettings.childGroups(); + QVERIFY(childGroups.contains(escapedMxId)); + QVERIFY(SettingsGroup(AccountsGroupName).childGroups().contains(mxId)); + SettingsGroup(AccountsGroupName).remove(mxId); + qSettings.sync(); + childGroups = qSettings.childGroups(); + QVERIFY(!childGroups.contains(escapedMxId)); + qSettings.endGroup(); +} + +QTEST_GUILESS_MAIN(TestSettings) +#include "testsettings.moc" diff --git a/quotest/quotest.cpp b/quotest/quotest.cpp index df43638fe..cec6b4315 100644 --- a/quotest/quotest.cpp +++ b/quotest/quotest.cpp @@ -3,6 +3,7 @@ #include #include +#include #include #include #include