Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[stable-3.15] when locking a file set If-Match header to ensure etag is correct #7620

Merged
merged 1 commit into from
Dec 9, 2024
Merged

[stable-3.15] when locking a file set If-Match header to ensure etag is correct #7620

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 3 additions & 2 deletions src/gui/editlocallyjob.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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());
Expand Down Expand Up @@ -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);
Expand Down
2 changes: 1 addition & 1 deletion src/gui/editlocallyjob.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@

#pragma once

#include <QObject>

Check failure on line 17 in src/gui/editlocallyjob.h

View workflow job for this annotation

GitHub Actions / build

src/gui/editlocallyjob.h:17:10 [clang-diagnostic-error] 

'QObject' file not found

#include "accountstate.h"
#include "syncfileitem.h"
Expand Down Expand Up @@ -65,7 +65,7 @@

void processLocalItem();
void openFile();
void lockFile();
void lockFile(const QString &etag);

void fileAlreadyLocked();
void fileLockSuccess(const OCC::SyncFileItemPtr &item);
Expand Down
2 changes: 2 additions & 0 deletions src/gui/folder.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@
* or FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License
* for more details.
*/
#include "common/syncjournaldb.h"

Check failure on line 16 in src/gui/folder.cpp

View workflow job for this annotation

GitHub Actions / build

src/gui/folder.cpp:16:10 [clang-diagnostic-error] 

'common/syncjournaldb.h' file not found
#include "config.h"

