jeirmeister/Cockatrice
1
0
Fork 0
mirror of https://github.com/Cockatrice/Cockatrice.git synced 2026-11-05 09:33:25 -08:00
Code Issues Projects Releases Packages Wiki Activity Actions 5

[DeckSync] Match server rows by identity and hash the whole deck

Browse source 
Sync keyed local files to server rows by path, but the server names a row
after the deck, not after the file, so any deck whose file name and deck name
differ uploaded again on every run. A second pass then brought the server copy
back as an extra local file.

- Match by the sidecar's row id, then guid, then content, and only then by the
  server path, so renaming either side no longer duplicates a deck.
- Key the server tree by row id: the server allows several rows with the same
  name in one folder, and keying by path silently dropped all but one.
- Stop discarding rows whose name contains '/' ("U/W Control"). They stay
  matchable, and only the download file name is sanitised.
- Build the download path from the folder plus a sanitised row name, including
  every folder segment, so a hostile server name cannot escape the deck folder.
- Download a colliding server row under a unique name and count it, instead of
  skipping it without a trace.
- Hash the whole deck instead of the card hash: SHA-1 over the native deck
  string covers name, printings, banner, tokens and sideboard plans. The client
  and the server hash the exact bytes they exchange, so both agree.

Adds deck_content_hash_test for the client/server hashing invariant and for the
name-only change the card hash cannot see.
This commit is contained in:
Lukas Brübach 2026-11-01 12:43:02 +01:00 • committed by Lukas Brübach
parent aa0ab333f6
commit 34e22df5ba
7 changed files with 215 additions and 81 deletions
Download patch file Download diff file Expand all files Collapse all files

195
cockatrice/src/interface/deck_sync/deck_sync_manager.cpp
View file

