Fix #6952: prevent deck loss when saving to a full disk (#6978)

* Fix #6952: prevent deck loss when saving to a full disk

DeckLoader::saveToFile() opened the target with QFile in WriteOnly mode,
which truncates the existing file to 0 bytes the moment it is opened. The
serializers (DeckList::saveToFile_Native/_Plain) always return true and the
result of flush() was ignored, so a write that failed part-way -- e.g.
because the disk was full -- left a 0-byte file behind yet was still
reported (and logged) as a successful save. The same truncate-then-write
pattern in updateLastLoadedTimestamp() could destroy a deck on load.

Switch both paths to QSaveFile, which writes to a temporary file and only
atomically replaces the target if commit() succeeds. On any write or flush
failure commit() returns false, the original deck is left untouched, and
the failure is logged instead of being reported as success.

* Use QSaveFile in convertToCockatriceFormat() too

convertToCockatriceFormat() had the same data-loss pattern: QFile WriteOnly
truncated the .cod, saveToFile_Native() always returns true, and the original
file was then removed unconditionally -- so a full disk during conversion wrote
a 0-byte .cod and then deleted the source deck.

Switch to QSaveFile (write + atomic commit), remove the original only after a
successful commit, and move the format check ahead of the file open so an
already-Cockatrice or unsupported deck never truncates or deletes anything.

Raised in review by ZeldaZach.
This commit is contained in:
Christo 2026-06-09 13:54:01 +08:00 committed by Vasco Guerreiro Vintém Morais
parent dcaa918eb0
commit 1416ef0ea5

View file

