From db751ca14c18b1bb887c1e50502e521763998c29 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Lukas=20Br=C3=BCbach?= Date: Sun, 1 Nov 2026 12:07:53 +0100 Subject: [PATCH] [Server] Fix alt search review findings - Seed the BFS with the stored user name so the searched account is not re-listed as its own alt at depth 1. - Normalise emails to lower case for the membership and fan-out checks; SQL compares them case-insensitively. - Widen the per-level row limit by the visited count so level 2+ cannot return only already-known accounts. - Prepare dynamic IN-list queries locally instead of through the shared prepared-statement cache, whose keys are query texts. - Count historic client-id fan-out from _sessions, not from the current _users ids. - Order capped identifiers by relevance (current email/client id first, then most recent session IPs) instead of alphabetically. --- .../src/servatrice_database_interface.cpp | 263 +++++++++++------- 1 file changed, 157 insertions(+), 106 deletions(-) diff --git a/servatrice/src/servatrice_database_interface.cpp b/servatrice/src/servatrice_database_interface.cpp index 310a1f7ba..e25ae5e6c 100644 --- a/servatrice/src/servatrice_database_interface.cpp +++ b/servatrice/src/servatrice_database_interface.cpp @@ -2018,14 +2018,22 @@ struct AltCandidate bool isActive = false; }; -QList cappedIdentifiers(QSet values) +// Keeps the first-seen order: callers add the strongest evidence first so the +// cap below drops the least relevant identifiers rather than an arbitrary slice. +void appendIdentifier(QList &list, QSet &seen, const QString &value) { - QList list = values.values(); - std::sort(list.begin(), list.end()); - if (list.size() > MAX_USER_ALT_IDENTIFIERS) { - list.erase(list.begin() + MAX_USER_ALT_IDENTIFIERS, list.end()); + if (!value.isEmpty() && !seen.contains(value)) { + seen.insert(value); + list << value; } - return list; +} + +QList cappedIdentifiers(const QList &values) +{ + if (values.size() <= MAX_USER_ALT_IDENTIFIERS) { + return values; + } + return values.mid(0, MAX_USER_ALT_IDENTIFIERS); } } // namespace @@ -2087,46 +2095,59 @@ QList Servatrice_DatabaseInterface::getUserAlts(const QStrin .isActive = seedQuery->value(7).toBool()}; appendAlt(seed, QStringLiteral("self"), 0, 1); - QSet visited{userName}; - QList frontier{userName}; + // Identifier lookups are case-insensitive in SQL, so the seed must be keyed by + // its stored casing: otherwise the level-1 query re-lists it as an alt. + QSet visited{seed.name}; + QList frontier{seed.name}; + + // Dynamic IN-lists bypass prepareQuery(): it caches by query text, so every + // distinct list size would leave a prepared statement in the long-lived cache + // and can exhaust MySQL's max_prepared_stmt_count. + const QString dbPrefix = server->getDbPrefix(); + auto prepareDynamicQuery = [&dbPrefix](QSqlQuery &query, const QString &queryText) { + QString text = queryText; + text.replace(QStringLiteral("{prefix}"), dbPrefix); + query.prepare(text); + }; + for (int level = 1; level <= depth && !frontier.isEmpty(); ++level) { const QString frontierClause = altInClause(QStringLiteral(":alt_name"), frontier); // Current identifiers of the frontier: registration email + client id. - QSet emails; - QSet clientIds; - QSet ips; - QSqlQuery *frontierQuery = - prepareQuery(QStringLiteral("SELECT email, clientid FROM {prefix}_users WHERE name IN ") + frontierClause); - altBindValues(frontierQuery, QStringLiteral(":alt_name"), frontier); - if (execSqlQuery(frontierQuery)) { - while (frontierQuery->next()) { - const QString email = frontierQuery->value(0).toString(); - const QString clientId = frontierQuery->value(1).toString(); - if (!email.isEmpty()) { - emails << email; - } - if (!clientId.isEmpty()) { - clientIds << clientId; - } + // Kept strongest-first so cappedIdentifiers() drops the least useful ones. + QList emails; + QList clientIds; + QList ips; + QSet emailKeys; + QSet clientIdKeys; + QSet ipKeys; + + QSqlQuery frontierQuery(sqlDatabase); + prepareDynamicQuery(frontierQuery, QStringLiteral("SELECT email, clientid FROM {prefix}_users WHERE name IN ") + + frontierClause); + altBindValues(&frontierQuery, QStringLiteral(":alt_name"), frontier); + if (execSqlQuery(&frontierQuery)) { + while (frontierQuery.next()) { + // Email comparison is case-insensitive in SQL, so normalise the + // keys too or the membership tests below miss real matches. + appendIdentifier(emails, emailKeys, frontierQuery.value(0).toString().toLower()); + appendIdentifier(clientIds, clientIdKeys, frontierQuery.value(1).toString()); } } - // Recent session identifiers: IPs plus historic client ids. - QSqlQuery *sessionQuery = prepareQuery( - QStringLiteral("SELECT DISTINCT ip_address, clientid FROM {prefix}_sessions WHERE user_name IN ") + - frontierClause + QStringLiteral(" AND start_time >= DATE_SUB(NOW(), INTERVAL 6 MONTH)")); - altBindValues(sessionQuery, QStringLiteral(":alt_name"), frontier); - if (execSqlQuery(sessionQuery)) { - while (sessionQuery->next()) { - const QString ip = sessionQuery->value(0).toString(); - const QString clientId = sessionQuery->value(1).toString(); - if (!ip.isEmpty()) { - ips << ip; - } - if (!clientId.isEmpty()) { - clientIds << clientId; - } + // Recent session identifiers: IPs plus historic client ids, most recent + // first so a capped list keeps what the account actually used lately. + QSqlQuery sessionQuery(sqlDatabase); + prepareDynamicQuery(sessionQuery, QStringLiteral("SELECT ip_address, clientid, MAX(start_time) AS last_seen " + "FROM {prefix}_sessions WHERE user_name IN ") + + frontierClause + + QStringLiteral(" AND start_time >= DATE_SUB(NOW(), INTERVAL 6 MONTH) " + "GROUP BY ip_address, clientid ORDER BY last_seen DESC")); + altBindValues(&sessionQuery, QStringLiteral(":alt_name"), frontier); + if (execSqlQuery(&sessionQuery)) { + while (sessionQuery.next()) { + appendIdentifier(ips, ipKeys, sessionQuery.value(0).toString()); + appendIdentifier(clientIds, clientIdKeys, sessionQuery.value(1).toString()); } } @@ -2146,9 +2167,9 @@ QList Servatrice_DatabaseInterface::getUserAlts(const QStrin "LEFT JOIN {prefix}_user_analytics a ON a.id = u.id " "WHERE "); QStringList conditions; - QList emailList = cappedIdentifiers(emails); - QList clientIdList = cappedIdentifiers(clientIds); - QList ipList = cappedIdentifiers(ips); + const QList emailList = cappedIdentifiers(emails); + const QList clientIdList = cappedIdentifiers(clientIds); + const QList ipList = cappedIdentifiers(ips); if (!emailList.isEmpty()) { conditions << QStringLiteral("u.email IN ") + altInClause(QStringLiteral(":alt_email"), emailList); } @@ -2168,33 +2189,38 @@ QList Servatrice_DatabaseInterface::getUserAlts(const QStrin QStringLiteral(" AND s.start_time >= DATE_SUB(NOW(), INTERVAL 6 MONTH))"); } candidateString += conditions.join(QStringLiteral(" OR ")); - candidateString += QStringLiteral(" ORDER BY u.name LIMIT ") + QString::number(MAX_USER_ALTS_PER_LEVEL); + // Already-visited accounts match their own identifiers at every level and + // are dropped below, so widen the row limit by that count: otherwise a + // second level can return only visited rows and no new candidates. + candidateString += + QStringLiteral(" ORDER BY u.name LIMIT ") + QString::number(MAX_USER_ALTS_PER_LEVEL + visited.size()); - QSqlQuery *candidateQuery = prepareQuery(candidateString); - altBindValues(candidateQuery, QStringLiteral(":alt_email"), emailList); - altBindValues(candidateQuery, QStringLiteral(":alt_cid"), clientIdList); - altBindValues(candidateQuery, QStringLiteral(":alt_ip"), ipList); - altBindValues(candidateQuery, QStringLiteral(":alt_hcid"), clientIdList); - if (!execSqlQuery(candidateQuery)) { + QSqlQuery candidateQuery(sqlDatabase); + prepareDynamicQuery(candidateQuery, candidateString); + altBindValues(&candidateQuery, QStringLiteral(":alt_email"), emailList); + altBindValues(&candidateQuery, QStringLiteral(":alt_cid"), clientIdList); + altBindValues(&candidateQuery, QStringLiteral(":alt_ip"), ipList); + altBindValues(&candidateQuery, QStringLiteral(":alt_hcid"), clientIdList); + if (!execSqlQuery(&candidateQuery)) { qCWarning(DatabaseInterfaceLog) << "Failed to collect user alt information: SQL Error"; return results; } QList candidates; - while (candidateQuery->next()) { + while (candidateQuery.next()) { AltCandidate candidate; - candidate.name = candidateQuery->value(0).toString(); + candidate.name = candidateQuery.value(0).toString(); if (visited.contains(candidate.name)) { continue; } - candidate.email = candidateQuery->value(1).toString(); - candidate.clientId = candidateQuery->value(2).toString(); - candidate.registrationTime = candidateQuery->value(3).toLongLong(); - candidate.hasLastLogin = !candidateQuery->value(4).isNull(); - candidate.lastLogin = candidateQuery->value(4).toLongLong(); - candidate.warnCount = candidateQuery->value(5).toInt(); - candidate.banCount = candidateQuery->value(6).toInt(); - candidate.isActive = candidateQuery->value(7).toBool(); + candidate.email = candidateQuery.value(1).toString(); + candidate.clientId = candidateQuery.value(2).toString(); + candidate.registrationTime = candidateQuery.value(3).toLongLong(); + candidate.hasLastLogin = !candidateQuery.value(4).isNull(); + candidate.lastLogin = candidateQuery.value(4).toLongLong(); + candidate.warnCount = candidateQuery.value(5).toInt(); + candidate.banCount = candidateQuery.value(6).toInt(); + candidate.isActive = candidateQuery.value(7).toBool(); candidates << candidate; } @@ -2204,13 +2230,13 @@ QList Servatrice_DatabaseInterface::getUserAlts(const QStrin QMap candidateHistoricClientId; QList unexplained; for (const AltCandidate &candidate : candidates) { - const bool emailMatch = !candidate.email.isEmpty() && emails.contains(candidate.email); - const bool clientIdMatch = !candidate.clientId.isEmpty() && clientIds.contains(candidate.clientId); + const bool emailMatch = !candidate.email.isEmpty() && emailKeys.contains(candidate.email.toLower()); + const bool clientIdMatch = !candidate.clientId.isEmpty() && clientIdKeys.contains(candidate.clientId); if (!emailMatch && !clientIdMatch) { unexplained << candidate.name; } } - if (!unexplained.isEmpty() && (!ips.isEmpty() || !clientIds.isEmpty())) { + if (!unexplained.isEmpty() && (!ipKeys.isEmpty() || !clientIdKeys.isEmpty())) { QString evidenceString = QStringLiteral("SELECT DISTINCT user_name, ip_address, clientid FROM {prefix}_sessions WHERE " "user_name IN ") + @@ -2227,19 +2253,20 @@ QList Servatrice_DatabaseInterface::getUserAlts(const QStrin evidenceString += evidenceConditions.join(QStringLiteral(" OR ")); evidenceString += QStringLiteral(") AND start_time >= DATE_SUB(NOW(), INTERVAL 6 MONTH)"); - QSqlQuery *evidenceQuery = prepareQuery(evidenceString); - altBindValues(evidenceQuery, QStringLiteral(":alt_ev"), unexplained); - altBindValues(evidenceQuery, QStringLiteral(":alt_eip"), ipList); - altBindValues(evidenceQuery, QStringLiteral(":alt_ecid"), clientIdList); - if (execSqlQuery(evidenceQuery)) { - while (evidenceQuery->next()) { - const QString name = evidenceQuery->value(0).toString(); - const QString ip = evidenceQuery->value(1).toString(); - const QString clientId = evidenceQuery->value(2).toString(); - if (!ip.isEmpty() && ips.contains(ip) && !candidateIp.contains(name)) { + QSqlQuery evidenceQuery(sqlDatabase); + prepareDynamicQuery(evidenceQuery, evidenceString); + altBindValues(&evidenceQuery, QStringLiteral(":alt_ev"), unexplained); + altBindValues(&evidenceQuery, QStringLiteral(":alt_eip"), ipList); + altBindValues(&evidenceQuery, QStringLiteral(":alt_ecid"), clientIdList); + if (execSqlQuery(&evidenceQuery)) { + while (evidenceQuery.next()) { + const QString name = evidenceQuery.value(0).toString(); + const QString ip = evidenceQuery.value(1).toString(); + const QString clientId = evidenceQuery.value(2).toString(); + if (!ip.isEmpty() && ipKeys.contains(ip) && !candidateIp.contains(name)) { candidateIp.insert(name, ip); } - if (!clientId.isEmpty() && clientIds.contains(clientId) && + if (!clientId.isEmpty() && clientIdKeys.contains(clientId) && !candidateHistoricClientId.contains(name)) { candidateHistoricClientId.insert(name, clientId); } @@ -2250,58 +2277,82 @@ QList Servatrice_DatabaseInterface::getUserAlts(const QStrin // Fan-out counts so moderators can spot VPN/shared-network false positives. QMap emailCounts; QMap clientIdCounts; + QMap historicClientIdCounts; QMap ipCounts; QSet neededEmails; QSet neededClientIds; + QSet neededHistoricClientIds; QSet neededIps; for (const AltCandidate &candidate : candidates) { - if (!candidate.email.isEmpty() && emails.contains(candidate.email)) { - neededEmails << candidate.email; - } else if (!candidate.clientId.isEmpty() && clientIds.contains(candidate.clientId)) { + if (!candidate.email.isEmpty() && emailKeys.contains(candidate.email.toLower())) { + neededEmails << candidate.email.toLower(); + } else if (!candidate.clientId.isEmpty() && clientIdKeys.contains(candidate.clientId)) { neededClientIds << candidate.clientId; } else if (candidateHistoricClientId.contains(candidate.name)) { - neededClientIds << candidateHistoricClientId.value(candidate.name); + neededHistoricClientIds << candidateHistoricClientId.value(candidate.name); } else if (candidateIp.contains(candidate.name)) { neededIps << candidateIp.value(candidate.name); } } if (!neededEmails.isEmpty()) { const QList neededEmailList = neededEmails.values(); - QSqlQuery *countQuery = prepareQuery(QStringLiteral("SELECT email, COUNT(*) FROM {prefix}_users WHERE " - "email IN ") + - altInClause(QStringLiteral(":alt_ce"), neededEmailList) + - QStringLiteral(" GROUP BY email")); - altBindValues(countQuery, QStringLiteral(":alt_ce"), neededEmailList); - if (execSqlQuery(countQuery)) { - while (countQuery->next()) { - emailCounts.insert(countQuery->value(0).toString(), countQuery->value(1).toInt()); + QSqlQuery countQuery(sqlDatabase); + prepareDynamicQuery(countQuery, QStringLiteral("SELECT email, COUNT(*) FROM {prefix}_users WHERE " + "email IN ") + + altInClause(QStringLiteral(":alt_ce"), neededEmailList) + + QStringLiteral(" GROUP BY email")); + altBindValues(&countQuery, QStringLiteral(":alt_ce"), neededEmailList); + if (execSqlQuery(&countQuery)) { + while (countQuery.next()) { + emailCounts.insert(countQuery.value(0).toString().toLower(), countQuery.value(1).toInt()); } } } if (!neededClientIds.isEmpty()) { const QList neededClientIdList = neededClientIds.values(); - QSqlQuery *countQuery = prepareQuery(QStringLiteral("SELECT clientid, COUNT(*) FROM {prefix}_users WHERE " - "clientid IN ") + - altInClause(QStringLiteral(":alt_cc"), neededClientIdList) + - QStringLiteral(" GROUP BY clientid")); - altBindValues(countQuery, QStringLiteral(":alt_cc"), neededClientIdList); - if (execSqlQuery(countQuery)) { - while (countQuery->next()) { - clientIdCounts.insert(countQuery->value(0).toString(), countQuery->value(1).toInt()); + QSqlQuery countQuery(sqlDatabase); + prepareDynamicQuery(countQuery, QStringLiteral("SELECT clientid, COUNT(*) FROM {prefix}_users WHERE " + "clientid IN ") + + altInClause(QStringLiteral(":alt_cc"), neededClientIdList) + + QStringLiteral(" GROUP BY clientid")); + altBindValues(&countQuery, QStringLiteral(":alt_cc"), neededClientIdList); + if (execSqlQuery(&countQuery)) { + while (countQuery.next()) { + clientIdCounts.insert(countQuery.value(0).toString(), countQuery.value(1).toInt()); + } + } + } + // Historic client ids are matched through sessions, so their fan-out must + // come from _sessions too: _users only knows each account's current id. + if (!neededHistoricClientIds.isEmpty()) { + const QList neededHistoricClientIdList = neededHistoricClientIds.values(); + QSqlQuery countQuery(sqlDatabase); + prepareDynamicQuery(countQuery, + QStringLiteral("SELECT clientid, COUNT(DISTINCT user_name) FROM {prefix}_sessions " + "WHERE clientid IN ") + + altInClause(QStringLiteral(":alt_hcc"), neededHistoricClientIdList) + + QStringLiteral(" AND start_time >= DATE_SUB(NOW(), INTERVAL 6 MONTH) " + "GROUP BY clientid")); + altBindValues(&countQuery, QStringLiteral(":alt_hcc"), neededHistoricClientIdList); + if (execSqlQuery(&countQuery)) { + while (countQuery.next()) { + historicClientIdCounts.insert(countQuery.value(0).toString(), countQuery.value(1).toInt()); } } } if (!neededIps.isEmpty()) { const QList neededIpList = neededIps.values(); - QSqlQuery *countQuery = prepareQuery( - QStringLiteral("SELECT ip_address, COUNT(DISTINCT user_name) FROM {prefix}_sessions WHERE " - "ip_address IN ") + - altInClause(QStringLiteral(":alt_ci"), neededIpList) + - QStringLiteral(" AND start_time >= DATE_SUB(NOW(), INTERVAL 6 MONTH) GROUP BY ip_address")); - altBindValues(countQuery, QStringLiteral(":alt_ci"), neededIpList); - if (execSqlQuery(countQuery)) { - while (countQuery->next()) { - ipCounts.insert(countQuery->value(0).toString(), countQuery->value(1).toInt()); + QSqlQuery countQuery(sqlDatabase); + prepareDynamicQuery(countQuery, + QStringLiteral("SELECT ip_address, COUNT(DISTINCT user_name) FROM {prefix}_sessions " + "WHERE ip_address IN ") + + altInClause(QStringLiteral(":alt_ci"), neededIpList) + + QStringLiteral(" AND start_time >= DATE_SUB(NOW(), INTERVAL 6 MONTH) " + "GROUP BY ip_address")); + altBindValues(&countQuery, QStringLiteral(":alt_ci"), neededIpList); + if (execSqlQuery(&countQuery)) { + while (countQuery.next()) { + ipCounts.insert(countQuery.value(0).toString(), countQuery.value(1).toInt()); } } } @@ -2310,16 +2361,16 @@ QList Servatrice_DatabaseInterface::getUserAlts(const QStrin for (const AltCandidate &candidate : candidates) { QString matchReason; int sharedCount = 1; - if (!candidate.email.isEmpty() && emails.contains(candidate.email)) { + if (!candidate.email.isEmpty() && emailKeys.contains(candidate.email.toLower())) { matchReason = QStringLiteral("email"); - sharedCount = qMax(1, emailCounts.value(candidate.email, 1)); - } else if (!candidate.clientId.isEmpty() && clientIds.contains(candidate.clientId)) { + sharedCount = qMax(1, emailCounts.value(candidate.email.toLower(), 1)); + } else if (!candidate.clientId.isEmpty() && clientIdKeys.contains(candidate.clientId)) { matchReason = QStringLiteral("clientid:") + candidate.clientId; sharedCount = qMax(1, clientIdCounts.value(candidate.clientId, 1)); } else if (candidateHistoricClientId.contains(candidate.name)) { const QString historicId = candidateHistoricClientId.value(candidate.name); matchReason = QStringLiteral("clientid:") + historicId; - sharedCount = qMax(1, clientIdCounts.value(historicId, 1)); + sharedCount = qMax(1, historicClientIdCounts.value(historicId, 1)); } else if (candidateIp.contains(candidate.name)) { const QString ip = candidateIp.value(candidate.name); matchReason = QStringLiteral("ip:") + ip;