[Pkg-owncloud-commits] [owncloud-client] 15/175: Propagator: Overwrite local data only if unchanged. #3156
Sandro Knauß
hefee-guest at moszumanska.debian.org
Sat Aug 8 10:36:21 UTC 2015
This is an automated email from the git hooks/post-receive script.
hefee-guest pushed a commit to branch master
in repository owncloud-client.
commit 79ac61684c8f20a393a629c815b2ad0610348f7b
Author: Christian Kamm <kamm at incasoftware.de>
Date: Thu May 7 11:41:29 2015 +0200
Propagator: Overwrite local data only if unchanged. #3156
---
src/libsync/filesystem.cpp | 46 ++++++++++++++++++++++++++++++++++++++-
src/libsync/filesystem.h | 37 ++++++++++++++++++++++++++++---
src/libsync/propagatedownload.cpp | 32 +++++++++++++++++++++------
src/libsync/propagateupload.cpp | 11 ++--------
src/libsync/propagator_legacy.cpp | 6 ++++-
5 files changed, 111 insertions(+), 21 deletions(-)
diff --git a/src/libsync/filesystem.cpp b/src/libsync/filesystem.cpp
index 8411d93..4191686 100644
--- a/src/libsync/filesystem.cpp
+++ b/src/libsync/filesystem.cpp
@@ -16,6 +16,7 @@
#include "utility.h"
#include <QFile>
#include <QFileInfo>
+#include <QCoreApplication>
#include <QDebug>
#if QT_VERSION < QT_VERSION_CHECK(5, 0, 0)
@@ -155,7 +156,50 @@ bool FileSystem::rename(const QString &originFileName,
return success;
}
-bool FileSystem::renameReplace(const QString& originFileName, const QString& destinationFileName, QString* errorString)
+bool FileSystem::fileChanged(const QString& fileName,
+ qint64 previousSize,
+ time_t previousMtime)
+{
+ return getSize(fileName) != previousSize
+ || getModTime(fileName) != previousMtime;
+}
+
+bool FileSystem::verifyFileUnchanged(const QString& fileName,
+ qint64 previousSize,
+ time_t previousMtime)
+{
+ const qint64 actualSize = getSize(fileName);
+ const time_t actualMtime = getModTime(fileName);
+ if (actualSize != previousSize || actualMtime != previousMtime) {
+ qDebug() << "File" << fileName << "has changed:"
+ << "size: " << previousSize << "<->" << actualSize
+ << ", mtime: " << previousMtime << "<->" << actualMtime;
+ return false;
+ }
+ return true;
+}
+
+bool FileSystem::renameReplace(const QString& originFileName,
+ const QString& destinationFileName,
+ qint64 destinationSize,
+ time_t destinationMtime,
+ QString* errorString)
+{
+ if (fileExists(destinationFileName)
+ && fileChanged(destinationFileName, destinationSize, destinationMtime)) {
+ if (errorString) {
+ *errorString = qApp->translate("FileSystem",
+ "The destination file has an unexpected size or modification time");
+ }
+ return false;
+ }
+
+ return uncheckedRenameReplace(originFileName, destinationFileName, errorString);
+}
+
+bool FileSystem::uncheckedRenameReplace(const QString& originFileName,
+ const QString& destinationFileName,
+ QString* errorString)
{
#ifndef Q_OS_WIN
bool success;
diff --git a/src/libsync/filesystem.h b/src/libsync/filesystem.h
index eee0721..ac51e63 100644
--- a/src/libsync/filesystem.h
+++ b/src/libsync/filesystem.h
@@ -67,14 +67,45 @@ bool OWNCLOUDSYNC_EXPORT fileExists(const QString& filename);
bool OWNCLOUDSYNC_EXPORT rename(const QString& originFileName,
const QString& destinationFileName,
QString* errorString = NULL);
+
+/**
+ * Returns true if the file's mtime or size are not what is expected.
+ * Nonexisting files are covered through mtime: they have an mtime of -1.
+ */
+bool fileChanged(const QString& fileName,
+ qint64 previousSize,
+ time_t previousMtime);
+
+/**
+ * Like !fileChanged() but with verbose logging if the file *did* change.
+ */
+bool verifyFileUnchanged(const QString& fileName,
+ qint64 previousSize,
+ time_t previousMtime);
+
/**
- * Rename the file \a originFileName to \a destinationFileName, and overwrite the destination if it
- * already exists
+ * Rename the file \a originFileName to \a destinationFileName, and
+ * overwrite the destination if it already exists - as long as the
+ * destination file has the expected \a destinationSize and
+ * \a destinationMtime.
+ * If the destination file does not exist, the given size and mtime are
+ * ignored.
*/
-bool renameReplace(const QString &originFileName, const QString &destinationFileName,
+bool renameReplace(const QString &originFileName,
+ const QString &destinationFileName,
+ qint64 destinationSize,
+ time_t destinationMtime,
QString *errorString);
/**
+ * Rename the file \a originFileName to \a destinationFileName, and
+ * overwrite the destination if it already exists - without extra checks.
+ */
+bool uncheckedRenameReplace(const QString &originFileName,
+ const QString &destinationFileName,
+ QString *errorString);
+
+/**
* Replacement for QFile::open(ReadOnly) followed by a seek().
* This version sets a more permissive sharing mode on Windows.
*
diff --git a/src/libsync/propagatedownload.cpp b/src/libsync/propagatedownload.cpp
index 6aa94ee..9e2f5fe 100644
--- a/src/libsync/propagatedownload.cpp
+++ b/src/libsync/propagatedownload.cpp
@@ -514,21 +514,37 @@ void PropagateDownloadFileQNAM::downloadFinished()
}
}
- QFileInfo existingFile(fn);
- if(FileSystem::fileExists(fn) && existingFile.permissions() != _tmpFile.permissions()) {
- _tmpFile.setPermissions(existingFile.permissions());
- }
-
FileSystem::setModTime(_tmpFile.fileName(), _item._modtime);
// We need to fetch the time again because some file system such as FAT have a less than a second
// Accuracy, and we really need the time from the file system. (#3103)
_item._modtime = FileSystem::getModTime(_tmpFile.fileName());
+ if (FileSystem::fileExists(fn)) {
+ // Preserve the existing file permissions.
+ QFileInfo existingFile(fn);
+ if (existingFile.permissions() != _tmpFile.permissions()) {
+ _tmpFile.setPermissions(existingFile.permissions());
+ }
+
+ // Check whether the existing file has changed since the discovery
+ // phase by comparing size and mtime to the previous values. This
+ // is necessary to avoid overwriting user changes that happened between
+ // the discovery phase and now.
+ const qint64 expectedSize = _item.log._other_size;
+ const time_t expectedMtime = _item.log._other_modtime;
+ if (! FileSystem::verifyFileUnchanged(fn, expectedSize, expectedMtime)) {
+ _propagator->_anotherSyncNeeded = true;
+ done(SyncFileItem::SoftError, tr("File has changed since discovery"));
+ return;
+ }
+ }
+
QString error;
_propagator->addTouchedFile(fn);
- FileSystem::setFileHidden(_tmpFile.fileName(), false);
- if (!FileSystem::renameReplace(_tmpFile.fileName(), fn, &error)) {
+ // The fileChanged() check is done above to generate better error messages.
+ if (!FileSystem::uncheckedRenameReplace(_tmpFile.fileName(), fn, &error)) {
qDebug() << Q_FUNC_INFO << QString("Rename failed: %1 => %2").arg(_tmpFile.fileName()).arg(fn);
+
// If we moved away the original file due to a conflict but can't
// put the downloaded file in its place, we are in a bad spot:
// If we do nothing the next sync run will assume the user deleted
@@ -540,10 +556,12 @@ void PropagateDownloadFileQNAM::downloadFinished()
_propagator->_journal->deleteFileRecord(fn);
_propagator->_journal->commit("download finished");
}
+
_propagator->_anotherSyncNeeded = true;
done(SyncFileItem::SoftError, error);
return;
}
+ FileSystem::setFileHidden(fn, false);
// Maybe we downloaded a newer version of the file than we thought we would...
// Get up to date information for the journal.
diff --git a/src/libsync/propagateupload.cpp b/src/libsync/propagateupload.cpp
index 4e6a308..887731e 100644
--- a/src/libsync/propagateupload.cpp
+++ b/src/libsync/propagateupload.cpp
@@ -573,15 +573,8 @@ void PropagateUploadFileQNAM::slotPutFinished()
}
}
- // compare expected and real modification time of the file and size
- const time_t new_mtime = FileSystem::getModTime(fullFilePath);
- const quint64 new_size = static_cast<quint64>(FileSystem::getSize(fullFilePath));
- QFileInfo fi(_propagator->getFilePath(_item._file));
- if (new_mtime != _item._modtime || new_size != _item._size) {
- qDebug() << "The local file has changed during upload:"
- << "mtime: " << _item._modtime << "<->" << new_mtime
- << ", size: " << _item._size << "<->" << new_size
- << ", QFileInfo: " << Utility::qDateTimeToTime_t(fi.lastModified()) << fi.lastModified();
+ // Check whether the file changed since discovery.
+ if (! FileSystem::verifyFileUnchanged(fullFilePath, _item._size, _item._modtime)) {
_propagator->_anotherSyncNeeded = true;
if( !finished ) {
abortWithError(SyncFileItem::SoftError, tr("Local file changed during sync."));
diff --git a/src/libsync/propagator_legacy.cpp b/src/libsync/propagator_legacy.cpp
index b9dc7e8..26f6553 100644
--- a/src/libsync/propagator_legacy.cpp
+++ b/src/libsync/propagator_legacy.cpp
@@ -714,7 +714,11 @@ void PropagateDownloadFileLegacy::start()
QString error;
_propagator->addTouchedFile(fn);
- if (!FileSystem::renameReplace(tmpFile.fileName(), fn, &error)) {
+ const qint64 expectedFileSize = _item.log._other_size;
+ const time_t expectedFileMtime = _item.log._other_modtime;
+ if (!FileSystem::renameReplace(tmpFile.fileName(), fn,
+ expectedFileSize, expectedFileMtime,
+ &error)) {
done(SyncFileItem::NormalError, error);
return;
}
--
Alioth's /usr/local/bin/git-commit-notice on /srv/git.debian.org/git/pkg-owncloud/owncloud-client.git
More information about the Pkg-owncloud-commits
mailing list