@ -7,6 +7,7 @@
#include <QDirIterator>
#include <QFile>
#include <QFileInfo>
#include <QStringList>
#include <QUuid>
#include <libcockatrice/deck_list/deck_file_format.h>
#include <libcockatrice/deck_list/deck_list.h>
@ -36,6 +37,35 @@ QString serverPathFor(const QString &relativePath)
}
return path;
}
// Rows are named after the deck, not after the file, so a row name may contain
// characters a file name cannot ("U/W Control"). Downloads therefore rebuild the
// local path from the folder plus a sanitised row name, and never from the raw
// server-supplied path.
QString sanitizedSegment(const QString &segment)
{
QString safe = segment;
safe.replace('/', '-');
safe.replace('\\', '-');
safe = safe.trimmed();
if (safe.isEmpty() || safe == QStringLiteral(".") || safe == QStringLiteral("..")) {
return QStringLiteral("deck");
}
return safe;
}
QString localPathFor(const QString &dir, const QString &name)
{
// Every segment is server-supplied, so each one is sanitised before it can
// become part of a local path.
QStringList segments;
for (const QString &segment : dir.split('/', Qt::SkipEmptyParts)) {
segments << sanitizedSegment(segment);
}
segments << sanitizedSegment(name);
const QString path = segments.join('/');
return path.endsWith(".cod", Qt::CaseInsensitive) ? path : path + QStringLiteral(".cod");
}
} // namespace
DeckSyncManager::DeckSyncManager(QObject *parent) : QObject(parent)
@ -111,7 +141,7 @@ void DeckSyncManager::syncNow()
continue;
}
localFiles.insert(relativePath, LocalFile{.absolutePath = absolutePath,
.hash = loaded->deckList.getDeckHash(),
.hash = loaded->deckList.getContentHash(),
.modified = QFileInfo(absolutePath).lastModified()});
}
@ -123,28 +153,30 @@ void DeckSyncManager::syncNow()
void DeckSyncManager::collectServerFiles(const ServerInfo_DeckStorage_Folder &folder,
const QString &prefix,
QMap<QString, ServerFile> &files,
QMap<int, ServerFile> &files,
QSet<QString> &dirs)
{
for (const auto &item : folder.items()) {
const QString name = QString::fromStdString(item.name());
if (name.isEmpty() || name == "." || name == ".." || name.contains('/') || name.contains('\\')) {
continue;
}
const QString path = prefix.isEmpty() ? name : prefix + "/" + name;
if (item.has_folder()) {
const QString path = prefix.isEmpty() ? name : prefix + "/" + name;
dirs.insert(path);
collectServerFiles(item.folder(), path, files, dirs);
} else if (item.has_file()) {
const QString path = prefix.isEmpty() ? name : prefix + "/" + name;
const ServerInfo_DeckStorage_File &file = item.file();
const int id = static_cast<int>(item.id());
ServerFile entry{
.id = static_cast<int>(item.id()),
.id = id,
.path = path,
.dir = prefix,
.name = name,
.guid = QString::fromStdString(file.guid()),
.hash = QString::fromStdString(file.hash()),
.lastModifiedSecs = file.has_last_modified() ? static_cast<qint64>(file.last_modified()) : 0,
};
files.insert(path, entry);
// Keyed by id: the server allows several rows with the same name in a
// folder, and keying by path silently dropped all but one of them.
files.insert(id, entry);
}
}
}
@ -159,7 +191,7 @@ void DeckSyncManager::onListFinished(const Response &response, const CommandCont
return;
}
QMap<QString, ServerFile> serverFiles;
QMap<int, ServerFile> serverFiles;
QSet<QString> serverDirs;
const Response_DeckList &list = response.GetExtension(Response_DeckList::ext);
collectServerFiles(list.root(), QString(), serverFiles, serverDirs);
@ -169,57 +201,78 @@ void DeckSyncManager::onListFinished(const Response &response, const CommandCont
DeckSyncSidecar sidecar(sidecarPath);
sidecar.load();
const qint64 nowSecs = QDateTime::currentDateTime().toSecsSinceEpoch();
// Reuse the sidecar's guid so the server keeps recognizing the row, and mint
// one only for a deck that has never been synced.
auto guidFor = [&sidecar](const QString &relativePath) {
const DeckSyncRecord record = sidecar.get(relativePath);
return record.guid.isEmpty() ? QUuid::createUuid().toString(QUuid::WithoutBraces) : record.guid;
};
QMap<int, QString> serverPathById;
QMap<QString, QString> serverPathByGuid;
QMap<QString, int> serverIdByPath;
QMap<QString, int> serverIdByGuid;
for (auto sit = serverFiles.begin(); sit != serverFiles.end(); ++sit) {
serverPathById.insert(sit.value().id, sit.key());
serverIdByPath.insert(sit.value().path, sit.key());
if (!sit.value().guid.isEmpty()) {
serverPathByGuid.insert(sit.value().guid, sit.key());
serverIdByGuid.insert(sit.value().guid, sit.key());
}
}
QSet<QString> matchedServerPaths;
QSet<int> matchedServerIds;
// A local file without a usable sidecar record still usually has a server
// counterpart: rows are named after the deck, so compare content rather than
// file names, and only upload when no unmatched row has the same content.
auto unmatchedByHash = [&serverFiles, &matchedServerIds](const QString &hash) {
if (hash.isEmpty()) {
return -1;
}
for (auto sit = serverFiles.begin(); sit != serverFiles.end(); ++sit) {
if (!matchedServerIds.contains(sit.key()) && sit.value().hash == hash) {
return sit.key();
}
}
return -1;
};
for (auto it = localFiles.begin(); it != localFiles.end(); ++it) {
const DeckSyncRecord sidecarRecord = sidecar.get(it.key());
QString serverPath;
if (sidecarRecord.remoteId != -1 && serverPathById.contains(sidecarRecord.remoteId)) {
serverPath = serverPathById.value(sidecarRecord.remoteId);
} else if (!sidecarRecord.guid.isEmpty() && serverPathByGuid.contains(sidecarRecord.guid)) {
serverPath = serverPathByGuid.value(sidecarRecord.guid);
int serverId = -1;
if (sidecarRecord.remoteId != -1 && serverFiles.contains(sidecarRecord.remoteId)) {
serverId = sidecarRecord.remoteId;
} else if (!sidecarRecord.guid.isEmpty() && serverIdByGuid.contains(sidecarRecord.guid)) {
serverId = serverIdByGuid.value(sidecarRecord.guid);
} else {
serverPath = serverPathFor(it.key());
if (!serverFiles.contains(serverPath)) {
UploadOp op;
op.relativePath = it.key();
op.serverPath = serverPath;
op.deckId = -1;
op.guid = sidecar.contains(it.key()) && !sidecar.get(it.key()).guid.isEmpty()
? sidecar.get(it.key()).guid
: QUuid::createUuid().toString(QUuid::WithoutBraces);
pendingUploads.append(op);
continue;
// Content first: a row with the same deck is unambiguous, while a
// matching file name may belong to a different deck entirely.
serverId = unmatchedByHash(it.value().hash);
if (serverId == -1) {
const QString wantedPath = serverPathFor(it.key());
if (serverIdByPath.contains(wantedPath)) {
const int pathId = serverIdByPath.value(wantedPath);
if (!matchedServerIds.contains(pathId)) {
serverId = pathId;
}
}
}
}
auto serverIt = serverFiles.find(serverPath);
if (serverIt == serverFiles.end()) {
UploadOp op;
op.relativePath = it.key();
op.serverPath = serverPath;
op.deckId = -1;
op.guid = sidecar.contains(it.key()) && !sidecar.get(it.key()).guid.isEmpty()
? sidecar.get(it.key()).guid
: QUuid::createUuid().toString(QUuid::WithoutBraces);
pendingUploads.append(op);
if (serverId == -1) {
const QString serverPath = serverPathFor(it.key());
pendingUploads.append(UploadOp{.relativePath = it.key(),
.serverPath = serverPath,
.serverDir = parentDir(serverPath),
.deckId = -1,
.guid = guidFor(it.key())});
continue;
}
matchedServerPaths.insert(serverIt.key());
if (it.value().hash == serverIt.value().hash) {
const ServerFile &server = serverFiles.value(serverId);
matchedServerIds.insert(serverId);
if (it.value().hash == server.hash) {
DeckSyncRecord record = sidecar.get(it.key());
if (record.guid.isEmpty()) {
record.guid = serverIt.value().guid;
record.guid = server.guid;
}
record.remoteId = serverIt.value().id;
record.remoteId = server.id;
record.baseHash = it.value().hash;
record.lastSyncSecs = nowSecs;
record.deleted = false;
@ -230,44 +283,36 @@ void DeckSyncManager::onListFinished(const Response &response, const CommandCont
const qint64 localSecs = it.value().modified.toSecsSinceEpoch();
// A server row without last_modified predates sync: the local file wins.
// Otherwise strictly newer wins; ties go to the server.
if (serverIt.value().lastModifiedSecs == 0 || localSecs > serverIt.value().lastModifiedSecs) {
UploadOp op;
op.relativePath = it.key();
op.serverPath = serverPath;
op.deckId = serverIt.value().id;
op.guid = serverIt.value().guid;
pendingUploads.append(op);
if (server.lastModifiedSecs == 0 || localSecs > server.lastModifiedSecs) {
pendingUploads.append(UploadOp{.relativePath = it.key(),
.serverPath = server.path,
.serverDir = server.dir,
.deckId = server.id,
.guid = server.guid});
} else {
DownloadOp op;
op.relativePath = it.key();
op.deckId = serverIt.value().id;
op.guid = serverIt.value().guid;
op.hash = serverIt.value().hash;
op.lastModifiedSecs = serverIt.value().lastModifiedSecs;
pendingDownloads.append(op);
pendingDownloads.append(DownloadOp{.relativePath = it.key(),
.deckId = server.id,
.guid = server.guid,
.hash = server.hash,
.lastModifiedSecs = server.lastModifiedSecs});
}
}
for (auto it = serverFiles.begin(); it != serverFiles.end(); ++it) {
if (matchedServerPaths.contains(it.key())) {
continue;
}
const QString wantedPath = it.key().endsWith(".cod", Qt::CaseInsensitive) ? it.key() : it.key() + ".cod";
if (localFiles.contains(wantedPath) || QFileInfo(deckPath + "/" + wantedPath).exists()) {
if (matchedServerIds.contains(it.key())) {
continue;
}
const QString wantedPath = localPathFor(it.value().dir, it.value().name);
const QString relativePath = uniqueLocalPath(wantedPath);
if (relativePath.isEmpty()) {
++skippedCount;
continue;
}
DownloadOp op;
op.relativePath = relativePath;
op.deckId = it.value().id;
op.guid = it.value().guid;
op.hash = it.value().hash;
op.lastModifiedSecs = it.value().lastModifiedSecs;
pendingDownloads.append(op);
pendingDownloads.append(DownloadOp{.relativePath = relativePath,
.deckId = it.value().id,
.guid = it.value().guid,
.hash = it.value().hash,
.lastModifiedSecs = it.value().lastModifiedSecs});
}
sidecar.save();
@ -297,7 +342,7 @@ void DeckSyncManager::processNext()
void DeckSyncManager::processNextUpload()
{
const UploadOp op = pendingUploads.first();
const QString dir = parentDir(op.serverPath);
const QString dir = op.serverDir;
if (!ensuredDirs.contains(dir)) {
sendMkDirFor(dir);
return;
@ -370,7 +415,7 @@ void DeckSyncManager::sendUpload(const UploadOp &op)
Command_DeckUpload cmd;
if (op.deckId == -1) {
cmd.set_path(parentDir(op.serverPath).toStdString());
cmd.set_path(op.serverDir.toStdString());
} else {
cmd.set_deck_id(op.deckId);
}
@ -416,7 +461,7 @@ void DeckSyncManager::onUploadFinished(const Response &response,
} else {
record.lastSyncSecs = QDateTime::currentDateTime().toSecsSinceEpoch();
}
record.remoteId = file.id();
record.remoteId = static_cast<int>(file.id());
} else if (localFiles.contains(relativePath)) {
record.baseHash = localFiles.value(relativePath).hash;
record.lastSyncSecs = QDateTime::currentDateTime().toSecsSinceEpoch();
@ -517,7 +562,7 @@ void DeckSyncManager::onDownloadFinished(const Response &response,
if (!download.hash().empty()) {
record.baseHash = QString::fromStdString(download.hash());
} else {
record.baseHash = deck.getDeckHash();
record.baseHash = deck.getContentHash();
}
record.lastSyncSecs = download.has_last_modified() ? static_cast<qint64>(download.last_modified())
: QDateTime::currentDateTime().toSecsSinceEpoch();

16
cockatrice/src/interface/deck_sync/deck_sync_manager.h
View file

@ -30,9 +30,11 @@ class ServerInfo_DeckStorage_Folder;
* Each syncNow() run loads the profile sidecar, scans the deck folder, fetches
* the server tree and mirrors both sides: unknown local decks upload, unknown
* server decks download, and decks changed on both sides resolve newer-wins
* (ties go to the server). Matching is path-based; a rename looks like a
* delete plus an add. Before a download overwrites a locally modified file, the
* local content is copied to a .sync-backup file next to it.
* (ties go to the server). A local file is matched to its server row by the
* sidecar's row id, then guid, then the server path, and finally by content, so
* renaming either side does not duplicate a deck. Before a download overwrites a
* locally modified file, the local content is copied to a .sync-backup file next
* to it.
*
* Ownership: fire-and-forget per run, driven by PendingCommand round trips.
* No watcher, no auto triggers, no conflict UI — those arrive in later layers.
@ -81,6 +83,9 @@ private:
struct ServerFile
{
int id = -1;
QString path; ///< Server-side address: folder prefix plus row name.
QString dir; ///< Folder prefix, empty in the root.
QString name; ///< Row name exactly as the server stores it.
QString guid;
QString hash;
qint64 lastModifiedSecs = 0;
@ -94,7 +99,8 @@ private:
struct UploadOp
{
QString relativePath;
QString serverPath;
QString serverPath; ///< Target address of a brand-new row.
QString serverDir; ///< Folder that must exist before the upload.
int deckId = -1;
QString guid;
};
@ -112,7 +118,7 @@ private:
[[nodiscard]] SyncSummary currentSummary() const;
void collectServerFiles(const ServerInfo_DeckStorage_Folder &folder,
const QString &prefix,
QMap<QString, ServerFile> &files,
QMap<int, ServerFile> &files,
QSet<QString> &dirs);
void processNext();
void processNextUpload();

11
libcockatrice_deck_list/libcockatrice/deck_list/deck_list.cpp
View file

@ -9,6 +9,7 @@
#include "tree/inner_deck_list_node.h"
#include <QAnyStringView>
#include <QCryptographicHash>
#include <QDebug>
#include <QSet>
#include <QSharedPointer>
@ -453,6 +454,16 @@ void DeckList::refreshDeckHash()
cachedDeckHash = QString();
}
QString DeckList::contentHash(const QString &nativeDeck)
{
return QString::fromLatin1(QCryptographicHash::hash(nativeDeck.toUtf8(), QCryptographicHash::Sha1).toHex());
}
QString DeckList::getContentHash() const
{
return contentHash(writeToString_Native());
}
/**
* Calls a given function on each card in the deck.
*/

10
libcockatrice_deck_list/libcockatrice/deck_list/deck_list.h
View file

@ -289,6 +289,16 @@ public:
/** @name Deck identity */
///@{
QString getDeckHash() const;
/**
* @brief SHA-1 of a native deck string, covering everything the card hash
* ignores (name, printings, banner, tokens, comments, sideboard plans).
*
* Deck-collection sync hashes the exact string it sends or receives, so both
* peers compare the same bytes.
*/
static QString contentHash(const QString &nativeDeck);
/** @brief contentHash() of this deck's native serialisation. */
QString getContentHash() const;
void refreshDeckHash();
///@}

4
servatrice/src/serversocketinterface.cpp
View file

@ -1114,7 +1114,9 @@ Response::ResponseCode AbstractServerSocketInterface::cmdDeckUpload(const Comman
const QString bannerCardProvider = deck.getBannerCard().providerId.left(32);
const QString tagsJson = serializeDeckTags(deck.getTags());
const QString colorIdentity = sanitizeColorIdentity(nameFromStdString(cmd.color_identity()));
const QString deckHash = deck.getDeckHash();
// Hash the exact bytes the client sent: both peers must compare the same
// string, not each side's re-serialisation of it.
const QString deckHash = DeckList::contentHash(deckStr);
QString guid = cmd.has_guid() ? nameFromStdString(cmd.guid()).left(36) : QString();
if (QUuid::fromString(guid).isNull()) {
guid = QUuid::createUuid().toString(QUuid::WithoutBraces);

9
tests/CMakeLists.txt
View file

@ -416,6 +416,15 @@ cockatrice_add_test(
libcockatrice_deck_list
libcockatrice_utility
)
cockatrice_add_test(
NAME
deck_content_hash_test
SOURCES
deck_content_hash_test.cpp
LIBS
libcockatrice_deck_list
libcockatrice_utility
)
cockatrice_add_test(
NAME
deck_sync_profile_test

51
tests/deck_content_hash_test.cpp Normal file
View file

@ -0,0 +1,51 @@
#include "gtest/gtest.h"
#include <QString>
#include <libcockatrice/deck_list/deck_list.h>
namespace
{
QString deckWithName(const QString &name)
{
return QStringLiteral(R"(<?xml version="1.0"?><cockatrice_deck version="1">)"
R"(<deckname>%1</deckname><comments></comments>)"
R"(<zone name="main"><card number="4" name="Lightning Bolt"/></zone>)"
R"(<zone name="side"><card number="2" name="Pyroblast"/></zone>)"
R"(</cockatrice_deck>)")
.arg(name);
}
} // namespace
TEST(DeckContentHashTest, MatchesNativeSerialisation)
{
DeckList deck;
ASSERT_TRUE(deck.loadFromString_Native(deckWithName(QStringLiteral("Burn"))));
EXPECT_EQ(deck.getContentHash(), DeckList::contentHash(deck.writeToString_Native()));
}
TEST(DeckContentHashTest, IsStableAcrossLoads)
{
DeckList first;
DeckList second;
ASSERT_TRUE(first.loadFromString_Native(deckWithName(QStringLiteral("Burn"))));
ASSERT_TRUE(second.loadFromString_Native(deckWithName(QStringLiteral("Burn"))));
EXPECT_EQ(first.getContentHash(), second.getContentHash());
}
TEST(DeckContentHashTest, CoversDeckName)
{
DeckList burn;
DeckList renamed;
ASSERT_TRUE(burn.loadFromString_Native(deckWithName(QStringLiteral("Burn"))));
ASSERT_TRUE(renamed.loadFromString_Native(deckWithName(QStringLiteral("Burn v2"))));
// The card hash only looks at card names and counts, so it cannot tell the
// two decks apart; the content hash must.
EXPECT_EQ(burn.getDeckHash(), renamed.getDeckHash());
EXPECT_NE(burn.getContentHash(), renamed.getContentHash());
}
int main(int argc, char **argv)
{
::testing::InitGoogleTest(&argc, argv);
return RUN_ALL_TESTS();
}