Skip to content

Commit

Permalink
FIX(server): Stale username cache-entries due to case differences
Browse files Browse the repository at this point in the history
Mumble treats usernames case-insensitively. However, in the server,
there is a cache that maps usernames to IDs of registered users. We do
lazy loading so when a given name is not yet in the cache, we check the
database for a match and if that returns something, we insert the result
into the cache. However, the DB does case-insensitive lookups whereas
the cache used to be case-sensitive. Therefore, different casings of the
same username could end up in the cache (all pointing to the same user
ID). If that user got unregistered, only one of the possible ways of
writing their name gets removed from the cache, leaving the other
entries (which are now stale) untouched.

This can cause problems when another user now wants to register with a
name that corresponds to one of those stale cache entries. The server
will think that the given username is already taken, ensuring leading to
a rejection of the registration request.

A server restart purges the stale entries from the cache.

This commit ensures that the cache is now also operating in a
case-insensitive manner such that we shouldn't ever create any duplicate
entries in there in the first place.

Fixes mumble-voip#6383
  • Loading branch information
Krzmbrzl committed Jul 13, 2024
1 parent 1c58bd3 commit c85dfe6
Show file tree
Hide file tree
Showing 6 changed files with 227 additions and 1 deletion.
74 changes: 74 additions & 0 deletions src/QtUtils.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,8 @@
// that can be found in the LICENSE file at the root of the
// Mumble source tree or at <https://www.mumble.info/LICENSE>.

#include "QtUtils.h"

#include <QObject>
#include <QStringList>
#include <QUrl>
Expand All @@ -23,6 +25,78 @@ namespace QtUtils {
return QString();
}

CaseInsensitiveQString::CaseInsensitiveQString(const QString &str) : m_str(str) {}

CaseInsensitiveQString::CaseInsensitiveQString(QString &&str) : m_str(std::move(str)) {}

CaseInsensitiveQString &CaseInsensitiveQString::operator=(const QString &str) {
m_str = str;

return *this;
}

CaseInsensitiveQString &CaseInsensitiveQString::operator=(QString &&str) {
m_str = std::move(str);

return *this;
}

CaseInsensitiveQString::operator const QString &() const { return m_str; }

CaseInsensitiveQString::operator QString &() { return m_str; }

bool operator==(const QString &lhs, const CaseInsensitiveQString &rhs) {
return lhs.compare(rhs.m_str, Qt::CaseInsensitive) == 0;
}

bool operator==(const CaseInsensitiveQString &lhs, const CaseInsensitiveQString &rhs) { return lhs.m_str == rhs; }

bool operator==(const CaseInsensitiveQString &lhs, const QString &rhs) { return rhs == lhs; }

bool operator!=(const QString &lhs, const CaseInsensitiveQString &rhs) { return !(lhs == rhs); }

bool operator!=(const CaseInsensitiveQString &lhs, const CaseInsensitiveQString &rhs) { return !(lhs == rhs); }

bool operator!=(const CaseInsensitiveQString &lhs, const QString &rhs) { return !(lhs == rhs); }

bool operator<(const QString &lhs, const CaseInsensitiveQString &rhs) {
return lhs.compare(rhs.m_str, Qt::CaseInsensitive) < 0;
}

bool operator<(const CaseInsensitiveQString &lhs, const CaseInsensitiveQString &rhs) { return lhs.m_str < rhs; }

bool operator<(const CaseInsensitiveQString &lhs, const QString &rhs) { return rhs >= lhs; }

bool operator<=(const QString &lhs, const CaseInsensitiveQString &rhs) {
return lhs.compare(rhs.m_str, Qt::CaseInsensitive) <= 0;
}

bool operator<=(const CaseInsensitiveQString &lhs, const CaseInsensitiveQString &rhs) { return lhs.m_str <= rhs; }

bool operator<=(const CaseInsensitiveQString &lhs, const QString &rhs) { return rhs >= lhs; }

bool operator>(const QString &lhs, const CaseInsensitiveQString &rhs) {
return lhs.compare(rhs.m_str, Qt::CaseInsensitive) > 0;
}

bool operator>(const CaseInsensitiveQString &lhs, const CaseInsensitiveQString &rhs) { return lhs.m_str > rhs; }

bool operator>(const CaseInsensitiveQString &lhs, const QString &rhs) {
return rhs <= lhs;
}

