Skip to content

Commit

Permalink
AccountSettings: escape forward- and backslashes in MXIDs
Browse files Browse the repository at this point in the history
Fixes #842.
  • Loading branch information
KitsuneRal committed Dec 22, 2024
1 parent 757f3ce commit f82ebbf
Show file tree
Hide file tree
Showing 5 changed files with 69 additions and 3 deletions.
22 changes: 20 additions & 2 deletions Quotient/settings.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,8 @@

#include "settings.h"

#include "ranges_extras.h"

#include <QtCore/QUrl>

using namespace Quotient;
Expand All @@ -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)
{}

Expand Down Expand Up @@ -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;
}

Expand All @@ -78,7 +96,7 @@ QStringList SettingsGroup::childGroups() const
{
const_cast<SettingsGroup*>(this)->beginGroup(groupPath);
const_cast<QSettings&>(legacySettings).beginGroup(groupPath);
QStringList l = Settings::childGroups();
auto l = Settings::childGroups();
const_cast<SettingsGroup*>(this)->endGroup();
const_cast<QSettings&>(legacySettings).endGroup();
return l;
Expand All @@ -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);
}

Expand Down
8 changes: 7 additions & 1 deletion Quotient/settings.h
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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;
Expand Down
1 change: 1 addition & 0 deletions autotests/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -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)
40 changes: 40 additions & 0 deletions autotests/testsettings.cpp
Original file line number Diff line number Diff line change
@@ -0,0 +1,40 @@
// SPDX-FileCopyrightText: The Quotient Project Contributors
// SPDX-License-Identifier: LGPL-3.0-or-later

#include <QTest>

#include <Quotient/settings.h>

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"
1 change: 1 addition & 0 deletions quotest/quotest.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@

#include <Quotient/connection.h>
#include <Quotient/room.h>
#include <Quotient/settings.h>
#include <Quotient/thread.h>
#include <Quotient/user.h>
#include <Quotient/uriresolver.h>
Expand Down

0 comments on commit f82ebbf

Please sign in to comment.