@ -9,6 +9,7 @@
#include <QFutureWatcher> #include <QFutureWatcher>
#include <QPrinter> #include <QPrinter>
#include <QRegularExpression> #include <QRegularExpression>
#include <QSaveFile>
#include <QStringList> #include <QStringList>
#include <QTextCursor> #include <QTextCursor>
#include <QTextDocument> #include <QTextDocument>
@ -129,7 +130,10 @@ std::optional<LoadedDeck> DeckLoader::loadFromRemote(const QString &nativeString
std::optional<LoadedDeck::LoadInfo> std::optional<LoadedDeck::LoadInfo>
DeckLoader::saveToFile(const DeckList &deck, const QString &fileName, DeckFileFormat::Format fmt) DeckLoader::saveToFile(const DeckList &deck, const QString &fileName, DeckFileFormat::Format fmt)
{ {
QFile file(fileName); // Use QSaveFile so that a failed write (e.g. a full disk) leaves the existing deck untouched
// instead of truncating it to a 0-byte file. The target is only replaced once every byte has
// been flushed successfully in commit().
QSaveFile file(fileName);
if (!file.open(QIODevice::WriteOnly | QIODevice::Text)) { if (!file.open(QIODevice::WriteOnly | QIODevice::Text)) {
qCWarning(DeckLoaderLog) << "Could not create or open file:" << fileName; qCWarning(DeckLoaderLog) << "Could not create or open file:" << fileName;
return std::nullopt; return std::nullopt;
@ -145,15 +149,19 @@ DeckLoader::saveToFile(const DeckList &deck, const QString &fileName, DeckFileFo
break; break;
} }
file.flush();
file.close();
qCInfo(DeckLoaderLog) << "Saved deck to " << fileName << "with format" << fmt << "-" << success;
if (!success) { if (!success) {
file.cancelWriting();
qCWarning(DeckLoaderLog) << "Failed to serialize deck for file:" << fileName;
return std::nullopt; return std::nullopt;
} }
if (!file.commit()) {
qCWarning(DeckLoaderLog) << "Failed to save deck to " << fileName << ":" << file.errorString();
return std::nullopt;
}
qCInfo(DeckLoaderLog) << "Saved deck to " << fileName << "with format" << fmt;
LoadedDeck::LoadInfo lastLoadInfo = {fileName, fmt}; LoadedDeck::LoadInfo lastLoadInfo = {fileName, fmt};
return lastLoadInfo; return lastLoadInfo;
} }
@ -196,38 +204,44 @@ bool DeckLoader::updateLastLoadedTimestamp(LoadedDeck &deck)
QDateTime originalTimestamp = fileInfo.lastModified(); QDateTime originalTimestamp = fileInfo.lastModified();
// Open the file for writing // Use QSaveFile so that a failed write (e.g. a full disk) cannot truncate an existing deck to a
QFile file(fileName); // 0-byte file while merely bumping its timestamp.
QSaveFile file(fileName);
if (!file.open(QIODevice::WriteOnly | QIODevice::Text)) { if (!file.open(QIODevice::WriteOnly | QIODevice::Text)) {
qCWarning(DeckLoaderLog) << "Failed to open file for writing:" << fileName; qCWarning(DeckLoaderLog) << "Failed to open file for writing:" << fileName;
return false; return false;
} }
bool result = false;
// Perform file modifications // Perform file modifications
deck.deckList.setLastLoadedTimestamp(QDateTime::currentDateTime().toString()); deck.deckList.setLastLoadedTimestamp(QDateTime::currentDateTime().toString());
result = deck.deckList.saveToFile_Native(&file);
file.close(); // Close the file to ensure changes are flushed if (!deck.deckList.saveToFile_Native(&file)) {
file.cancelWriting();
if (result) { qCWarning(DeckLoaderLog) << "Failed to serialize deck for file:" << fileName;
// Re-open the file and set the original timestamp return false;
if (!file.open(QIODevice::ReadWrite)) {
qCWarning(DeckLoaderLog) << "Failed to re-open file to set timestamp:" << fileName;
return false;
}
if (!file.setFileTime(originalTimestamp, QFileDevice::FileModificationTime)) {
qCWarning(DeckLoaderLog) << "Failed to set modification time for file:" << fileName;
file.close();
return false;
}
file.close();
} }
return result; if (!file.commit()) {
qCWarning(DeckLoaderLog) << "Failed to update timestamp for file:" << fileName << ":" << file.errorString();
return false;
}
// Re-open the file and restore the original timestamp, so that updating the lastLoadedTimestamp
// does not change the file's modification time.
QFile timestampFile(fileName);
if (!timestampFile.open(QIODevice::ReadWrite)) {
qCWarning(DeckLoaderLog) << "Failed to re-open file to set timestamp:" << fileName;
return false;
}
if (!timestampFile.setFileTime(originalTimestamp, QFileDevice::FileModificationTime)) {
qCWarning(DeckLoaderLog) << "Failed to set modification time for file:" << fileName;
timestampFile.close();
return false;
}
timestampFile.close();
return true;
} }
static QString getDomainForWebsite(DeckLoader::DecklistWebsite website) static QString getDomainForWebsite(DeckLoader::DecklistWebsite website)
@ -444,51 +458,54 @@ bool DeckLoader::convertToCockatriceFormat(LoadedDeck &deck)
return false; return false;
} }
// Determine the format before touching any file, so an already-converted or
// unsupported deck never truncates or deletes anything.
switch (DeckFileFormat::getFormatFromName(fileName)) {
case DeckFileFormat::PlainText:
break;
case DeckFileFormat::Cockatrice:
qCInfo(DeckLoaderLog) << "File is already in Cockatrice format. No conversion needed.";
return true;
default:
qCWarning(DeckLoaderLog) << "Unsupported file format for conversion:" << fileName;
return false;
}
// Change the file extension to .cod // Change the file extension to .cod
QFileInfo fileInfo(fileName); QFileInfo fileInfo(fileName);
QString newFileName = QDir::toNativeSeparators(fileInfo.path() + "/" + fileInfo.completeBaseName() + ".cod"); QString newFileName = QDir::toNativeSeparators(fileInfo.path() + "/" + fileInfo.completeBaseName() + ".cod");
// Open the new file for writing // Use QSaveFile so a failed write (e.g. a full disk) cannot leave a 0-byte .cod
QFile file(newFileName); // behind and then delete the original deck.
QSaveFile file(newFileName);
if (!file.open(QIODevice::WriteOnly | QIODevice::Text)) { if (!file.open(QIODevice::WriteOnly | QIODevice::Text)) {
qCWarning(DeckLoaderLog) << "Failed to open file for writing:" << newFileName; qCWarning(DeckLoaderLog) << "Failed to open file for writing:" << newFileName;
return false; return false;
} }
bool result = false; if (!deck.deckList.saveToFile_Native(&file)) {
file.cancelWriting();
// Perform file modifications based on the detected format qCWarning(DeckLoaderLog) << "Failed to serialize deck for file:" << newFileName;
switch (DeckFileFormat::getFormatFromName(fileName)) { return false;
case DeckFileFormat::PlainText:
// Save in Cockatrice's native format
result = deck.deckList.saveToFile_Native(&file);
break;
case DeckFileFormat::Cockatrice:
qCInfo(DeckLoaderLog) << "File is already in Cockatrice format. No conversion needed.";
result = true;
break;
default:
qCWarning(DeckLoaderLog) << "Unsupported file format for conversion:" << fileName;
result = false;
break;
} }
file.close(); if (!file.commit()) {
qCWarning(DeckLoaderLog) << "Failed to convert deck to " << newFileName << ":" << file.errorString();
// Delete the old file if conversion was successful return false;
if (result) {
if (!QFile::remove(fileName)) {
qCWarning(DeckLoaderLog) << "Failed to delete original file:" << fileName;
} else {
qCInfo(DeckLoaderLog) << "Original file deleted successfully:" << fileName;
}
deck.lastLoadInfo = {
.fileName = newFileName,
.fileFormat = DeckFileFormat::Cockatrice,
};
} }
return result; // Conversion succeeded: delete the original file.
if (!QFile::remove(fileName)) {
qCWarning(DeckLoaderLog) << "Failed to delete original file:" << fileName;
} else {
qCInfo(DeckLoaderLog) << "Original file deleted successfully:" << fileName;
}
deck.lastLoadInfo = {
.fileName = newFileName,
.fileFormat = DeckFileFormat::Cockatrice,
};
return true;
} }
void DeckLoader::printDeckListNode(QTextCursor *cursor, const InnerDecklistNode *node) void DeckLoader::printDeckListNode(QTextCursor *cursor, const InnerDecklistNode *node)