bool operator>=(const QString &lhs, const CaseInsensitiveQString &rhs) {
return lhs.compare(rhs.m_str, Qt::CaseInsensitive) >= 0;
}

bool operator>=(const CaseInsensitiveQString &lhs, const CaseInsensitiveQString &rhs) { return lhs.m_str >= rhs; }

bool operator>=(const CaseInsensitiveQString &lhs, const QString &rhs) { return rhs <= lhs; }

} // namespace QtUtils
} // namespace Mumble

uint qHash(const Mumble::QtUtils::CaseInsensitiveQString &str, uint seed) {
const QString &lower = static_cast< const QString & >(str).toLower();
return qHash(lower, seed);
}
47 changes: 47 additions & 0 deletions src/QtUtils.h
Original file line number Diff line number Diff line change
Expand Up @@ -34,9 +34,56 @@ namespace QtUtils {
*/
QString decode_first_utf8_qssl_string(const QStringList &list);

/**
* A wrapper around a QString object that ensures all comparisons and hashes are performed
* in a case-insensitive manner.
*/
class CaseInsensitiveQString {
public:
CaseInsensitiveQString() = default;
~CaseInsensitiveQString() = default;

CaseInsensitiveQString(const CaseInsensitiveQString &) = default;
CaseInsensitiveQString(CaseInsensitiveQString &&) = default;
CaseInsensitiveQString &operator=(const CaseInsensitiveQString &) = default;
CaseInsensitiveQString &operator=(CaseInsensitiveQString &&) = default;

CaseInsensitiveQString(const QString &str);
CaseInsensitiveQString(QString &&str);
CaseInsensitiveQString &operator=(const QString &str);
CaseInsensitiveQString &operator=(QString &&str);

operator const QString &() const;
operator QString &();

friend bool operator==(const QString &lhs, const CaseInsensitiveQString &rhs);
friend bool operator==(const CaseInsensitiveQString &lhs, const CaseInsensitiveQString &rhs);
friend bool operator==(const CaseInsensitiveQString &lhs, const QString &rhs);
friend bool operator!=(const QString &lhs, const CaseInsensitiveQString &rhs);
friend bool operator!=(const CaseInsensitiveQString &lhs, const CaseInsensitiveQString &rhs);
friend bool operator!=(const CaseInsensitiveQString &lhs, const QString &rhs);
friend bool operator<(const QString &lhs, const CaseInsensitiveQString &rhs);
friend bool operator<(const CaseInsensitiveQString &lhs, const CaseInsensitiveQString &rhs);
friend bool operator<(const CaseInsensitiveQString &lhs, const QString &rhs);
friend bool operator<=(const QString &lhs, const CaseInsensitiveQString &rhs);
friend bool operator<=(const CaseInsensitiveQString &lhs, const CaseInsensitiveQString &rhs);
friend bool operator<=(const CaseInsensitiveQString &lhs, const QString &rhs);
friend bool operator>(const QString &lhs, const CaseInsensitiveQString &rhs);
friend bool operator>(const CaseInsensitiveQString &lhs, const CaseInsensitiveQString &rhs);
friend bool operator>(const CaseInsensitiveQString &lhs, const QString &rhs);
friend bool operator>=(const QString &lhs, const CaseInsensitiveQString &rhs);
friend bool operator>=(const CaseInsensitiveQString &lhs, const CaseInsensitiveQString &rhs);
friend bool operator>=(const CaseInsensitiveQString &lhs, const QString &rhs);

private:
QString m_str;
};

} // namespace QtUtils
} // namespace Mumble

uint qHash(const Mumble::QtUtils::CaseInsensitiveQString &str, uint seed = 0);

template< typename T > using qt_unique_ptr = std::unique_ptr< T, decltype(&Mumble::QtUtils::deleteQObject) >;

/// Creates a new unique_ptr with custom deleter for any given QObject*
Expand Down
3 changes: 2 additions & 1 deletion src/murmur/Server.h
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@
#include "User.h"
#include "Version.h"
#include "VolumeAdjustment.h"
#include "QtUtils.h"

#include "database/ConnectionParameter.h"

Expand Down Expand Up @@ -319,7 +320,7 @@ public slots:
ChanACL::ACLCache acCache;

QHash< int, QString > qhUserNameCache;
QHash< QString, int > qhUserIDCache;
QHash< Mumble::QtUtils::CaseInsensitiveQString, int > qhUserIDCache;

