when locking a file set If-Match header to ensure etag is correct

the file being locked may have been changed sinc the client synced it

so the server side file may have a modified etag. In such cases we do
not want to lock the file and would rather sync the nex server changes
before being able to lock the file

that would ensure that on client side the file being locked is matching
teh state of teh file on server side and would prevent inadvertently
overriding server changes

Signed-off-by: Matthieu Gallien <matthieu.gallien@nextcloud.com>

diff --git a/src/gui/editlocallyjob.cpp b/src/gui/editlocallyjob.cpp
index 0cf1a51e40400d180b88a46fb9a1f230a02c1748..07a1d8f2afb7cc4685d4f83740869ccd3e3f4369 100644 (file)
--- a/src/gui/editlocallyjob.cpp
+++ b/src/gui/editlocallyjob.cpp
@@ -467,11 +467,11 @@ void EditLocallyJob::processLocalItem()
     if (rec.isDirectory() || !_accountState->account()->capabilities().filesLockAvailable()) {
         openFile();
     } else {
-        lockFile();
+        lockFile(rec._etag);
     }
 }
 
-void EditLocallyJob::lockFile()
+void EditLocallyJob::lockFile(const QString &etag)
 {
     Q_ASSERT(_accountState);
     Q_ASSERT(_accountState->account());
@@ -506,6 +506,7 @@ void EditLocallyJob::lockFile()
     _folderForFile->accountState()->account()->setLockFileState(_relPath,
                                                                 _folderForFile->remotePathTrailingSlash(),
                                                                 _folderForFile->path(),
+                                                                etag,
                                                                 _folderForFile->journalDb(),
                                                                 SyncFileItem::LockStatus::LockedItem,
                                                                 SyncFileItem::LockOwnerType::TokenLock);

diff --git a/src/gui/editlocallyjob.h b/src/gui/editlocallyjob.h
index d1029376c23e5dfa0748e929f4e4969635aebf80..d84c5467dd56542dfa26b7bac16a111d31c477c4 100644 (file)
--- a/src/gui/editlocallyjob.h
+++ b/src/gui/editlocallyjob.h
@@ -65,7 +65,7 @@ private slots:
 
     void processLocalItem();
     void openFile();
-    void lockFile();
+    void lockFile(const QString &etag);
 
     void fileAlreadyLocked();
     void fileLockSuccess(const OCC::SyncFileItemPtr &item);

diff --git a/src/gui/folder.cpp b/src/gui/folder.cpp
index a2547dafafa69e14e78b1c9aac71cbdbabd8d0d7..8b8bb0c16f828ac4479334860177f5e46a1ed913 100644 (file)
--- a/src/gui/folder.cpp
+++ b/src/gui/folder.cpp
@@ -696,6 +696,7 @@ void Folder::slotFilesLockReleased(const QSet<QString> &files)
         _accountState->account()->setLockFileState(remoteFilePath,
                                                    remotePathTrailingSlash(),
                                                    path(),
+                                                   rec._etag,
                                                    journalDb(),
                                                    SyncFileItem::LockStatus::UnlockedItem,
                                                    lockOwnerType);
@@ -744,6 +745,7 @@ void Folder::slotLockedFilesFound(const QSet<QString> &files)
         _accountState->account()->setLockFileState(remoteFilePath,
                                                    remotePathTrailingSlash(),
                                                    path(),
+                                                   rec._etag,
                                                    journalDb(),
                                                    SyncFileItem::LockStatus::LockedItem,
                                                    SyncFileItem::LockOwnerType::TokenLock);

diff --git a/src/gui/socketapi/socketapi.cpp b/src/gui/socketapi/socketapi.cpp
index ae32d0c6be3aa80adb6d8815faf73d2f6e0abf44..071ae448b381454b41d8007794dc6d4414549860 100644 (file)
--- a/src/gui/socketapi/socketapi.cpp
+++ b/src/gui/socketapi/socketapi.cpp
@@ -1096,6 +1096,7 @@ void SocketApi::setFileLock(const QString &localFile, const SyncFileItem::LockSt
     shareFolder->accountState()->account()->setLockFileState(fileData.serverRelativePath,
                                                              shareFolder->remotePathTrailingSlash(),
                                                              shareFolder->path(),
+                                                             record._etag,
                                                              shareFolder->journalDb(),
                                                              lockState,
                                                              (lockState == SyncFileItem::LockStatus::UnlockedItem) ? static_cast<SyncFileItem::LockOwnerType>(record._lockstate._lockOwnerType) : SyncFileItem::LockOwnerType::UserLock);

diff --git a/src/libsync/account.cpp b/src/libsync/account.cpp
index 0b20a3551bdc9218e1d74fcaaab8f9fe21acbd5f..82f0376dd5e8564aab1ab4db7417a0cf7ace9f7e 100644 (file)
--- a/src/libsync/account.cpp
+++ b/src/libsync/account.cpp
@@ -983,6 +983,7 @@ std::shared_ptr<UserStatusConnector> Account::userStatusConnector() const
 void Account::setLockFileState(const QString &serverRelativePath,
                                const QString &remoteSyncPathWithTrailingSlash,
                                const QString &localSyncPath,
+                               const QString &etag,
                                SyncJournalDb * const journal,
                                const SyncFileItem::LockStatus lockStatus,
                                const SyncFileItem::LockOwnerType lockOwnerType)
@@ -993,7 +994,7 @@ void Account::setLockFileState(const QString &serverRelativePath,
         return;
     }
     lockStatusJobInProgress.push_back(lockStatus);
-    auto job = std::make_unique<LockFileJob>(sharedFromThis(), journal, serverRelativePath, remoteSyncPathWithTrailingSlash, localSyncPath, lockStatus, lockOwnerType);
+    auto job = std::make_unique<LockFileJob>(sharedFromThis(), journal, serverRelativePath, remoteSyncPathWithTrailingSlash, localSyncPath, etag, lockStatus, lockOwnerType);
     connect(job.get(), &LockFileJob::finishedWithoutError, this, [this, serverRelativePath, lockStatus]() {
         removeLockStatusChangeInprogress(serverRelativePath, lockStatus);
         Q_EMIT lockFileSuccess();

diff --git a/src/libsync/account.h b/src/libsync/account.h
index 28165f05320fdad97f4493db130701e34b708d24..dcf1fb623bd643369feb698fe91e5d7a7edd195d 100644 (file)
--- a/src/libsync/account.h
+++ b/src/libsync/account.h
@@ -345,7 +345,7 @@ public:
 
     void setLockFileState(const QString &serverRelativePath,
                           const QString &remoteSyncPathWithTrailingSlash,
-                          const QString &localSyncPath,
+                          const QString &localSyncPath, const QString &etag,
                           SyncJournalDb * const journal,
                           const SyncFileItem::LockStatus lockStatus,
                           const SyncFileItem::LockOwnerType lockOwnerType);

diff --git a/src/libsync/lockfilejobs.cpp b/src/libsync/lockfilejobs.cpp
index 77e48afcbc66ffc7bbc9b1fb14236a3d04280262..b5e31e9f2fe97fc17be1070ec59ee8a1e4e9e786 100644 (file)
--- a/src/libsync/lockfilejobs.cpp
+++ b/src/libsync/lockfilejobs.cpp
@@ -30,6 +30,7 @@ LockFileJob::LockFileJob(const AccountPtr account,
                          const QString &path,
                          const QString &remoteSyncPathWithTrailingSlash,
                          const QString &localSyncPath,
+                         const QString &etag,
                          const SyncFileItem::LockStatus requestedLockState,
                          const SyncFileItem::LockOwnerType lockOwnerType,
                          QObject *parent)
@@ -39,6 +40,7 @@ LockFileJob::LockFileJob(const AccountPtr account,
     , _requestedLockOwnerType(lockOwnerType)
     , _remoteSyncPathWithTrailingSlash(remoteSyncPathWithTrailingSlash)
     , _localSyncPath(localSyncPath)
+    , _existingEtag(etag)
 {
     if (!_localSyncPath.endsWith(QLatin1Char('/'))) {
         _localSyncPath.append(QLatin1Char('/'));
@@ -65,8 +67,12 @@ void LockFileJob::start()
     switch(_requestedLockState)
     {
     case SyncFileItem::LockStatus::LockedItem:
+    {
+        const auto etagValue = QLatin1String("\"%1\"").arg(_existingEtag.toLatin1());
+        request.setRawHeader(QByteArrayLiteral("If-Match"), etagValue.toLatin1());
         verb = "LOCK";
         break;
+    }
     case SyncFileItem::LockStatus::UnlockedItem:
         verb = "UNLOCK";
         break;
@@ -79,7 +85,7 @@ void LockFileJob::start()
 bool LockFileJob::finished()
 {
     if (reply()->error() != QNetworkReply::NoError) {
-        qCInfo(lcLockFileJob()) << "finished with error" << reply()->error() << reply()->errorString() << _requestedLockState << _requestedLockOwnerType;
+        qCInfo(lcLockFileJob()) << "finished with error" << reply()->error() << reply()->errorString() << _requestedLockState << _requestedLockOwnerType << _existingEtag;
         const auto httpErrorCode = reply()->attribute(QNetworkRequest::HttpStatusCodeAttribute).toInt();
         if (httpErrorCode == LOCKED_HTTP_ERROR_CODE) {
             const auto record = handleReply();

diff --git a/src/libsync/lockfilejobs.h b/src/libsync/lockfilejobs.h
index 9b8be46c5523fde2b0d0fccb6ddb14fe28aa66c2..269f1e91d32d2af99915d43510dd2a3c4a1b8abe 100644 (file)
--- a/src/libsync/lockfilejobs.h
+++ b/src/libsync/lockfilejobs.h
@@ -24,6 +24,7 @@ public:
                          const QString &path,
                          const QString &remoteSyncPathWithTrailingSlash,
                          const QString &localSyncPath,
+                         const QString &etag,
                          const SyncFileItem::LockStatus requestedLockState,
                          const SyncFileItem::LockOwnerType lockOwnerType,
                          QObject *parent = nullptr);
@@ -62,6 +63,7 @@ private:
     QString _lockToken;
     QString _remoteSyncPathWithTrailingSlash;
     QString _localSyncPath;
+    QString _existingEtag;
 };
 
 }

diff --git a/test/testlockfile.cpp b/test/testlockfile.cpp
index ce91016f7ae0979db864866ddee245a669ff66cb..24d6ffa15d54fad7abf7df79a22c853ff96e6f04 100644 (file)
--- a/test/testlockfile.cpp
+++ b/test/testlockfile.cpp
@@ -43,6 +43,7 @@ private slots:
         fakeFolder.account()->setLockFileState(QStringLiteral("/") + testFileName,
                                                QStringLiteral("/"),
                                                fakeFolder.localPath(),
+                                               {},
                                                &fakeFolder.syncJournal(),
                                                OCC::SyncFileItem::LockStatus::LockedItem,
                                                OCC::SyncFileItem::LockOwnerType::UserLock);
@@ -89,6 +90,7 @@ private slots:
         fakeFolder.account()->setLockFileState(QStringLiteral("/") + testFileName,
                                                QStringLiteral("/"),
                                                fakeFolder.localPath(),
+                                               {},
                                                &fakeFolder.syncJournal(),
                                                OCC::SyncFileItem::LockStatus::LockedItem,
                                                OCC::SyncFileItem::LockOwnerType::UserLock);
@@ -114,6 +116,7 @@ private slots:
         fakeFolder.account()->setLockFileState(QStringLiteral("/") + testFileName,
                                                QStringLiteral("/"),
                                                fakeFolder.localPath(),
+                                               {},
                                                &fakeFolder.syncJournal(),
                                                OCC::SyncFileItem::LockStatus::LockedItem,
                                                OCC::SyncFileItem::LockOwnerType::UserLock);
@@ -142,6 +145,7 @@ private slots:
         fakeFolder.account()->setLockFileState(QStringLiteral("/") + testFileName,
                                                QStringLiteral("/"),
                                                fakeFolder.localPath(),
+                                               {},
                                                &fakeFolder.syncJournal(),
                                                OCC::SyncFileItem::LockStatus::LockedItem,
                                                OCC::SyncFileItem::LockOwnerType::UserLock);
@@ -168,6 +172,7 @@ private slots:
                                         QStringLiteral("/") + testFileName,
                                         QStringLiteral("/"),
                                         fakeFolder.localPath(),
+                                        {},
                                         OCC::SyncFileItem::LockStatus::LockedItem,
                                         OCC::SyncFileItem::LockOwnerType::UserLock);
 
@@ -207,6 +212,7 @@ private slots:
                                                 QStringLiteral("/") + testFileName,
                                                 QStringLiteral("/"),
                                                 fakeFolder.localPath(),
+                                                {},
                                                 OCC::SyncFileItem::LockStatus::LockedItem,
                                                 OCC::SyncFileItem::LockOwnerType::UserLock);
 
@@ -225,6 +231,7 @@ private slots:
                                                   QStringLiteral("/") + testFileName,
                                                   QStringLiteral("/"),
                                                   fakeFolder.localPath(),
+                                                  {},
                                                   OCC::SyncFileItem::LockStatus::UnlockedItem,
                                                   OCC::SyncFileItem::LockOwnerType::UserLock);
 
@@ -285,6 +292,7 @@ private slots:
                                         QStringLiteral("/") + testFileName,
                                         QStringLiteral("/"),
                                         fakeFolder.localPath(),
+                                        {},
                                         OCC::SyncFileItem::LockStatus::LockedItem,
                                         OCC::SyncFileItem::LockOwnerType::UserLock);
 
@@ -339,6 +347,7 @@ private slots:
                                         QStringLiteral("/") + testFileName,
                                         QStringLiteral("/"),
                                         fakeFolder.localPath(),
+                                        {},
                                         OCC::SyncFileItem::LockStatus::LockedItem,
                                         OCC::SyncFileItem::LockOwnerType::UserLock);
 
@@ -393,6 +402,7 @@ private slots:
                                         QStringLiteral("/") + testFileName,
                                         QStringLiteral("/"),
                                         fakeFolder.localPath(),
+                                        {},
                                         OCC::SyncFileItem::LockStatus::UnlockedItem,
                                         OCC::SyncFileItem::LockOwnerType::UserLock);
 
@@ -445,6 +455,7 @@ private slots:
                                         QStringLiteral("/") + testFileName,
                                         QStringLiteral("/"),
                                         fakeFolder.localPath(),
+                                        {},
                                         OCC::SyncFileItem::LockStatus::UnlockedItem,
                                         OCC::SyncFileItem::LockOwnerType::UserLock);
 
@@ -484,6 +495,7 @@ private slots:
                                                 QStringLiteral("/") + testFileName,
                                                 QStringLiteral("/"),
                                                 fakeFolder.localPath(),
+                                                {},
                                                 OCC::SyncFileItem::LockStatus::LockedItem,
                                                 OCC::SyncFileItem::LockOwnerType::UserLock);
 
@@ -502,6 +514,7 @@ private slots:
                                                   QStringLiteral("/") + testFileName,
                                                   QStringLiteral("/"),
                                                   fakeFolder.localPath(),
+                                                  {},
                                                   OCC::SyncFileItem::LockStatus::UnlockedItem,
                                                   OCC::SyncFileItem::LockOwnerType::UserLock);
 
@@ -556,6 +569,7 @@ private slots:
                                         QStringLiteral("/") + testFileName,
                                         QStringLiteral("/"),
                                         fakeFolder.localPath(),
+                                        {},
                                         OCC::SyncFileItem::LockStatus::UnlockedItem,
                                         OCC::SyncFileItem::LockOwnerType::UserLock);