Skip to content

Commit

Permalink
FIX(server): Improve rememberchannelduration compare logic
Browse files Browse the repository at this point in the history
The previous implementation of rememberchannelduration (mumble-voip#4147)
acknowledged the unreliability of the last_disconnect database
field. It used a workaround to set the last_disconnect field
to NULL for every user on every start of the mumble server.
However, with a default behavior for NULL, this created the
unintended side-effect that rememberchannel was erroneously
used for every user on their first connection after a server
restart.

This new implementation reverts the workaround of mumble-voip#4147 and
uses the last_active database field to check if last_disconnect
is reliable. If last_disconnect is unreliable, it will now use
the server uptime and compare it to rememberchanneduration.
We expect last_disconnect to be unreliable when the server
was shut down while clients are still connected.

Fixes mumble-voip#5639
  • Loading branch information
Hartmnt committed May 5, 2022
1 parent 836f356 commit 3767a10
Show file tree
Hide file tree
Showing 3 changed files with 32 additions and 26 deletions.
52 changes: 32 additions & 20 deletions src/murmur/ServerDB.cpp
Expand Up @@ -27,6 +27,8 @@
#include <QtSql/QSqlError>
#include <QtSql/QSqlQuery>

#include <cstdint>

#ifdef Q_OS_WIN
# include <winsock2.h>
#else
Expand Down Expand Up @@ -2156,25 +2158,44 @@ int Server::readLastChannel(int id) {
TransactionHolder th;
QSqlQuery &query = *th.qsqQuery;

SQLPREP("SELECT `lastchannel`,`last_disconnect` FROM `%1users` WHERE `server_id` = ? AND `user_id` = ?");
SQLPREP("SELECT `lastchannel`,`last_active`,`last_disconnect` FROM `%1users` WHERE `server_id` = ? AND `user_id` = ?");
query.addBindValue(iServerNum);
query.addBindValue(id);
SQLEXEC();

if (query.next()) {
int cid = query.value(0).toInt();
if (query.value(1).isNull()) {
return qhChannels.contains(cid) ? cid : -1;
}
QDateTime last_disconnect = QDateTime::fromString(query.value(1).toString(), Qt::ISODate);
last_disconnect.setTimeSpec(Qt::UTC);
QDateTime now = QDateTime::currentDateTime();
now = now.toTimeSpec(Qt::UTC);

if (!qhChannels.contains(cid))
return -1;

int duration = Meta::mp.iRememberChanDuration;
if (duration <= 0 || last_disconnect.secsTo(now) <= duration) {
if (qhChannels.contains(cid))
return cid;

if (duration <= 0)
return cid;

if (query.value(1).isNull())
return -1;

QDateTime last_active = QDateTime::fromString(query.value(1).toString(), Qt::ISODate);
QDateTime last_disconnect;

// NULL column for last_disconnect will yield an empty invalid QDateTime object.
// Using that object with QDateTime::secsTo() will return 0 as per Qt specification.
if (!query.value(2).isNull())
last_disconnect = QDateTime::fromString(query.value(2).toString(), Qt::ISODate);

if (last_active.secsTo(last_disconnect) <= 0) {
// last_disconnect < last_active (or NULL). This can happen, if last_disconnect is invalid, because it has not been
// updated properly (e.g. because the entire server was shut down while the user was still connected).
// Use the server uptime as a reference point instead.
last_disconnect = QDateTime::currentDateTime();
std::int64_t uptimeMSecs = static_cast< std::int64_t >(tUptime.elapsed() / 1000ULL);
last_disconnect = last_disconnect.addMSecs(-uptimeMSecs);
}

if (last_disconnect.secsTo(QDateTime::currentDateTime()) <= duration) {
return cid;
}
}
return -1;
Expand Down Expand Up @@ -2487,12 +2508,3 @@ void ServerDB::deleteServer(int server_id) {
query.addBindValue(server_id);
SQLEXEC();
}

void ServerDB::clearLastDisconnect(Server *server) {
TransactionHolder th;
QSqlQuery &query = *th.qsqQuery;

SQLPREP("UPDATE `%1users` SET `last_disconnect` = NULL WHERE `server_id` = ?");
query.addBindValue(server->iServerNum);
SQLEXEC();
}
4 changes: 0 additions & 4 deletions src/murmur/ServerDB.h
Expand Up @@ -68,10 +68,6 @@ class ServerDB : public QObject {
private:
static void loadOrSetupMetaPBKDF2IterationCount(QSqlQuery &query);
static void writeSUPW(int srvnum, const QString &pwHash, const QString &saltHash, const QVariant &kdfIterations);

public slots:
/// Clear last_disconnect date of every user of the server
void clearLastDisconnect(Server *);
};

#endif
2 changes: 0 additions & 2 deletions src/murmur/main.cpp
Expand Up @@ -559,8 +559,6 @@ int main(int argc, char **argv) {

meta = new Meta();

QObject::connect(meta, SIGNAL(started(Server *)), &db, SLOT(clearLastDisconnect(Server *)));

#ifdef Q_OS_UNIX
// It doesn't matter that this code comes after the forking because detach is
// set to false when readPw is set to true.
Expand Down

0 comments on commit 3767a10

Please sign in to comment.