std::vector< Ban > m_bans;

Expand Down
1 change: 1 addition & 0 deletions src/tests/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@ if(server)
endif()

# Shared tests
add_subdirectory("TestCaseInsensitiveQString")
add_subdirectory("TestCryptographicHash")
add_subdirectory("TestCryptographicRandom")
add_subdirectory("TestDatabase")
Expand Down
12 changes: 12 additions & 0 deletions src/tests/TestCaseInsensitiveQString/CMakeLists.txt
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
# Copyright 2024 The Mumble Developers. All rights reserved.
# Use of this source code is governed by a BSD-style license
# that can be found in the LICENSE file at the root of the
# Mumble source tree or at <https://www.mumble.info/LICENSE>.

add_executable(TestCaseInsensitiveQString TestCaseInsensitiveQString.cpp)

set_target_properties(TestCaseInsensitiveQString PROPERTIES AUTOMOC ON)

target_link_libraries(TestCaseInsensitiveQString PRIVATE shared Qt5::Test)

add_test(NAME TestCaseInsensitiveQString COMMAND $<TARGET_FILE:TestCaseInsensitiveQString>)
Original file line number Diff line number Diff line change
@@ -0,0 +1,91 @@
// Copyright 2024 The Mumble Developers. All rights reserved.
// Use of this source code is governed by a BSD-style license
// that can be found in the LICENSE file at the root of the
// Mumble source tree or at <https://www.mumble.info/LICENSE>.

#include "QtUtils.h"

#include <QHash>
#include <QObject>
#include <QTest>

using namespace Mumble::QtUtils;

class TestCaseInsensitiveQString : public QObject {
Q_OBJECT
private slots:

void equality() const {
CaseInsensitiveQString istr("test");

QVERIFY(QString("Test") == istr);
QVERIFY(istr == QString("TEST"));
QVERIFY(istr == istr);
}

void inequality() const {
CaseInsensitiveQString istr("test");

QVERIFY(QString("Tezt") != istr);
QVERIFY(istr != QString("TETS"));
QVERIFY(istr != CaseInsensitiveQString("test2"));
}

void less_than() const {
CaseInsensitiveQString istr("test");

QVERIFY(QString("abc") < istr);
QVERIFY(istr < QString("xyz"));
QVERIFY(CaseInsensitiveQString("another") < istr);
}

void less_equal() const {
CaseInsensitiveQString istr("test");

QVERIFY(QString("Abc") <= istr);
QVERIFY(istr <= QString("xyz"));
QVERIFY(CaseInsensitiveQString("Taxi") <= istr);

QVERIFY(QString("teSt") <= istr);
QVERIFY(istr <= QString("Test"));
QVERIFY(CaseInsensitiveQString("TEST") <= istr);
}

void greater_than() const {
CaseInsensitiveQString istr("test");

QVERIFY(QString("xyz") > istr);
QVERIFY(istr > QString("abc"));
QVERIFY(CaseInsensitiveQString("Xenia") > istr);
}

void greater_equal() const {
CaseInsensitiveQString istr("test");

QVERIFY(QString("xyz") >= istr);
QVERIFY(istr >= QString("abc"));
QVERIFY(CaseInsensitiveQString("Xenia") >= istr);

QVERIFY(QString("Test") >= istr);
QVERIFY(istr >= QString("teST"));
QVERIFY(CaseInsensitiveQString("TeSt") >= istr);
}

void hash() const {
QVERIFY(qHash(CaseInsensitiveQString("test")) == qHash(CaseInsensitiveQString("TEST")));
QVERIFY(qHash(CaseInsensitiveQString("test")) != qHash(CaseInsensitiveQString("TESTER")));
}

void use_in_qhash() const {
QHash< CaseInsensitiveQString, int > hash;
hash.insert(QString("TeSt"), 42);

QVERIFY(hash.contains(QString("test")));
QCOMPARE(hash[QString("test")], 42);
QCOMPARE(static_cast< const QString & >(hash.begin().key()), QString("TeSt"));
QVERIFY(static_cast< const QString & >(hash.begin().key()) != QString("test"));
}
};

QTEST_MAIN(TestCaseInsensitiveQString)
#include "TestCaseInsensitiveQString.moc"

0 comments on commit c85dfe6

Please sign in to comment.