#include "account.h"
Expand Down Expand Up @@ -696,6 +696,7 @@
_accountState->account()->setLockFileState(remoteFilePath,
remotePathTrailingSlash(),
path(),
rec._etag,
journalDb(),
SyncFileItem::LockStatus::UnlockedItem,
lockOwnerType);
Expand Down Expand Up @@ -744,6 +745,7 @@
_accountState->account()->setLockFileState(remoteFilePath,
remotePathTrailingSlash(),
path(),
rec._etag,
journalDb(),
SyncFileItem::LockStatus::LockedItem,
SyncFileItem::LockOwnerType::TokenLock);
Expand Down
1 change: 1 addition & 0 deletions src/gui/socketapi/socketapi.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
3 changes: 2 additions & 1 deletion src/libsync/account.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
/*

Check notice on line 1 in src/libsync/account.cpp

View workflow job for this annotation

GitHub Actions / build

Run clang-format on src/libsync/account.cpp 

File src/libsync/account.cpp does not conform to Custom style guidelines. (lines 997)
* Copyright (C) by Daniel Molkentin <[email protected]>
*
* This program is free software; you can redistribute it and/or modify
Expand Down Expand Up @@ -983,6 +983,7 @@
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)
Expand All @@ -993,7 +994,7 @@
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();
Expand Down
2 changes: 1 addition & 1 deletion src/libsync/account.h
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
/*

Check notice on line 1 in src/libsync/account.h

View workflow job for this annotation

GitHub Actions / build

Run clang-format on src/libsync/account.h 

File src/libsync/account.h does not conform to Custom style guidelines. (lines 348)
* Copyright (C) by Daniel Molkentin <[email protected]>
*
* This program is free software; you can redistribute it and/or modify
Expand Down Expand Up @@ -345,7 +345,7 @@

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);
Expand Down
8 changes: 7 additions & 1 deletion src/libsync/lockfilejobs.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
/*

Check notice on line 1 in src/libsync/lockfilejobs.cpp

View workflow job for this annotation

GitHub Actions / build

Run clang-format on src/libsync/lockfilejobs.cpp 

File src/libsync/lockfilejobs.cpp does not conform to Custom style guidelines. (lines 88)
* Copyright (C) by Matthieu Gallien <[email protected]>
*
* This program is free software; you can redistribute it and/or modify
Expand Down Expand Up @@ -30,6 +30,7 @@
const QString &path,
const QString &remoteSyncPathWithTrailingSlash,
const QString &localSyncPath,
const QString &etag,
const SyncFileItem::LockStatus requestedLockState,
const SyncFileItem::LockOwnerType lockOwnerType,
QObject *parent)
Expand All @@ -39,6 +40,7 @@
, _requestedLockOwnerType(lockOwnerType)
, _remoteSyncPathWithTrailingSlash(remoteSyncPathWithTrailingSlash)
, _localSyncPath(localSyncPath)
, _existingEtag(etag)
{
if (!_localSyncPath.endsWith(QLatin1Char('/'))) {
_localSyncPath.append(QLatin1Char('/'));
Expand All @@ -65,8 +67,12 @@
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;
Expand All @@ -79,7 +85,7 @@
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();
Expand Down
2 changes: 2 additions & 0 deletions src/libsync/lockfilejobs.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ class OWNCLOUDSYNC_EXPORT LockFileJob : public AbstractNetworkJob
const QString &path,
const QString &remoteSyncPathWithTrailingSlash,
const QString &localSyncPath,
const QString &etag,
const SyncFileItem::LockStatus requestedLockState,
const SyncFileItem::LockOwnerType lockOwnerType,
QObject *parent = nullptr);
Expand Down Expand Up @@ -62,6 +63,7 @@ class OWNCLOUDSYNC_EXPORT LockFileJob : public AbstractNetworkJob
QString _lockToken;
QString _remoteSyncPathWithTrailingSlash;
QString _localSyncPath;
QString _existingEtag;
};

}
Expand Down
14 changes: 14 additions & 0 deletions test/testlockfile.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
#include "lockfilejobs.h"

Check failure on line 1 in test/testlockfile.cpp

View workflow job for this annotation

GitHub Actions / build

test/testlockfile.cpp:1:10 [clang-diagnostic-error] 

'lockfilejobs.h' file not found

#include "account.h"
#include "accountstate.h"
Expand Down Expand Up @@ -43,6 +43,7 @@
fakeFolder.account()->setLockFileState(QStringLiteral("/") + testFileName,
QStringLiteral("/"),
fakeFolder.localPath(),
{},
&fakeFolder.syncJournal(),
OCC::SyncFileItem::LockStatus::LockedItem,
OCC::SyncFileItem::LockOwnerType::UserLock);
Expand Down Expand Up @@ -89,6 +90,7 @@
fakeFolder.account()->setLockFileState(QStringLiteral("/") + testFileName,
QStringLiteral("/"),
fakeFolder.localPath(),
{},
&fakeFolder.syncJournal(),
OCC::SyncFileItem::LockStatus::LockedItem,
OCC::SyncFileItem::LockOwnerType::UserLock);
Expand All @@ -114,6 +116,7 @@
fakeFolder.account()->setLockFileState(QStringLiteral("/") + testFileName,
QStringLiteral("/"),
fakeFolder.localPath(),
{},
&fakeFolder.syncJournal(),
OCC::SyncFileItem::LockStatus::LockedItem,
OCC::SyncFileItem::LockOwnerType::UserLock);
Expand Down Expand Up @@ -142,6 +145,7 @@
fakeFolder.account()->setLockFileState(QStringLiteral("/") + testFileName,
QStringLiteral("/"),
fakeFolder.localPath(),
{},
&fakeFolder.syncJournal(),
OCC::SyncFileItem::LockStatus::LockedItem,
OCC::SyncFileItem::LockOwnerType::UserLock);
Expand All @@ -168,6 +172,7 @@
QStringLiteral("/") + testFileName,
QStringLiteral("/"),
fakeFolder.localPath(),
{},
OCC::SyncFileItem::LockStatus::LockedItem,
OCC::SyncFileItem::LockOwnerType::UserLock);

Expand Down Expand Up @@ -207,6 +212,7 @@
QStringLiteral("/") + testFileName,
QStringLiteral("/"),
fakeFolder.localPath(),
{},
OCC::SyncFileItem::LockStatus::LockedItem,
OCC::SyncFileItem::LockOwnerType::UserLock);

Expand All @@ -225,6 +231,7 @@
QStringLiteral("/") + testFileName,
QStringLiteral("/"),
fakeFolder.localPath(),
{},
OCC::SyncFileItem::LockStatus::UnlockedItem,
OCC::SyncFileItem::LockOwnerType::UserLock);

Expand Down Expand Up @@ -285,6 +292,7 @@
QStringLiteral("/") + testFileName,
QStringLiteral("/"),
fakeFolder.localPath(),
{},
OCC::SyncFileItem::LockStatus::LockedItem,
OCC::SyncFileItem::LockOwnerType::UserLock);

Expand Down Expand Up @@ -339,6 +347,7 @@
QStringLiteral("/") + testFileName,
QStringLiteral("/"),
fakeFolder.localPath(),
{},
OCC::SyncFileItem::LockStatus::LockedItem,
OCC::SyncFileItem::LockOwnerType::UserLock);

Expand Down Expand Up @@ -393,6 +402,7 @@
QStringLiteral("/") + testFileName,
QStringLiteral("/"),
fakeFolder.localPath(),
{},
OCC::SyncFileItem::LockStatus::UnlockedItem,
OCC::SyncFileItem::LockOwnerType::UserLock);

Expand Down Expand Up @@ -445,6 +455,7 @@
QStringLiteral("/") + testFileName,
QStringLiteral("/"),
fakeFolder.localPath(),
{},
OCC::SyncFileItem::LockStatus::UnlockedItem,
OCC::SyncFileItem::LockOwnerType::UserLock);

Expand Down Expand Up @@ -484,6 +495,7 @@
QStringLiteral("/") + testFileName,
QStringLiteral("/"),
fakeFolder.localPath(),
{},
OCC::SyncFileItem::LockStatus::LockedItem,
OCC::SyncFileItem::LockOwnerType::UserLock);

Expand All @@ -502,6 +514,7 @@
QStringLiteral("/") + testFileName,
QStringLiteral("/"),
fakeFolder.localPath(),
{},
OCC::SyncFileItem::LockStatus::UnlockedItem,
OCC::SyncFileItem::LockOwnerType::UserLock);

Expand Down Expand Up @@ -556,6 +569,7 @@
QStringLiteral("/") + testFileName,
QStringLiteral("/"),
fakeFolder.localPath(),
{},
OCC::SyncFileItem::LockStatus::UnlockedItem,
OCC::SyncFileItem::LockOwnerType::UserLock);

Expand Down
Loading