From 4f4fd60aff6d964c4a06f08a2ff4697404ad35a8 Mon Sep 17 00:00:00 2001 From: Tamino Bauknecht Date: Tue, 31 Oct 2023 19:37:06 +0100 Subject: [PATCH 01/34] gitignore: Ignore clangd-generated directory .cache/ Signed-off-by: Tamino Bauknecht --- .gitignore | 3 +++ 1 file changed, 3 insertions(+) diff --git a/.gitignore b/.gitignore index 85a87ed91464e..c8b8ee783160b 100644 --- a/.gitignore +++ b/.gitignore @@ -51,6 +51,9 @@ bld/ [Tt]est[Rr]esult*/ [Bb]uild[Ll]og.* +# clangd cache directory +.cache/ + #NUNIT *.VisualState.xml TestResult.xml From ed5fdd83a84b5cae4fad40cf451ddb5a7d8bff3e Mon Sep 17 00:00:00 2001 From: Tamino Bauknecht Date: Tue, 31 Oct 2023 19:37:37 +0100 Subject: [PATCH 02/34] cmake: Export compile_commands.json by default Signed-off-by: Tamino Bauknecht --- CMakeLists.txt | 2 ++ 1 file changed, 2 insertions(+) diff --git a/CMakeLists.txt b/CMakeLists.txt index ee8bc487a8b14..8b5b43e0407d7 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -7,6 +7,8 @@ if (CLANG_TIDY_EXE) set(CMAKE_CXX_CLANG_TIDY ${CLANG_TIDY_EXE} -checks=-*,modernize-use-auto,modernize-use-using,modernize-use-nodiscard,modernize-use-nullptr,modernize-use-override,cppcoreguidelines-pro-type-static-cast-downcast,modernize-use-default-member-init,cppcoreguidelines-pro-type-member-init,cppcoreguidelines-init-variables) endif() +set(CMAKE_EXPORT_COMPILE_COMMANDS ON) + project(client) include(FeatureSummary) From 5812d5cbd47a9fa5b39931975b83ed17e38393fe Mon Sep 17 00:00:00 2001 From: Tamino Bauknecht Date: Tue, 31 Oct 2023 19:40:01 +0100 Subject: [PATCH 03/34] libsync: Remove auto-exclude for symlinks Signed-off-by: Tamino Bauknecht --- src/libsync/discovery.cpp | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/src/libsync/discovery.cpp b/src/libsync/discovery.cpp index de62a8075065b..3b50bc56977ac 100644 --- a/src/libsync/discovery.cpp +++ b/src/libsync/discovery.cpp @@ -306,7 +306,7 @@ bool ProcessDirectoryJob::handleExcluded(const QString &path, const Entries &ent } } - if (excluded == CSYNC_NOT_EXCLUDED && !entries.localEntry.isSymLink) { + if (excluded == CSYNC_NOT_EXCLUDED) { return false; } else if (excluded == CSYNC_FILE_SILENTLY_EXCLUDED || excluded == CSYNC_FILE_EXCLUDE_AND_REMOVE) { emit _discoveryData->silentlyExcluded(path); @@ -327,8 +327,6 @@ bool ProcessDirectoryJob::handleExcluded(const QString &path, const Entries &ent } if (entries.localEntry.isSymLink) { - /* Symbolic links are ignored. */ - item->_errorString = tr("Symbolic links are not supported in syncing."); } else { switch (excluded) { case CSYNC_NOT_EXCLUDED: From 40145c913ede7e7e58d8c82e125f5960caf46837 Mon Sep 17 00:00:00 2001 From: Tamino Bauknecht Date: Tue, 31 Oct 2023 19:42:31 +0100 Subject: [PATCH 04/34] libsync/common: Store actual type for items Before, the common assumption was being made that if an item is not a directory, it must be a file (or the other way around). Signed-off-by: Tamino Bauknecht --- src/common/syncjournalfilerecord.h | 1 + src/libsync/discovery.cpp | 20 +++++++++++++++----- src/libsync/owncloudpropagator.cpp | 2 +- 3 files changed, 17 insertions(+), 6 deletions(-) diff --git a/src/common/syncjournalfilerecord.h b/src/common/syncjournalfilerecord.h index 7270fac137962..cf292b7bba6d5 100644 --- a/src/common/syncjournalfilerecord.h +++ b/src/common/syncjournalfilerecord.h @@ -65,6 +65,7 @@ class OCSYNC_EXPORT SyncJournalFileRecord [[nodiscard]] QDateTime modDateTime() const { return Utility::qDateTimeFromTime_t(_modtime); } [[nodiscard]] bool isDirectory() const { return _type == ItemTypeDirectory; } + [[nodiscard]] bool isSymLink() const { return _type == ItemTypeSoftLink; } [[nodiscard]] bool isFile() const { return _type == ItemTypeFile || _type == ItemTypeVirtualFileDehydration; } [[nodiscard]] bool isVirtualFile() const { return _type == ItemTypeVirtualFile || _type == ItemTypeVirtualFileDownload; } [[nodiscard]] QString path() const { return QString::fromUtf8(_path); } diff --git a/src/libsync/discovery.cpp b/src/libsync/discovery.cpp index 3b50bc56977ac..887f98b3362fe 100644 --- a/src/libsync/discovery.cpp +++ b/src/libsync/discovery.cpp @@ -1068,8 +1068,18 @@ void ProcessDirectoryJob::processFileAnalyzeLocalInfo( item->_inode = localEntry.inode; + + auto getItemType = [](const LocalInfo& localEntry, bool allowVirtualFile = false) { + return localEntry.isDirectory ? ItemTypeDirectory : + localEntry.isSymLink ? ItemTypeSoftLink : + localEntry.isVirtualFile && allowVirtualFile ? ItemTypeVirtualFile : + ItemTypeFile; + }; + + if (dbEntry.isValid()) { - bool typeChange = localEntry.isDirectory != dbEntry.isDirectory(); + bool typeChange = localEntry.isDirectory != dbEntry.isDirectory() || + localEntry.isSymLink != dbEntry.isSymLink(); if (!typeChange && localEntry.isVirtualFile) { if (noServerEntry) { item->_instruction = CSYNC_INSTRUCTION_REMOVE; @@ -1139,7 +1149,7 @@ void ProcessDirectoryJob::processFileAnalyzeLocalInfo( item->_checksumHeader.clear(); item->_size = localEntry.size; item->_modtime = localEntry.modtime; - item->_type = localEntry.isDirectory ? ItemTypeDirectory : ItemTypeFile; + item->_type = getItemType(localEntry); _childModified = true; } else if (dbEntry._modtime > 0 && (localEntry.modtime <= 0 || localEntry.modtime >= 0xFFFFFFFF) && dbEntry._fileSize == localEntry.size) { item->_instruction = CSYNC_INSTRUCTION_UPDATE_METADATA; @@ -1147,7 +1157,7 @@ void ProcessDirectoryJob::processFileAnalyzeLocalInfo( item->_size = localEntry.size > 0 ? localEntry.size : dbEntry._fileSize; item->_modtime = dbEntry._modtime; item->_previousModtime = dbEntry._modtime; - item->_type = localEntry.isDirectory ? ItemTypeDirectory : ItemTypeFile; + item->_type = getItemType(localEntry); qCDebug(lcDisco) << "CSYNC_INSTRUCTION_SYNC: File" << item->_file << "if (dbEntry._modtime > 0 && localEntry.modtime <= 0)" << "dbEntry._modtime:" << dbEntry._modtime << "localEntry.modtime:" << localEntry.modtime; @@ -1209,7 +1219,7 @@ void ProcessDirectoryJob::processFileAnalyzeLocalInfo( item->_checksumHeader.clear(); item->_size = localEntry.size; item->_modtime = localEntry.modtime; - item->_type = localEntry.isDirectory ? ItemTypeDirectory : localEntry.isVirtualFile ? ItemTypeVirtualFile : ItemTypeFile; + item->_type = getItemType(localEntry, true); _childModified = true; if (!localEntry.caseClashConflictingName.isEmpty()) { @@ -1322,7 +1332,7 @@ void ProcessDirectoryJob::processFileAnalyzeLocalInfo( return false; } - if (base.isDirectory() != item->isDirectory()) { + if (item->_type != base._type) { qCInfo(lcDisco) << "Not a move, types don't match" << base._type << item->_type << localEntry.type; return false; } diff --git a/src/libsync/owncloudpropagator.cpp b/src/libsync/owncloudpropagator.cpp index 4745b4f846fbe..e2410f1714eb7 100644 --- a/src/libsync/owncloudpropagator.cpp +++ b/src/libsync/owncloudpropagator.cpp @@ -680,7 +680,7 @@ void OwncloudPropagator::startFilePropagation(const SyncFileItemPtr &item, QString &removedDirectory, QString &maybeConflictDirectory) { - if (item->_instruction == CSYNC_INSTRUCTION_TYPE_CHANGE) { + if (item->_instruction == CSYNC_INSTRUCTION_TYPE_CHANGE && item->_type == ItemTypeDirectory) { // will delete directories, so defer execution auto job = createJob(item); if (job) { From 0370594246f77d851537199ff65f2d0dc607920f Mon Sep 17 00:00:00 2001 From: Tamino Bauknecht Date: Tue, 31 Oct 2023 19:46:15 +0100 Subject: [PATCH 05/34] libsync: Add header field OC-File-Type for symlink synchronization Signed-off-by: Tamino Bauknecht --- src/libsync/propagateupload.cpp | 1 + src/libsync/propagateupload.h | 1 + src/libsync/propagateuploadv1.cpp | 1 + 3 files changed, 3 insertions(+) diff --git a/src/libsync/propagateupload.cpp b/src/libsync/propagateupload.cpp index f74727ec7bb80..8a7f1b82a481a 100644 --- a/src/libsync/propagateupload.cpp +++ b/src/libsync/propagateupload.cpp @@ -258,6 +258,7 @@ void PropagateUploadFileCommon::setupUnencryptedFile() _fileToUpload._file = _item->_file; _fileToUpload._size = _item->_size; _fileToUpload._path = propagator()->fullLocalPath(_fileToUpload._file); + _fileToUpload._isSymlink = _item->_type == ItemTypeSoftLink; startUploadFile(); } diff --git a/src/libsync/propagateupload.h b/src/libsync/propagateupload.h index 33db43f297755..9ee6e1f8162d2 100644 --- a/src/libsync/propagateupload.h +++ b/src/libsync/propagateupload.h @@ -233,6 +233,7 @@ class PropagateUploadFileCommon : public PropagateItemJob QString _file; /// I'm still unsure if I should use a SyncFilePtr here. QString _path; /// the full path on disk. qint64 _size = 0LL; + bool _isSymlink = false; }; UploadFileInfo _fileToUpload; QByteArray _transmissionChecksumHeader; diff --git a/src/libsync/propagateuploadv1.cpp b/src/libsync/propagateuploadv1.cpp index 33cd8892d7335..39d1294ed04f9 100644 --- a/src/libsync/propagateuploadv1.cpp +++ b/src/libsync/propagateuploadv1.cpp @@ -99,6 +99,7 @@ void PropagateUploadFileV1::startNextChunk() auto headers = PropagateUploadFileCommon::headers(); headers[QByteArrayLiteral("OC-Total-Length")] = QByteArray::number(fileSize); headers[QByteArrayLiteral("OC-Chunk-Size")] = QByteArray::number(chunkSize()); + headers[QByteArrayLiteral("OC-File-Type")] = QByteArray::number(_fileToUpload._isSymlink ? ItemTypeSoftLink : ItemTypeFile); QString path = _fileToUpload._file; From d66c7045f2327e48ba17dceabcad4fdf8be72ce9 Mon Sep 17 00:00:00 2001 From: Tamino Bauknecht Date: Fri, 3 Nov 2023 00:44:50 +0100 Subject: [PATCH 06/34] libsync: Add SymLinkUploadDevice for reading symlinks Signed-off-by: Tamino Bauknecht --- src/libsync/CMakeLists.txt | 2 + src/libsync/propagateupload.h | 2 +- src/libsync/symlinkuploaddevice.cpp | 108 ++++++++++++++++++++++++++++ src/libsync/symlinkuploaddevice.h | 39 ++++++++++ 4 files changed, 150 insertions(+), 1 deletion(-) create mode 100644 src/libsync/symlinkuploaddevice.cpp create mode 100644 src/libsync/symlinkuploaddevice.h diff --git a/src/libsync/CMakeLists.txt b/src/libsync/CMakeLists.txt index fab99be586e3c..7f6c53612b1cd 100644 --- a/src/libsync/CMakeLists.txt +++ b/src/libsync/CMakeLists.txt @@ -85,6 +85,8 @@ set(libsync_SRCS propagateuploadencrypted.cpp propagatedownloadencrypted.h propagatedownloadencrypted.cpp + symlinkuploaddevice.h + symlinkuploaddevice.cpp syncengine.h syncengine.cpp syncfileitem.h diff --git a/src/libsync/propagateupload.h b/src/libsync/propagateupload.h index 9ee6e1f8162d2..dc32649f2486f 100644 --- a/src/libsync/propagateupload.h +++ b/src/libsync/propagateupload.h @@ -60,7 +60,7 @@ class UploadDevice : public QIODevice signals: -private: +protected: /// The local file to read data from QFile _file; diff --git a/src/libsync/symlinkuploaddevice.cpp b/src/libsync/symlinkuploaddevice.cpp new file mode 100644 index 0000000000000..9b1e1ce7ed94d --- /dev/null +++ b/src/libsync/symlinkuploaddevice.cpp @@ -0,0 +1,108 @@ +#include "symlinkuploaddevice.h" + +#include +#include + +namespace { +// Helper function to read raw symlink target; +// QFileInfo::readSymLink() only available in Qt 6.6 or newer +// and QFileInfo::symLinkTarget() will transform path to absolute path +// which might break relative symlinks for cross-device synchronization +QString readRawSymlink(const QString& path) +{ +#if defined(Q_OS_UNIX) || defined(Q_OS_LINUX) || defined(Q_OS_MAC) + QByteArray buffer(255, '\0'); + ssize_t readLength{}; + + auto tryReadlink = [&path, &buffer]() { + return readlink(path.toUtf8().data(), buffer.data(), buffer.size()); + }; + while ((readLength = tryReadlink()) >= static_cast(buffer.size())) { + buffer.resize(buffer.size() + 100); + } + + if (readLength > 0) + { + buffer[static_cast(readLength)] = '\0'; + return QString(buffer); + } +#else + Q_UNUSED(path); +#endif + return QString(); +} +} + +namespace OCC { +SymLinkUploadDevice::SymLinkUploadDevice(const QString &fileName, qint64 start, qint64 size, BandwidthManager *bwm) + : UploadDevice(fileName, start, size, bwm) +{ +} + +bool SymLinkUploadDevice::open(QIODevice::OpenMode mode) +{ + if (mode & QIODevice::WriteOnly) + return false; + + auto symlinkContent = readRawSymlink(_file.fileName()); + if (symlinkContent.isEmpty()) { + setErrorString("Unable to read symlink '" + _file.fileName() + "'"); + return false; + } + + _symlinkContent = symlinkContent.toUtf8(); + _size = qBound(0ll, _size, _symlinkContent.size() - _start); + _read = 0; + + return QIODevice::open(mode); +} + +qint64 SymLinkUploadDevice::readData(char *data, qint64 maxlen) +{ + if (_size - _read <= 0) { + // at end + if (_bandwidthManager) { + _bandwidthManager->unregisterUploadDevice(this); + } + return -1; + } + maxlen = qMin(maxlen, _size - _read); + if (maxlen <= 0) { + return 0; + } + if (isChoked()) { + return 0; + } + if (isBandwidthLimited()) { + maxlen = qMin(maxlen, _bandwidthQuota); + if (maxlen <= 0) { // no quota + return 0; + } + _bandwidthQuota -= maxlen; + } + + auto readStart = _start + _read; + auto readEnd = _start + _size; + if (_size - _read - _start < maxlen) { + _read = _size - _start; + } else { + _read += maxlen; + readEnd = _start + _read; + } + Q_ASSERT(readEnd <= _symlinkContent.size()); + std::copy(_symlinkContent.begin() + readStart, _symlinkContent.begin() + readEnd, data); + return readEnd - readStart; +} + +bool SymLinkUploadDevice::seek(qint64 pos) +{ + if (!QIODevice::seek(pos)) { + return false; + } + if (pos < 0 || pos > _size) { + return false; + } + _read = pos; + return true; +} +} diff --git a/src/libsync/symlinkuploaddevice.h b/src/libsync/symlinkuploaddevice.h new file mode 100644 index 0000000000000..c5c76e6be7d00 --- /dev/null +++ b/src/libsync/symlinkuploaddevice.h @@ -0,0 +1,39 @@ +/* + * Copyright (C) by Tamino Bauknecht + * + * This program is free software; you can redistribute it and/or modify + * it under the terms of the GNU General Public License as published by + * the Free Software Foundation; either version 2 of the License, or + * (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, but + * WITHOUT ANY WARRANTY; without even the implied warranty of MERCHANTABILITY + * or FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License + * for more details. + */ +#pragma once + +#include "propagateupload.h" + +namespace OCC { + +/** + * @brief Symlink specialization of an UploadDevice + * @ingroup libsync + */ +class SymLinkUploadDevice : public UploadDevice +{ + Q_OBJECT + +public: + SymLinkUploadDevice(const QString &fileName, qint64 start, qint64 size, BandwidthManager *bwm); + + bool open(QIODevice::OpenMode mode) override; + + qint64 readData(char *data, qint64 maxlen) override; + bool seek(qint64 pos) override; + +protected: + QByteArray _symlinkContent; +}; +} From c34659b58246231f55e41c1c62d47529b59aa9f7 Mon Sep 17 00:00:00 2001 From: Tamino Bauknecht Date: Fri, 3 Nov 2023 00:47:57 +0100 Subject: [PATCH 07/34] libsync: Use SymLinkUploadDevice for single symlink uploads Signed-off-by: Tamino Bauknecht --- src/libsync/propagateuploadv1.cpp | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-) diff --git a/src/libsync/propagateuploadv1.cpp b/src/libsync/propagateuploadv1.cpp index 39d1294ed04f9..83b48036561bd 100644 --- a/src/libsync/propagateuploadv1.cpp +++ b/src/libsync/propagateuploadv1.cpp @@ -23,6 +23,7 @@ #include "filesystem.h" #include "propagatorjobs.h" #include "common/checksums.h" +#include "symlinkuploaddevice.h" #include "syncengine.h" #include "propagateremotedelete.h" #include "common/asserts.h" @@ -136,8 +137,14 @@ void PropagateUploadFileV1::startNextChunk() } const QString fileName = _fileToUpload._path; - auto device = std::make_unique( - fileName, chunkStart, currentChunkSize, &propagator()->_bandwidthManager); + std::unique_ptr device; + if (_fileToUpload._isSymlink) { + device = std::make_unique( + fileName, chunkStart, currentChunkSize, &propagator()->_bandwidthManager); + } else { + device = std::make_unique( + fileName, chunkStart, currentChunkSize, &propagator()->_bandwidthManager); + } if (!device->open(QIODevice::ReadOnly)) { qCWarning(lcPropagateUploadV1) << "Could not prepare upload device: " << device->errorString(); From 7b92b2ecbddf0923edfae1590b011ef55de22026 Mon Sep 17 00:00:00 2001 From: Tamino Bauknecht Date: Sat, 4 Nov 2023 20:30:30 +0100 Subject: [PATCH 08/34] libsync: Add support for symlinks in bulk uploads Use SymLinkUploadDevice for symlinks in bulk uploads and add the header field X-File-Type to recognize symlinks on the server. Signed-off-by: Tamino Bauknecht --- src/libsync/bulkpropagatorjob.cpp | 12 +++++++++++- src/libsync/syncfileitem.h | 5 +++++ 2 files changed, 16 insertions(+), 1 deletion(-) diff --git a/src/libsync/bulkpropagatorjob.cpp b/src/libsync/bulkpropagatorjob.cpp index 0f0a7db3fcfa4..3061e929564ad 100644 --- a/src/libsync/bulkpropagatorjob.cpp +++ b/src/libsync/bulkpropagatorjob.cpp @@ -16,6 +16,7 @@ #include "putmultifilejob.h" #include "owncloudpropagator_p.h" +#include "symlinkuploaddevice.h" #include "syncfileitem.h" #include "syncengine.h" #include "propagateupload.h" @@ -203,10 +204,18 @@ void BulkPropagatorJob::triggerUpload() int timeout = 0; for(auto &singleFile : _filesToUpload) { // job takes ownership of device via a QScopedPointer. Job deletes itself when finishing - auto device = std::make_unique(singleFile._localPath, + std::unique_ptr device; + if (singleFile._item->isSymLink()) { + device = std::make_unique(singleFile._localPath, + 0, + std::numeric_limits::max(), + &propagator()->_bandwidthManager); + } else { + device = std::make_unique(singleFile._localPath, 0, singleFile._fileSize, &propagator()->_bandwidthManager); + } if (!device->open(QIODevice::ReadOnly)) { qCWarning(lcBulkPropagatorJob) << "Could not prepare upload device: " << device->errorString(); @@ -224,6 +233,7 @@ void BulkPropagatorJob::triggerUpload() } singleFile._headers["X-File-Path"] = singleFile._remotePath.toUtf8(); + singleFile._headers["X-File-Type"] = QString::number(singleFile._item->_type == ItemTypeSoftLink).toUtf8(); uploadParametersData.push_back({std::move(device), singleFile._headers}); timeout += singleFile._fileSize; } diff --git a/src/libsync/syncfileitem.h b/src/libsync/syncfileitem.h index 399546dcd0459..3777a4de42d71 100644 --- a/src/libsync/syncfileitem.h +++ b/src/libsync/syncfileitem.h @@ -205,6 +205,11 @@ class OWNCLOUDSYNC_EXPORT SyncFileItem return _type == ItemTypeDirectory; } + [[nodiscard]] bool isSymLink() const + { + return _type == ItemTypeSoftLink; + } + /** * True if the item had any kind of error. */ From 390e8f61f11cc64dc1eb359ff108c720171a2484 Mon Sep 17 00:00:00 2001 From: Tamino Bauknecht Date: Sun, 5 Nov 2023 22:51:09 +0100 Subject: [PATCH 09/34] libsync: Fix _read for SymLinkUploadDevice Signed-off-by: Tamino Bauknecht --- src/libsync/symlinkuploaddevice.cpp | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/libsync/symlinkuploaddevice.cpp b/src/libsync/symlinkuploaddevice.cpp index 9b1e1ce7ed94d..beefa6d1f25a2 100644 --- a/src/libsync/symlinkuploaddevice.cpp +++ b/src/libsync/symlinkuploaddevice.cpp @@ -82,9 +82,9 @@ qint64 SymLinkUploadDevice::readData(char *data, qint64 maxlen) } auto readStart = _start + _read; - auto readEnd = _start + _size; - if (_size - _read - _start < maxlen) { - _read = _size - _start; + auto readEnd = _size; + if (_size - _read < maxlen) { + _read = _size; } else { _read += maxlen; readEnd = _start + _read; From af53d58683dc53d151b5d4f4a7fce927e4bfcd47 Mon Sep 17 00:00:00 2001 From: Tamino Bauknecht Date: Sun, 5 Nov 2023 22:54:36 +0100 Subject: [PATCH 10/34] libsync: Allow download of symlinks Signed-off-by: Tamino Bauknecht --- src/libsync/propagatedownload.cpp | 51 +++++++++++++++++++------------ 1 file changed, 31 insertions(+), 20 deletions(-) diff --git a/src/libsync/propagatedownload.cpp b/src/libsync/propagatedownload.cpp index 6d7d5e2ebee0b..2bb1423829838 100644 --- a/src/libsync/propagatedownload.cpp +++ b/src/libsync/propagatedownload.cpp @@ -321,13 +321,22 @@ void GETFileJob::slotReadyRead() return; } - const qint64 writtenBytes = writeToDevice(QByteArray::fromRawData(buffer.constData(), readBytes)); - if (writtenBytes != readBytes) { - _errorString = _device->errorString(); - _errorStatus = SyncFileItem::NormalError; - qCWarning(lcGetJob) << "Error while writing to file" << writtenBytes << readBytes << _errorString; - reply()->abort(); - return; + if (reply()->hasRawHeader("OC-File-Type") && + reply()->rawHeader("OC-File-Type").toInt() == ItemTypeSoftLink) { + // TODO: handle non-Unix OS + auto file_device = static_cast(_device); + file_device->close(); + FileSystem::remove(file_device->fileName()); + QFile::link(QString(buffer), file_device->fileName()); + } else { + const qint64 writtenBytes = writeToDevice(QByteArray::fromRawData(buffer.constData(), readBytes)); + if (writtenBytes != readBytes) { + _errorString = _device->errorString(); + _errorStatus = SyncFileItem::NormalError; + qCWarning(lcGetJob) << "Error while writing to file" << writtenBytes << readBytes << _errorString; + reply()->abort(); + return; + } } } @@ -789,7 +798,7 @@ void PropagateDownloadFile::slotGetFinished() // Don't keep the temporary file if it is empty or we // used a bad range header or the file's not on the server anymore. - if (_tmpFile.exists() && (_tmpFile.size() == 0 || badRangeHeader || fileNotFound)) { + if (!QFileInfo(_tmpFile).isSymLink() && _tmpFile.exists() && (_tmpFile.size() == 0 || badRangeHeader || fileNotFound)) { _tmpFile.close(); FileSystem::remove(_tmpFile.fileName()); propagator()->_journal->setDownloadInfo(_item->_file, SyncJournalDb::DownloadInfo()); @@ -885,19 +894,21 @@ void PropagateDownloadFile::slotGetFinished() return; } - if (bodySize > 0 && bodySize != _tmpFile.size() - job->resumeStart()) { - qCDebug(lcPropagateDownload) << bodySize << _tmpFile.size() << job->resumeStart(); - propagator()->_anotherSyncNeeded = true; - done(SyncFileItem::SoftError, tr("The file could not be downloaded completely."), ErrorCategory::GenericError); - return; - } + if (!QFileInfo(_tmpFile).isSymLink()) { + if (bodySize > 0 && bodySize != _tmpFile.size() - job->resumeStart()) { + qCDebug(lcPropagateDownload) << bodySize << _tmpFile.size() << job->resumeStart(); + propagator()->_anotherSyncNeeded = true; + done(SyncFileItem::SoftError, tr("The file could not be downloaded completely."), ErrorCategory::GenericError); + return; + } - if (_tmpFile.size() == 0 && _item->_size > 0) { - FileSystem::remove(_tmpFile.fileName()); - done(SyncFileItem::NormalError, - tr("The downloaded file is empty, but the server said it should have been %1.") - .arg(Utility::octetsToString(_item->_size)), ErrorCategory::GenericError); - return; + if (_tmpFile.size() == 0 && _item->_size > 0) { + FileSystem::remove(_tmpFile.fileName()); + done(SyncFileItem::NormalError, + tr("The downloaded file is empty, but the server said it should have been %1.") + .arg(Utility::octetsToString(_item->_size)), ErrorCategory::GenericError); + return; + } } // Did the file come with conflict headers? If so, store them now! From aee660159a3869fc736af1fac53aa719c71bebf1 Mon Sep 17 00:00:00 2001 From: Tamino Bauknecht Date: Mon, 13 Nov 2023 00:20:06 +0100 Subject: [PATCH 11/34] libsync: Fix Content-Length for symlinks in bulk upload Signed-off-by: Tamino Bauknecht --- src/libsync/bulkpropagatorjob.cpp | 1 + 1 file changed, 1 insertion(+) diff --git a/src/libsync/bulkpropagatorjob.cpp b/src/libsync/bulkpropagatorjob.cpp index 3061e929564ad..10eecfe585845 100644 --- a/src/libsync/bulkpropagatorjob.cpp +++ b/src/libsync/bulkpropagatorjob.cpp @@ -210,6 +210,7 @@ void BulkPropagatorJob::triggerUpload() 0, std::numeric_limits::max(), &propagator()->_bandwidthManager); + singleFile._headers["Content-Length"] = QByteArray::number(device->size()); } else { device = std::make_unique(singleFile._localPath, 0, From a551b2453dc5f9cf9664b7b0705d646061d5c34e Mon Sep 17 00:00:00 2001 From: Tamino Bauknecht Date: Wed, 15 Nov 2023 21:12:21 +0100 Subject: [PATCH 12/34] common: Do not dereference symlink when checking existence Signed-off-by: Tamino Bauknecht --- src/common/filesystembase.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/common/filesystembase.cpp b/src/common/filesystembase.cpp index ef9ba1efd6778..be8c2dea4f0d9 100644 --- a/src/common/filesystembase.cpp +++ b/src/common/filesystembase.cpp @@ -312,13 +312,13 @@ bool FileSystem::fileExists(const QString &filename, const QFileInfo &fileInfo) return fileExistsWin(filename); } #endif - bool re = fileInfo.exists(); + bool re = fileInfo.exists() || fileInfo.isSymLink(); // if the filename is different from the filename in fileInfo, the fileInfo is // not valid. There needs to be one initialised here. Otherwise the incoming // fileInfo is re-used. if (fileInfo.filePath() != filename) { QFileInfo myFI(filename); - re = myFI.exists(); + re = myFI.exists() || myFI.isSymLink(); } return re; } From 7876341789221033faa3fd0128b970f0ae028b14 Mon Sep 17 00:00:00 2001 From: Tamino Bauknecht Date: Wed, 15 Nov 2023 21:17:50 +0100 Subject: [PATCH 13/34] libsync: Fix checksum and size for symlinks in bulk upload Signed-off-by: Tamino Bauknecht --- src/libsync/bulkpropagatorjob.cpp | 31 ++++++++++++++++++++++--------- 1 file changed, 22 insertions(+), 9 deletions(-) diff --git a/src/libsync/bulkpropagatorjob.cpp b/src/libsync/bulkpropagatorjob.cpp index 10eecfe585845..73f4463e8a85f 100644 --- a/src/libsync/bulkpropagatorjob.cpp +++ b/src/libsync/bulkpropagatorjob.cpp @@ -210,7 +210,6 @@ void BulkPropagatorJob::triggerUpload() 0, std::numeric_limits::max(), &propagator()->_bandwidthManager); - singleFile._headers["Content-Length"] = QByteArray::number(device->size()); } else { device = std::make_unique(singleFile._localPath, 0, @@ -234,7 +233,7 @@ void BulkPropagatorJob::triggerUpload() } singleFile._headers["X-File-Path"] = singleFile._remotePath.toUtf8(); - singleFile._headers["X-File-Type"] = QString::number(singleFile._item->_type == ItemTypeSoftLink).toUtf8(); + singleFile._headers["OC-File-Type"] = QString::number(singleFile._item->_type).toUtf8(); uploadParametersData.push_back({std::move(device), singleFile._headers}); timeout += singleFile._fileSize; } @@ -282,12 +281,24 @@ void BulkPropagatorJob::slotComputeTransmissionChecksum(SyncFileItemPtr item, const auto checksumType = uploadChecksumEnabled() ? "MD5" : ""; computeChecksum->setChecksumType(checksumType); - connect(computeChecksum, &ComputeChecksum::done, this, [this, item, fileToUpload] (const QByteArray &contentChecksumType, const QByteArray &contentChecksum) { - slotStartUpload(item, fileToUpload, contentChecksumType, contentChecksum); - }); - connect(computeChecksum, &ComputeChecksum::done, computeChecksum, &QObject::deleteLater); + if (item->_type == CSyncEnums::ItemTypeSoftLink) { + auto checksumDevice = QSharedPointer::create(fileToUpload._path, + 0, + std::numeric_limits::max(), + &propagator()->_bandwidthManager); + const auto contentChecksum = ComputeChecksum().computeNow(checksumDevice, checksumType); + item->_size = checksumDevice->size(); + fileToUpload._size = checksumDevice->size(); + slotStartUpload(item, fileToUpload, checksumType, contentChecksum); + computeChecksum->deleteLater(); + } else { + connect(computeChecksum, &ComputeChecksum::done, this, [this, item, fileToUpload] (const QByteArray &contentChecksumType, const QByteArray &contentChecksum) { + slotStartUpload(item, fileToUpload, contentChecksumType, contentChecksum); + }); + connect(computeChecksum, &ComputeChecksum::done, computeChecksum, &QObject::deleteLater); - computeChecksum->start(fileToUpload._path); + computeChecksum->start(fileToUpload._path); + } } void BulkPropagatorJob::slotStartUpload(SyncFileItemPtr item, @@ -333,8 +344,10 @@ void BulkPropagatorJob::slotStartUpload(SyncFileItemPtr item, return; } - fileToUpload._size = FileSystem::getSize(fullFilePath); - item->_size = FileSystem::getSize(originalFilePath); + if (!item->isSymLink()) { + fileToUpload._size = FileSystem::getSize(fullFilePath); + item->_size = FileSystem::getSize(originalFilePath); + } // But skip the file if the mtime is too close to 'now'! // That usually indicates a file that is still being changed From c5b8235de6d21253e760410fdca2f5b1b8f4126d Mon Sep 17 00:00:00 2001 From: Tamino Bauknecht Date: Thu, 23 Nov 2023 21:16:32 +0100 Subject: [PATCH 14/34] libsync: Add symlink type to DiscoveryPhase Signed-off-by: Tamino Bauknecht --- src/libsync/discovery.cpp | 2 +- src/libsync/discoveryphase.cpp | 1 + src/libsync/discoveryphase.h | 1 + 3 files changed, 3 insertions(+), 1 deletion(-) diff --git a/src/libsync/discovery.cpp b/src/libsync/discovery.cpp index 887f98b3362fe..fe12fe80091ee 100644 --- a/src/libsync/discovery.cpp +++ b/src/libsync/discovery.cpp @@ -711,7 +711,7 @@ void ProcessDirectoryJob::processFileAnalyzeRemoteInfo(const SyncFileItemPtr &it _discoveryData->checkSelectiveSyncExistingFolder(path._server); } - if (serverEntry.isDirectory != dbEntry.isDirectory()) { + if (serverEntry.isSymLink != dbEntry.isSymLink() || serverEntry.isDirectory != dbEntry.isDirectory()) { // If the type of the entity changed, it's like NEW, but // needs to delete the other entity first. item->_instruction = CSYNC_INSTRUCTION_TYPE_CHANGE; diff --git a/src/libsync/discoveryphase.cpp b/src/libsync/discoveryphase.cpp index 77a10b4fe244e..740e302e15918 100644 --- a/src/libsync/discoveryphase.cpp +++ b/src/libsync/discoveryphase.cpp @@ -442,6 +442,7 @@ static void propertyMapToRemoteInfo(const QMap &map, RemoteInf QString value = it.value(); if (property == QLatin1String("resourcetype")) { result.isDirectory = value.contains(QLatin1String("collection")); + result.isSymLink = value.contains(QLatin1String("symlink")); } else if (property == QLatin1String("getlastmodified")) { const auto date = QDateTime::fromString(value, Qt::RFC2822Date); Q_ASSERT(date.isValid()); diff --git a/src/libsync/discoveryphase.h b/src/libsync/discoveryphase.h index 561e9ee9f08d6..b4b74d9102d10 100644 --- a/src/libsync/discoveryphase.h +++ b/src/libsync/discoveryphase.h @@ -68,6 +68,7 @@ struct RemoteInfo int64_t size = 0; int64_t sizeOfFolder = 0; bool isDirectory = false; + bool isSymLink = false; bool _isE2eEncrypted = false; bool isFileDropDetected = false; QString e2eMangledName; From 10d0fcc7fbf3e158e7a12a47b4a678a42239ebfc Mon Sep 17 00:00:00 2001 From: Tamino Bauknecht Date: Mon, 27 Nov 2023 16:46:56 +0100 Subject: [PATCH 15/34] common: Allow checksum calculation on QIODevice again This is required to be able to calculate the checksum for symlinks without code duplication. The commit reverts some of the removal done in commit dd178f032c704b01a33ed1e22abccbf337730332. Signed-off-by: Tamino Bauknecht --- src/common/checksumcalculator.cpp | 8 +++++++- src/common/checksumcalculator.h | 6 +++--- src/common/checksums.cpp | 11 +++++++++++ src/common/checksums.h | 1 + src/libsync/bulkpropagatorjob.cpp | 10 +++++----- 5 files changed, 27 insertions(+), 9 deletions(-) diff --git a/src/common/checksumcalculator.cpp b/src/common/checksumcalculator.cpp index ec9faa2de8c5a..114729ee92a38 100644 --- a/src/common/checksumcalculator.cpp +++ b/src/common/checksumcalculator.cpp @@ -12,6 +12,7 @@ * for more details. */ #include "checksumcalculator.h" +#include "checksumconsts.h" #include @@ -48,7 +49,12 @@ static QCryptographicHash::Algorithm algorithmTypeToQCryptoHashAlgorithm(Checksu } ChecksumCalculator::ChecksumCalculator(const QString &filePath, const QByteArray &checksumTypeName) - : _device(new QFile(filePath)) + : ChecksumCalculator(QSharedPointer(new QFile(filePath)), checksumTypeName) +{ +} + +ChecksumCalculator::ChecksumCalculator(QSharedPointer fileDevice, const QByteArray &checksumTypeName) + : _device(fileDevice) { if (checksumTypeName == checkSumMD5C) { _algorithmType = AlgorithmType::MD5; diff --git a/src/common/checksumcalculator.h b/src/common/checksumcalculator.h index da1263abeee57..cd1b580b32915 100644 --- a/src/common/checksumcalculator.h +++ b/src/common/checksumcalculator.h @@ -16,13 +16,12 @@ #include "ocsynclib.h" #include "config.h" -#include "checksumconsts.h" #include #include #include #include -#include +#include class QCryptographicHash; @@ -42,13 +41,14 @@ class OCSYNC_EXPORT ChecksumCalculator }; ChecksumCalculator(const QString &filePath, const QByteArray &checksumTypeName); + ChecksumCalculator(QSharedPointer fileDevice, const QByteArray &checksumTypeName); ~ChecksumCalculator(); [[nodiscard]] QByteArray calculate(); private: void initChecksumAlgorithm(); bool addChunk(const QByteArray &chunk, const qint64 size); - QScopedPointer _device; + QSharedPointer _device; QScopedPointer _cryptographicHash; unsigned int _adlerHash = 0; bool _isInitialized = false; diff --git a/src/common/checksums.cpp b/src/common/checksums.cpp index 370730c3c3461..106b1d99530d1 100644 --- a/src/common/checksums.cpp +++ b/src/common/checksums.cpp @@ -218,6 +218,17 @@ QByteArray ComputeChecksum::computeNow(const QString &filePath, const QByteArray return checksumCalculator.calculate(); } +QByteArray ComputeChecksum::computeNow(QSharedPointer fileDevice, const QByteArray &checksumType) +{ + if (!checksumComputationEnabled()) { + qCWarning(lcChecksums) << "Checksum computation disabled by environment variable"; + return QByteArray(); + } + + ChecksumCalculator checksumCalculator(fileDevice, checksumType); + return checksumCalculator.calculate(); +} + void ComputeChecksum::slotCalculationDone() { QByteArray checksum = _watcher.future().result(); diff --git a/src/common/checksums.h b/src/common/checksums.h index bd13dc07fb82e..42de22bedccfb 100644 --- a/src/common/checksums.h +++ b/src/common/checksums.h @@ -85,6 +85,7 @@ class OCSYNC_EXPORT ComputeChecksum : public QObject * Computes the checksum synchronously. */ static QByteArray computeNow(const QString &filePath, const QByteArray &checksumType); + static QByteArray computeNow(QSharedPointer fileDevice, const QByteArray &checksumType); /** * Computes the checksum synchronously on file. Convenience wrapper for computeNow(). diff --git a/src/libsync/bulkpropagatorjob.cpp b/src/libsync/bulkpropagatorjob.cpp index 73f4463e8a85f..3f2407595664a 100644 --- a/src/libsync/bulkpropagatorjob.cpp +++ b/src/libsync/bulkpropagatorjob.cpp @@ -282,15 +282,15 @@ void BulkPropagatorJob::slotComputeTransmissionChecksum(SyncFileItemPtr item, computeChecksum->setChecksumType(checksumType); if (item->_type == CSyncEnums::ItemTypeSoftLink) { + computeChecksum->deleteLater(); auto checksumDevice = QSharedPointer::create(fileToUpload._path, - 0, - std::numeric_limits::max(), - &propagator()->_bandwidthManager); - const auto contentChecksum = ComputeChecksum().computeNow(checksumDevice, checksumType); + 0, + std::numeric_limits::max(), + &propagator()->_bandwidthManager); + const auto contentChecksum = ComputeChecksum::computeNow(checksumDevice, checksumType); item->_size = checksumDevice->size(); fileToUpload._size = checksumDevice->size(); slotStartUpload(item, fileToUpload, checksumType, contentChecksum); - computeChecksum->deleteLater(); } else { connect(computeChecksum, &ComputeChecksum::done, this, [this, item, fileToUpload] (const QByteArray &contentChecksumType, const QByteArray &contentChecksum) { slotStartUpload(item, fileToUpload, contentChecksumType, contentChecksum); From e7c357cbf1004b7882d4f07aecf12de759ac067a Mon Sep 17 00:00:00 2001 From: Tamino Bauknecht Date: Mon, 27 Nov 2023 18:19:46 +0100 Subject: [PATCH 16/34] libsync: Add readlink to FileSystem and fix getSize for symlinks Signed-off-by: Tamino Bauknecht --- src/libsync/filesystem.cpp | 16 +++++++++++- src/libsync/filesystem.h | 10 ++++++++ src/libsync/symlinkuploaddevice.cpp | 38 +++-------------------------- 3 files changed, 29 insertions(+), 35 deletions(-) diff --git a/src/libsync/filesystem.cpp b/src/libsync/filesystem.cpp index 7c58c7b2bde45..12eb1349423af 100644 --- a/src/libsync/filesystem.cpp +++ b/src/libsync/filesystem.cpp @@ -15,6 +15,7 @@ #include "filesystem.h" #include "common/utility.h" +#include #include #include #include @@ -128,7 +129,20 @@ qint64 FileSystem::getSize(const QString &filename) return getSizeWithCsync(filename); } #endif - return QFileInfo(filename).size(); + QFileInfo info(filename); + if (info.isSymLink()) { + return readlink(filename).size(); + } + return info.size(); +} + +QByteArray FileSystem::readlink(const QString &filename) +{ + if (!QFileInfo(filename).isSymLink()) { + return QByteArray(); + } + auto symlinkContent = std::filesystem::read_symlink(filename.toStdString()).u8string(); + return QByteArray(symlinkContent.data()); } // Code inspired from Qt5's QDir::removeRecursively diff --git a/src/libsync/filesystem.h b/src/libsync/filesystem.h index 602f74bb7f2bc..5129e8ad6dc64 100644 --- a/src/libsync/filesystem.h +++ b/src/libsync/filesystem.h @@ -68,6 +68,16 @@ namespace FileSystem { */ bool OWNCLOUDSYNC_EXPORT getInode(const QString &filename, quint64 *inode); + /** + * @brief Return raw content of symlink at given path. + * + * If the file is not a symlink or does not exist, the returned string will be empty. + * In Qt6.6+, QFileInfo::readSymLink() can be used instead. + * QFileInfo::symLinkTarget() can *not* be used because it transforms the target to an + * absolute path which might break relative symlinks for cross-device synchronization. + */ + QByteArray OWNCLOUDSYNC_EXPORT readlink(const QString &filename); + /** * @brief Check if \a fileName has changed given previous size and mtime * diff --git a/src/libsync/symlinkuploaddevice.cpp b/src/libsync/symlinkuploaddevice.cpp index beefa6d1f25a2..c7612abcd207b 100644 --- a/src/libsync/symlinkuploaddevice.cpp +++ b/src/libsync/symlinkuploaddevice.cpp @@ -1,37 +1,8 @@ #include "symlinkuploaddevice.h" -#include -#include - -namespace { -// Helper function to read raw symlink target; -// QFileInfo::readSymLink() only available in Qt 6.6 or newer -// and QFileInfo::symLinkTarget() will transform path to absolute path -// which might break relative symlinks for cross-device synchronization -QString readRawSymlink(const QString& path) -{ -#if defined(Q_OS_UNIX) || defined(Q_OS_LINUX) || defined(Q_OS_MAC) - QByteArray buffer(255, '\0'); - ssize_t readLength{}; - - auto tryReadlink = [&path, &buffer]() { - return readlink(path.toUtf8().data(), buffer.data(), buffer.size()); - }; - while ((readLength = tryReadlink()) >= static_cast(buffer.size())) { - buffer.resize(buffer.size() + 100); - } +#include "filesystem.h" - if (readLength > 0) - { - buffer[static_cast(readLength)] = '\0'; - return QString(buffer); - } -#else - Q_UNUSED(path); -#endif - return QString(); -} -} +#include namespace OCC { SymLinkUploadDevice::SymLinkUploadDevice(const QString &fileName, qint64 start, qint64 size, BandwidthManager *bwm) @@ -44,13 +15,12 @@ bool SymLinkUploadDevice::open(QIODevice::OpenMode mode) if (mode & QIODevice::WriteOnly) return false; - auto symlinkContent = readRawSymlink(_file.fileName()); - if (symlinkContent.isEmpty()) { + _symlinkContent = FileSystem::readlink(_file.fileName()); + if (_symlinkContent.isEmpty()) { setErrorString("Unable to read symlink '" + _file.fileName() + "'"); return false; } - _symlinkContent = symlinkContent.toUtf8(); _size = qBound(0ll, _size, _symlinkContent.size() - _start); _read = 0; From 719e014c918f38c459cbbe60bd7f9983c4bcab33 Mon Sep 17 00:00:00 2001 From: Tamino Bauknecht Date: Tue, 28 Nov 2023 21:07:58 +0100 Subject: [PATCH 17/34] BulkPropagatorJob: Remove symlink size special case Signed-off-by: Tamino Bauknecht --- src/libsync/bulkpropagatorjob.cpp | 8 ++------ 1 file changed, 2 insertions(+), 6 deletions(-) diff --git a/src/libsync/bulkpropagatorjob.cpp b/src/libsync/bulkpropagatorjob.cpp index 3f2407595664a..0943c199fe67e 100644 --- a/src/libsync/bulkpropagatorjob.cpp +++ b/src/libsync/bulkpropagatorjob.cpp @@ -288,8 +288,6 @@ void BulkPropagatorJob::slotComputeTransmissionChecksum(SyncFileItemPtr item, std::numeric_limits::max(), &propagator()->_bandwidthManager); const auto contentChecksum = ComputeChecksum::computeNow(checksumDevice, checksumType); - item->_size = checksumDevice->size(); - fileToUpload._size = checksumDevice->size(); slotStartUpload(item, fileToUpload, checksumType, contentChecksum); } else { connect(computeChecksum, &ComputeChecksum::done, this, [this, item, fileToUpload] (const QByteArray &contentChecksumType, const QByteArray &contentChecksum) { @@ -344,10 +342,8 @@ void BulkPropagatorJob::slotStartUpload(SyncFileItemPtr item, return; } - if (!item->isSymLink()) { - fileToUpload._size = FileSystem::getSize(fullFilePath); - item->_size = FileSystem::getSize(originalFilePath); - } + fileToUpload._size = FileSystem::getSize(fullFilePath); + item->_size = FileSystem::getSize(originalFilePath); // But skip the file if the mtime is too close to 'now'! // That usually indicates a file that is still being changed From 8f05e930acb28af30b2710c08c82dc94a2f0bbc5 Mon Sep 17 00:00:00 2001 From: Tamino Bauknecht Date: Tue, 28 Nov 2023 21:10:19 +0100 Subject: [PATCH 18/34] FileSystem: Fix fileEquals for symlinks Signed-off-by: Tamino Bauknecht --- src/libsync/filesystem.cpp | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/src/libsync/filesystem.cpp b/src/libsync/filesystem.cpp index 12eb1349423af..7c0bc17c053e0 100644 --- a/src/libsync/filesystem.cpp +++ b/src/libsync/filesystem.cpp @@ -31,6 +31,11 @@ namespace OCC { bool FileSystem::fileEquals(const QString &fn1, const QString &fn2) { // compare two files with given filename and return true if they have the same content + auto symlinkTargetFile1 = readlink(fn1); + auto symlinkTargetFile2 = readlink(fn2); + if (!symlinkTargetFile1.isEmpty() && symlinkTargetFile1 == symlinkTargetFile2) { + return true; + } QFile f1(fn1); QFile f2(fn2); if (!f1.open(QIODevice::ReadOnly) || !f2.open(QIODevice::ReadOnly)) { From f5d611007cf406059bf74667d8908a161e28b477 Mon Sep 17 00:00:00 2001 From: Tamino Bauknecht Date: Thu, 30 Nov 2023 16:13:34 +0100 Subject: [PATCH 19/34] gui/config: Add symlink synchronization as sync option This commit reverts the effects of commit "libsync: Remove auto-exclude for symlinks" (5812d5cbd47a9fa5b39931975b83ed17e38393fe). Signed-off-by: Tamino Bauknecht --- src/gui/folder.cpp | 1 + src/gui/generalsettings.cpp | 4 ++++ src/gui/generalsettings.ui | 11 +++++++++++ src/libsync/configfile.cpp | 12 ++++++++++++ src/libsync/configfile.h | 3 +++ src/libsync/discovery.cpp | 20 +++++++++++++------- src/libsync/discovery.h | 7 ++++--- src/libsync/syncengine.cpp | 2 ++ src/libsync/syncoptions.h | 3 +++ 9 files changed, 53 insertions(+), 10 deletions(-) diff --git a/src/gui/folder.cpp b/src/gui/folder.cpp index 21f3423460326..7bbf2e67e0838 100644 --- a/src/gui/folder.cpp +++ b/src/gui/folder.cpp @@ -1096,6 +1096,7 @@ SyncOptions Folder::initializeSyncOptions() const auto newFolderLimit = cfgFile.newBigFolderSizeLimit(); opt._newBigFolderSizeLimit = newFolderLimit.first ? newFolderLimit.second * 1000LL * 1000LL : -1; // convert from MB to B opt._confirmExternalStorage = cfgFile.confirmExternalStorage(); + opt._synchronizeSymlinks = cfgFile.synchronizeSymlinks(); opt._moveFilesToTrash = cfgFile.moveToTrash(); opt._vfs = _vfs; opt._parallelNetworkJobs = _accountState->account()->isHttp2Supported() ? 20 : 6; diff --git a/src/gui/generalsettings.cpp b/src/gui/generalsettings.cpp index 114aab7cd9dd8..00ed6f6e50393 100644 --- a/src/gui/generalsettings.cpp +++ b/src/gui/generalsettings.cpp @@ -191,6 +191,7 @@ GeneralSettings::GeneralSettings(QWidget *parent) connect(_ui->existingFolderLimitCheckBox, &QAbstractButton::toggled, this, &GeneralSettings::saveMiscSettings); connect(_ui->stopExistingFolderNowBigSyncCheckBox, &QAbstractButton::toggled, this, &GeneralSettings::saveMiscSettings); connect(_ui->newExternalStorage, &QAbstractButton::toggled, this, &GeneralSettings::saveMiscSettings); + connect(_ui->synchronizeSymlinks, &QAbstractButton::toggled, this, &GeneralSettings::saveMiscSettings); connect(_ui->moveFilesToTrashCheckBox, &QAbstractButton::toggled, this, &GeneralSettings::saveMiscSettings); #ifndef WITH_CRASHREPORTER @@ -259,6 +260,7 @@ void GeneralSettings::loadMiscSettings() _ui->showInExplorerNavigationPaneCheckBox->setChecked(cfgFile.showInExplorerNavigationPane()); _ui->crashreporterCheckBox->setChecked(cfgFile.crashReporter()); _ui->newExternalStorage->setChecked(cfgFile.confirmExternalStorage()); + _ui->synchronizeSymlinks->setChecked(cfgFile.synchronizeSymlinks()); _ui->monoIconsCheckBox->setChecked(cfgFile.monoIcons()); _ui->moveFilesToTrashCheckBox->setChecked(cfgFile.moveToTrash()); @@ -270,6 +272,7 @@ void GeneralSettings::loadMiscSettings() _ui->stopExistingFolderNowBigSyncCheckBox->setEnabled(_ui->existingFolderLimitCheckBox->isChecked()); _ui->stopExistingFolderNowBigSyncCheckBox->setChecked(_ui->existingFolderLimitCheckBox->isChecked() && cfgFile.stopSyncingExistingFoldersOverLimit()); _ui->newExternalStorage->setChecked(cfgFile.confirmExternalStorage()); + _ui->synchronizeSymlinks->setChecked(cfgFile.synchronizeSymlinks()); _ui->monoIconsCheckBox->setChecked(cfgFile.monoIcons()); } @@ -447,6 +450,7 @@ void GeneralSettings::saveMiscSettings() cfgFile.setMoveToTrash(_ui->moveFilesToTrashCheckBox->isChecked()); cfgFile.setNewBigFolderSizeLimit(newFolderLimitEnabled, _ui->newFolderLimitSpinBox->value()); cfgFile.setConfirmExternalStorage(_ui->newExternalStorage->isChecked()); + cfgFile.setSynchronizeSymlinks(_ui->synchronizeSymlinks->isChecked()); cfgFile.setNotifyExistingFoldersOverLimit(existingFolderLimitEnabled); cfgFile.setStopSyncingExistingFoldersOverLimit(stopSyncingExistingFoldersOverLimit); diff --git a/src/gui/generalsettings.ui b/src/gui/generalsettings.ui index 0bbb5483dc6dc..4f4d60abfe16c 100644 --- a/src/gui/generalsettings.ui +++ b/src/gui/generalsettings.ui @@ -139,6 +139,17 @@ + + + + + + [experimental] Synchronize symlinks + + + + + diff --git a/src/libsync/configfile.cpp b/src/libsync/configfile.cpp index 60cb2cc71aeba..d275abaaffa23 100644 --- a/src/libsync/configfile.cpp +++ b/src/libsync/configfile.cpp @@ -102,6 +102,7 @@ static constexpr char useNewBigFolderSizeLimitC[] = "useNewBigFolderSizeLimit"; static constexpr char notifyExistingFoldersOverLimitC[] = "notifyExistingFoldersOverLimit"; static constexpr char stopSyncingExistingFoldersOverLimitC[] = "stopSyncingExistingFoldersOverLimit"; static constexpr char confirmExternalStorageC[] = "confirmExternalStorage"; +static constexpr char synchronizeSymlinksC[] = "synchronizeSymlinks"; static constexpr char moveToTrashC[] = "moveToTrash"; static constexpr char certPath[] = "http_certificatePath"; @@ -947,6 +948,12 @@ bool ConfigFile::confirmExternalStorage() const return getPolicySetting(QLatin1String(confirmExternalStorageC), fallback).toBool(); } +bool ConfigFile::synchronizeSymlinks() const +{ + const auto fallback = getValue(synchronizeSymlinksC, QString(), false); + return getPolicySetting(QLatin1String(synchronizeSymlinksC), fallback).toBool(); +} + bool ConfigFile::useNewBigFolderSizeLimit() const { const auto fallback = getValue(useNewBigFolderSizeLimitC, QString(), true); @@ -981,6 +988,11 @@ void ConfigFile::setConfirmExternalStorage(bool isChecked) setValue(confirmExternalStorageC, isChecked); } +void ConfigFile::setSynchronizeSymlinks(bool isChecked) +{ + setValue(synchronizeSymlinksC, isChecked); +} + bool ConfigFile::moveToTrash() const { return getValue(moveToTrashC, QString(), false).toBool(); diff --git a/src/libsync/configfile.h b/src/libsync/configfile.h index 21e58412afcc5..fa193d406e143 100644 --- a/src/libsync/configfile.h +++ b/src/libsync/configfile.h @@ -149,6 +149,9 @@ class OWNCLOUDSYNC_EXPORT ConfigFile [[nodiscard]] bool confirmExternalStorage() const; void setConfirmExternalStorage(bool); + [[nodiscard]] bool synchronizeSymlinks() const; + void setSynchronizeSymlinks(bool); + /** If we should move the files deleted on the server in the trash */ [[nodiscard]] bool moveToTrash() const; void setMoveToTrash(bool); diff --git a/src/libsync/discovery.cpp b/src/libsync/discovery.cpp index fe12fe80091ee..454af3f878edd 100644 --- a/src/libsync/discovery.cpp +++ b/src/libsync/discovery.cpp @@ -39,16 +39,17 @@ namespace OCC { Q_LOGGING_CATEGORY(lcDisco, "nextcloud.sync.discovery", QtInfoMsg) -ProcessDirectoryJob::ProcessDirectoryJob(DiscoveryPhase *data, PinState basePinState, qint64 lastSyncTimestamp, QObject *parent) +ProcessDirectoryJob::ProcessDirectoryJob(DiscoveryPhase *data, PinState basePinState, qint64 lastSyncTimestamp, bool synchronizeSymlinks, QObject *parent) : QObject(parent) , _lastSyncTimestamp(lastSyncTimestamp) , _discoveryData(data) + , _synchronizeSymlinks(synchronizeSymlinks) { qCDebug(lcDisco) << data; computePinState(basePinState); } -ProcessDirectoryJob::ProcessDirectoryJob(const PathTuple &path, const SyncFileItemPtr &dirItem, QueryMode queryLocal, QueryMode queryServer, qint64 lastSyncTimestamp, ProcessDirectoryJob *parent) +ProcessDirectoryJob::ProcessDirectoryJob(const PathTuple &path, const SyncFileItemPtr &dirItem, QueryMode queryLocal, QueryMode queryServer, qint64 lastSyncTimestamp, bool synchronizeSymlinks, ProcessDirectoryJob *parent) : QObject(parent) , _dirItem(dirItem) , _lastSyncTimestamp(lastSyncTimestamp) @@ -56,18 +57,20 @@ ProcessDirectoryJob::ProcessDirectoryJob(const PathTuple &path, const SyncFileIt , _queryLocal(queryLocal) , _discoveryData(parent->_discoveryData) , _currentFolder(path) + , _synchronizeSymlinks(synchronizeSymlinks) { qCDebug(lcDisco) << path._server << queryServer << path._local << queryLocal << lastSyncTimestamp; computePinState(parent->_pinState); } -ProcessDirectoryJob::ProcessDirectoryJob(DiscoveryPhase *data, PinState basePinState, const PathTuple &path, const SyncFileItemPtr &dirItem, QueryMode queryLocal, qint64 lastSyncTimestamp, QObject *parent) +ProcessDirectoryJob::ProcessDirectoryJob(DiscoveryPhase *data, PinState basePinState, const PathTuple &path, const SyncFileItemPtr &dirItem, QueryMode queryLocal, qint64 lastSyncTimestamp, bool synchronizeSymlinks, QObject *parent) : QObject(parent) , _dirItem(dirItem) , _lastSyncTimestamp(lastSyncTimestamp) , _queryLocal(queryLocal) , _discoveryData(data) , _currentFolder(path) + , _synchronizeSymlinks(synchronizeSymlinks) { computePinState(basePinState); } @@ -306,7 +309,9 @@ bool ProcessDirectoryJob::handleExcluded(const QString &path, const Entries &ent } } - if (excluded == CSYNC_NOT_EXCLUDED) { + bool isSymlink = entries.localEntry.isSymLink || entries.serverEntry.isSymLink; + // All not excluded files except for symlinks if symlink synchronization is disabled + if (excluded == CSYNC_NOT_EXCLUDED && (!isSymlink || (_synchronizeSymlinks && isSymlink))) { return false; } else if (excluded == CSYNC_FILE_SILENTLY_EXCLUDED || excluded == CSYNC_FILE_EXCLUDE_AND_REMOVE) { emit _discoveryData->silentlyExcluded(path); @@ -326,7 +331,8 @@ bool ProcessDirectoryJob::handleExcluded(const QString &path, const Entries &ent return true; } - if (entries.localEntry.isSymLink) { + if (isSymlink && !_synchronizeSymlinks) { + item->_errorString = tr("Symbolic links synchronization is not enabled."); } else { switch (excluded) { case CSYNC_NOT_EXCLUDED: @@ -1653,7 +1659,7 @@ void ProcessDirectoryJob::processFileFinalize( } if (recurse) { auto job = new ProcessDirectoryJob(path, item, recurseQueryLocal, recurseQueryServer, - _lastSyncTimestamp, this); + _lastSyncTimestamp, _synchronizeSymlinks, this); job->setInsideEncryptedTree(isInsideEncryptedTree() || item->isEncrypted()); if (removed) { job->setParent(_discoveryData); @@ -1697,7 +1703,7 @@ void ProcessDirectoryJob::processBlacklisted(const PathTuple &path, const OCC::L qCInfo(lcDisco) << "Discovered (blacklisted) " << item->_file << item->_instruction << item->_direction << item->isDirectory(); if (item->isDirectory() && item->_instruction != CSYNC_INSTRUCTION_IGNORE) { - auto job = new ProcessDirectoryJob(path, item, NormalQuery, InBlackList, _lastSyncTimestamp, this); + auto job = new ProcessDirectoryJob(path, item, NormalQuery, InBlackList, _lastSyncTimestamp, _synchronizeSymlinks, this); connect(job, &ProcessDirectoryJob::finished, this, &ProcessDirectoryJob::subJobFinished); _queuedJobs.push_back(job); } else { diff --git a/src/libsync/discovery.h b/src/libsync/discovery.h index eaa2657697a91..78116bda6fb13 100644 --- a/src/libsync/discovery.h +++ b/src/libsync/discovery.h @@ -101,15 +101,15 @@ class ProcessDirectoryJob : public QObject * The base pin state is used if the root dir's pin state can't be retrieved. */ explicit ProcessDirectoryJob(DiscoveryPhase *data, PinState basePinState, - qint64 lastSyncTimestamp, QObject *parent); + qint64 lastSyncTimestamp, bool synchronizeSymlinks, QObject *parent); /// For creating subjobs explicit ProcessDirectoryJob(const PathTuple &path, const SyncFileItemPtr &dirItem, - QueryMode queryLocal, QueryMode queryServer, qint64 lastSyncTimestamp, + QueryMode queryLocal, QueryMode queryServer, qint64 lastSyncTimestamp, bool synchronizeSymlinks, ProcessDirectoryJob *parent); explicit ProcessDirectoryJob(DiscoveryPhase *data, PinState basePinState, const PathTuple &path, const SyncFileItemPtr &dirItem, - QueryMode queryLocal, qint64 lastSyncTimestamp, QObject *parent); + QueryMode queryLocal, qint64 lastSyncTimestamp, bool synchronizeSymlinks, QObject *parent); void start(); /** Start up to nbJobs, return the number of job started; emit finished() when done */ @@ -296,6 +296,7 @@ class ProcessDirectoryJob : public QObject bool _childIgnored = false; // The directory contains ignored item that would prevent deletion PinState _pinState = PinState::Unspecified; // The directory's pin-state, see computePinState() bool _isInsideEncryptedTree = false; // this directory is encrypted or is within the tree of directories with root directory encrypted + const bool _synchronizeSymlinks = false; signals: void finished(); diff --git a/src/libsync/syncengine.cpp b/src/libsync/syncengine.cpp index e4e76a7ff79b7..8cd1475a23240 100644 --- a/src/libsync/syncengine.cpp +++ b/src/libsync/syncengine.cpp @@ -717,6 +717,7 @@ void SyncEngine::startSync() singleItemDiscoveryOptions().discoveryDirItem, localQueryMode, _journal->keyValueStoreGetInt("last_sync", 0), + _syncOptions._synchronizeSymlinks, _discoveryPhase.data() ); } else { @@ -724,6 +725,7 @@ void SyncEngine::startSync() _discoveryPhase.data(), PinState::AlwaysLocal, _journal->keyValueStoreGetInt("last_sync", 0), + _syncOptions._synchronizeSymlinks, _discoveryPhase.data() ); } diff --git a/src/libsync/syncoptions.h b/src/libsync/syncoptions.h index 092ac6fb6f1c6..d3e91b79c4340 100644 --- a/src/libsync/syncoptions.h +++ b/src/libsync/syncoptions.h @@ -42,6 +42,9 @@ class OWNCLOUDSYNC_EXPORT SyncOptions /** If a confirmation should be asked for external storages */ bool _confirmExternalStorage = false; + /** If symlinks should be synchronized to the server as symlinks */ + bool _synchronizeSymlinks = false; + /** If remotely deleted files are needed to move to trash */ bool _moveFilesToTrash = false; From 48669223aaccc41e0a9854a35be7b0569abe594b Mon Sep 17 00:00:00 2001 From: Tamino Bauknecht Date: Sat, 2 Dec 2023 16:15:11 +0100 Subject: [PATCH 20/34] Revert "common: Allow checksum calculation on QIODevice again" This reverts commit 73063eb7fad4f469d280e02b4dfb9bf77dd28007. Since the checksum calculation on symlinks is required in multiple places, it is better to move this differentiation to the checksum calculation class. Signed-off-by: Tamino Bauknecht --- src/common/checksumcalculator.cpp | 8 +------- src/common/checksumcalculator.h | 6 +++--- src/common/checksums.cpp | 11 ----------- src/common/checksums.h | 1 - src/libsync/bulkpropagatorjob.cpp | 20 +++++--------------- 5 files changed, 9 insertions(+), 37 deletions(-) diff --git a/src/common/checksumcalculator.cpp b/src/common/checksumcalculator.cpp index 114729ee92a38..ec9faa2de8c5a 100644 --- a/src/common/checksumcalculator.cpp +++ b/src/common/checksumcalculator.cpp @@ -12,7 +12,6 @@ * for more details. */ #include "checksumcalculator.h" -#include "checksumconsts.h" #include @@ -49,12 +48,7 @@ static QCryptographicHash::Algorithm algorithmTypeToQCryptoHashAlgorithm(Checksu } ChecksumCalculator::ChecksumCalculator(const QString &filePath, const QByteArray &checksumTypeName) - : ChecksumCalculator(QSharedPointer(new QFile(filePath)), checksumTypeName) -{ -} - -ChecksumCalculator::ChecksumCalculator(QSharedPointer fileDevice, const QByteArray &checksumTypeName) - : _device(fileDevice) + : _device(new QFile(filePath)) { if (checksumTypeName == checkSumMD5C) { _algorithmType = AlgorithmType::MD5; diff --git a/src/common/checksumcalculator.h b/src/common/checksumcalculator.h index cd1b580b32915..da1263abeee57 100644 --- a/src/common/checksumcalculator.h +++ b/src/common/checksumcalculator.h @@ -16,12 +16,13 @@ #include "ocsynclib.h" #include "config.h" +#include "checksumconsts.h" #include #include #include #include -#include +#include class QCryptographicHash; @@ -41,14 +42,13 @@ class OCSYNC_EXPORT ChecksumCalculator }; ChecksumCalculator(const QString &filePath, const QByteArray &checksumTypeName); - ChecksumCalculator(QSharedPointer fileDevice, const QByteArray &checksumTypeName); ~ChecksumCalculator(); [[nodiscard]] QByteArray calculate(); private: void initChecksumAlgorithm(); bool addChunk(const QByteArray &chunk, const qint64 size); - QSharedPointer _device; + QScopedPointer _device; QScopedPointer _cryptographicHash; unsigned int _adlerHash = 0; bool _isInitialized = false; diff --git a/src/common/checksums.cpp b/src/common/checksums.cpp index 106b1d99530d1..370730c3c3461 100644 --- a/src/common/checksums.cpp +++ b/src/common/checksums.cpp @@ -218,17 +218,6 @@ QByteArray ComputeChecksum::computeNow(const QString &filePath, const QByteArray return checksumCalculator.calculate(); } -QByteArray ComputeChecksum::computeNow(QSharedPointer fileDevice, const QByteArray &checksumType) -{ - if (!checksumComputationEnabled()) { - qCWarning(lcChecksums) << "Checksum computation disabled by environment variable"; - return QByteArray(); - } - - ChecksumCalculator checksumCalculator(fileDevice, checksumType); - return checksumCalculator.calculate(); -} - void ComputeChecksum::slotCalculationDone() { QByteArray checksum = _watcher.future().result(); diff --git a/src/common/checksums.h b/src/common/checksums.h index 42de22bedccfb..bd13dc07fb82e 100644 --- a/src/common/checksums.h +++ b/src/common/checksums.h @@ -85,7 +85,6 @@ class OCSYNC_EXPORT ComputeChecksum : public QObject * Computes the checksum synchronously. */ static QByteArray computeNow(const QString &filePath, const QByteArray &checksumType); - static QByteArray computeNow(QSharedPointer fileDevice, const QByteArray &checksumType); /** * Computes the checksum synchronously on file. Convenience wrapper for computeNow(). diff --git a/src/libsync/bulkpropagatorjob.cpp b/src/libsync/bulkpropagatorjob.cpp index 0943c199fe67e..02581954ca40a 100644 --- a/src/libsync/bulkpropagatorjob.cpp +++ b/src/libsync/bulkpropagatorjob.cpp @@ -281,22 +281,12 @@ void BulkPropagatorJob::slotComputeTransmissionChecksum(SyncFileItemPtr item, const auto checksumType = uploadChecksumEnabled() ? "MD5" : ""; computeChecksum->setChecksumType(checksumType); - if (item->_type == CSyncEnums::ItemTypeSoftLink) { - computeChecksum->deleteLater(); - auto checksumDevice = QSharedPointer::create(fileToUpload._path, - 0, - std::numeric_limits::max(), - &propagator()->_bandwidthManager); - const auto contentChecksum = ComputeChecksum::computeNow(checksumDevice, checksumType); - slotStartUpload(item, fileToUpload, checksumType, contentChecksum); - } else { - connect(computeChecksum, &ComputeChecksum::done, this, [this, item, fileToUpload] (const QByteArray &contentChecksumType, const QByteArray &contentChecksum) { - slotStartUpload(item, fileToUpload, contentChecksumType, contentChecksum); - }); - connect(computeChecksum, &ComputeChecksum::done, computeChecksum, &QObject::deleteLater); + connect(computeChecksum, &ComputeChecksum::done, this, [this, item, fileToUpload] (const QByteArray &contentChecksumType, const QByteArray &contentChecksum) { + slotStartUpload(item, fileToUpload, contentChecksumType, contentChecksum); + }); + connect(computeChecksum, &ComputeChecksum::done, computeChecksum, &QObject::deleteLater); - computeChecksum->start(fileToUpload._path); - } + computeChecksum->start(fileToUpload._path); } void BulkPropagatorJob::slotStartUpload(SyncFileItemPtr item, From 6df006de8d5c9f5d67b7351ac4057019c63df013 Mon Sep 17 00:00:00 2001 From: Tamino Bauknecht Date: Mon, 4 Dec 2023 22:06:02 +0100 Subject: [PATCH 21/34] filesystem: Move readlink from libsync/filesystem.h to common/filesystembase.h Signed-off-by: Tamino Bauknecht --- src/common/filesystembase.cpp | 11 +++++++++++ src/common/filesystembase.h | 10 ++++++++++ src/libsync/filesystem.cpp | 10 ---------- src/libsync/filesystem.h | 10 ---------- 4 files changed, 21 insertions(+), 20 deletions(-) diff --git a/src/common/filesystembase.cpp b/src/common/filesystembase.cpp index be8c2dea4f0d9..bb379fdf0edb9 100644 --- a/src/common/filesystembase.cpp +++ b/src/common/filesystembase.cpp @@ -20,6 +20,8 @@ #include "utility.h" #include "common/asserts.h" +#include + #include #include #include @@ -127,6 +129,15 @@ bool FileSystem::setFileReadOnlyWeak(const QString &filename, bool readonly) return true; } +QByteArray FileSystem::readlink(const QString &filename) +{ + if (!QFileInfo(filename).isSymLink()) { + return QByteArray(); + } + auto symlinkContent = std::filesystem::read_symlink(filename.toStdString()).u8string(); + return QByteArray(symlinkContent.data()); +} + bool FileSystem::rename(const QString &originFileName, const QString &destinationFileName, QString *errorString) diff --git a/src/common/filesystembase.h b/src/common/filesystembase.h index fd0572804dcb2..323aa5e48e2d8 100644 --- a/src/common/filesystembase.h +++ b/src/common/filesystembase.h @@ -84,6 +84,16 @@ namespace FileSystem { */ bool OCSYNC_EXPORT fileExists(const QString &filename, const QFileInfo & = QFileInfo()); + /** + * @brief Return raw content of symlink at given path. + * + * If the file is not a symlink or does not exist, the returned string will be empty. + * In Qt6.6+, QFileInfo::readSymLink() can be used instead. + * QFileInfo::symLinkTarget() can *not* be used because it transforms the target to an + * absolute path which might break relative symlinks for cross-device synchronization. + */ + QByteArray OCSYNC_EXPORT readlink(const QString &filename); + /** * @brief Rename the file \a originFileName to \a destinationFileName. * diff --git a/src/libsync/filesystem.cpp b/src/libsync/filesystem.cpp index 7c0bc17c053e0..0576f496a6cda 100644 --- a/src/libsync/filesystem.cpp +++ b/src/libsync/filesystem.cpp @@ -15,7 +15,6 @@ #include "filesystem.h" #include "common/utility.h" -#include #include #include #include @@ -141,15 +140,6 @@ qint64 FileSystem::getSize(const QString &filename) return info.size(); } -QByteArray FileSystem::readlink(const QString &filename) -{ - if (!QFileInfo(filename).isSymLink()) { - return QByteArray(); - } - auto symlinkContent = std::filesystem::read_symlink(filename.toStdString()).u8string(); - return QByteArray(symlinkContent.data()); -} - // Code inspired from Qt5's QDir::removeRecursively bool FileSystem::removeRecursively(const QString &path, const std::function &onDeleted, QStringList *errors) { diff --git a/src/libsync/filesystem.h b/src/libsync/filesystem.h index 5129e8ad6dc64..602f74bb7f2bc 100644 --- a/src/libsync/filesystem.h +++ b/src/libsync/filesystem.h @@ -68,16 +68,6 @@ namespace FileSystem { */ bool OWNCLOUDSYNC_EXPORT getInode(const QString &filename, quint64 *inode); - /** - * @brief Return raw content of symlink at given path. - * - * If the file is not a symlink or does not exist, the returned string will be empty. - * In Qt6.6+, QFileInfo::readSymLink() can be used instead. - * QFileInfo::symLinkTarget() can *not* be used because it transforms the target to an - * absolute path which might break relative symlinks for cross-device synchronization. - */ - QByteArray OWNCLOUDSYNC_EXPORT readlink(const QString &filename); - /** * @brief Check if \a fileName has changed given previous size and mtime * From 97f015cc34e5a6665e651a9ed18d4a0ea757d68e Mon Sep 17 00:00:00 2001 From: Tamino Bauknecht Date: Mon, 4 Dec 2023 22:09:54 +0100 Subject: [PATCH 22/34] ChecksumCalculator: Calculate symlink checksum on symlink target Signed-off-by: Tamino Bauknecht --- src/common/checksumcalculator.cpp | 15 ++++++++++++++- src/common/checksumcalculator.h | 1 + 2 files changed, 15 insertions(+), 1 deletion(-) diff --git a/src/common/checksumcalculator.cpp b/src/common/checksumcalculator.cpp index ec9faa2de8c5a..360ed58c87964 100644 --- a/src/common/checksumcalculator.cpp +++ b/src/common/checksumcalculator.cpp @@ -12,11 +12,14 @@ * for more details. */ #include "checksumcalculator.h" +#include "filesystembase.h" #include +#include #include #include +#include #include namespace @@ -48,7 +51,7 @@ static QCryptographicHash::Algorithm algorithmTypeToQCryptoHashAlgorithm(Checksu } ChecksumCalculator::ChecksumCalculator(const QString &filePath, const QByteArray &checksumTypeName) - : _device(new QFile(filePath)) + : _device(openFile(filePath)) { if (checksumTypeName == checkSumMD5C) { _algorithmType = AlgorithmType::MD5; @@ -140,6 +143,16 @@ QByteArray ChecksumCalculator::calculate() return result; } +QScopedPointer ChecksumCalculator::openFile(const QString &filePath) +{ + if (QFileInfo(filePath).isSymLink()) { + auto symlinkContent = FileSystem::readlink(filePath); + return QScopedPointer(new QBuffer(&symlinkContent)); + } else { + return QScopedPointer(new QFile(filePath)); + } +} + void ChecksumCalculator::initChecksumAlgorithm() { if (_algorithmType == AlgorithmType::Undefined) { diff --git a/src/common/checksumcalculator.h b/src/common/checksumcalculator.h index da1263abeee57..a7f885f3c37d1 100644 --- a/src/common/checksumcalculator.h +++ b/src/common/checksumcalculator.h @@ -46,6 +46,7 @@ class OCSYNC_EXPORT ChecksumCalculator [[nodiscard]] QByteArray calculate(); private: + static QScopedPointer openFile(const QString &filePath); void initChecksumAlgorithm(); bool addChunk(const QByteArray &chunk, const qint64 size); QScopedPointer _device; From 845e1082bd1e889a5b7e0acf02bbc083f14fb1c0 Mon Sep 17 00:00:00 2001 From: Tamino Bauknecht Date: Mon, 4 Dec 2023 22:19:33 +0100 Subject: [PATCH 23/34] discoveryphase: Fix local entry size for symlinks Signed-off-by: Tamino Bauknecht --- src/libsync/discoveryphase.cpp | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/libsync/discoveryphase.cpp b/src/libsync/discoveryphase.cpp index 740e302e15918..b6012a7abf167 100644 --- a/src/libsync/discoveryphase.cpp +++ b/src/libsync/discoveryphase.cpp @@ -16,6 +16,7 @@ #include "common/utility.h" #include "configfile.h" #include "discovery.h" +#include "filesystem.h" #include "helpers.h" #include "progressdispatcher.h" @@ -335,7 +336,7 @@ void DiscoverySingleLocalDirectoryJob::run() { continue; } i.modtime = dirent->modtime; - i.size = dirent->size; + i.size = FileSystem::getSize(localPath + '/' + dirent->path); i.inode = dirent->inode; i.isDirectory = dirent->type == ItemTypeDirectory; i.isHidden = dirent->is_hidden; From 7a6df0ef4a849147dd0208250c1e6b17d422fd9f Mon Sep 17 00:00:00 2001 From: Tamino Bauknecht Date: Mon, 4 Dec 2023 23:28:12 +0100 Subject: [PATCH 24/34] ChecksumCalculator: Fix creation of QBuffer Signed-off-by: Tamino Bauknecht --- src/common/checksumcalculator.cpp | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/common/checksumcalculator.cpp b/src/common/checksumcalculator.cpp index 360ed58c87964..49edabee0f465 100644 --- a/src/common/checksumcalculator.cpp +++ b/src/common/checksumcalculator.cpp @@ -147,7 +147,9 @@ QScopedPointer ChecksumCalculator::openFile(const QString &filePath) { if (QFileInfo(filePath).isSymLink()) { auto symlinkContent = FileSystem::readlink(filePath); - return QScopedPointer(new QBuffer(&symlinkContent)); + QScopedPointer symlinkDevice(new QBuffer()); + symlinkDevice->setData(symlinkContent); + return QScopedPointer(symlinkDevice.take()); } else { return QScopedPointer(new QFile(filePath)); } From 81dbb9dbf1a8cfea148fc71faf4fb05212084f49 Mon Sep 17 00:00:00 2001 From: Tamino Bauknecht Date: Mon, 4 Dec 2023 23:28:52 +0100 Subject: [PATCH 25/34] FileSystem: Make readlink C++20 compliant Signed-off-by: Tamino Bauknecht --- src/common/filesystembase.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/common/filesystembase.cpp b/src/common/filesystembase.cpp index bb379fdf0edb9..f981081806110 100644 --- a/src/common/filesystembase.cpp +++ b/src/common/filesystembase.cpp @@ -134,7 +134,7 @@ QByteArray FileSystem::readlink(const QString &filename) if (!QFileInfo(filename).isSymLink()) { return QByteArray(); } - auto symlinkContent = std::filesystem::read_symlink(filename.toStdString()).u8string(); + auto symlinkContent = std::filesystem::read_symlink(filename.toStdString()).string(); return QByteArray(symlinkContent.data()); } From 91a5e54720f701d3866be75742da4a12980d4956 Mon Sep 17 00:00:00 2001 From: Tamino Bauknecht Date: Wed, 6 Dec 2023 20:16:10 +0100 Subject: [PATCH 26/34] csync/FileSystem: Adjust c_utimes to not follow symlinks Signed-off-by: Tamino Bauknecht --- src/csync/std/c_time.cpp | 10 +++++----- src/csync/std/c_time.h | 2 +- src/libsync/filesystem.cpp | 4 ++-- 3 files changed, 8 insertions(+), 8 deletions(-) diff --git a/src/csync/std/c_time.cpp b/src/csync/std/c_time.cpp index b51b5a2d1a0cb..75ce65b936089 100644 --- a/src/csync/std/c_time.cpp +++ b/src/csync/std/c_time.cpp @@ -25,8 +25,8 @@ #include #ifdef HAVE_UTIMES -int c_utimes(const QString &uri, const struct timeval *times) { - int ret = utimes(QFile::encodeName(uri).constData(), times); +int c_utimes(const QString &uri, const struct timespec *times) { + int ret = utimensat(AT_FDCWD, QFile::encodeName(uri).constData(), times, AT_SYMLINK_NOFOLLOW); return ret; } #else // HAVE_UTIMES @@ -39,15 +39,15 @@ int c_utimes(const QString &uri, const struct timeval *times) { #define CSYNC_SECONDS_SINCE_1601 11644473600LL #define CSYNC_USEC_IN_SEC 1000000LL //after Microsoft KB167296 -static void UnixTimevalToFileTime(struct timeval t, LPFILETIME pft) +static void UnixTimevalToFileTime(struct timespec t, LPFILETIME pft) { LONGLONG ll = 0; - ll = Int32x32To64(t.tv_sec, CSYNC_USEC_IN_SEC*10) + t.tv_usec*10 + CSYNC_SECONDS_SINCE_1601*CSYNC_USEC_IN_SEC*10; + ll = Int32x32To64(t.tv_sec, CSYNC_USEC_IN_SEC*10) + t.tv_nsec/100 + CSYNC_SECONDS_SINCE_1601*CSYNC_USEC_IN_SEC*10; pft->dwLowDateTime = (DWORD)ll; pft->dwHighDateTime = ll >> 32; } -int c_utimes(const QString &uri, const struct timeval *times) { +int c_utimes(const QString &uri, const struct timespec *times) { FILETIME LastAccessTime; FILETIME LastModificationTime; HANDLE hFile = nullptr; diff --git a/src/csync/std/c_time.h b/src/csync/std/c_time.h index 55a6aa6bc836f..b09da715f7fdf 100644 --- a/src/csync/std/c_time.h +++ b/src/csync/std/c_time.h @@ -31,7 +31,7 @@ #include #endif -OCSYNC_EXPORT int c_utimes(const QString &uri, const struct timeval *times); +OCSYNC_EXPORT int c_utimes(const QString &uri, const struct timespec *times); #endif /* _C_TIME_H */ diff --git a/src/libsync/filesystem.cpp b/src/libsync/filesystem.cpp index 0576f496a6cda..042da06df18dc 100644 --- a/src/libsync/filesystem.cpp +++ b/src/libsync/filesystem.cpp @@ -76,9 +76,9 @@ time_t FileSystem::getModTime(const QString &filename) bool FileSystem::setModTime(const QString &filename, time_t modTime) { - struct timeval times[2]; + struct timespec times[2]; times[0].tv_sec = times[1].tv_sec = modTime; - times[0].tv_usec = times[1].tv_usec = 0; + times[0].tv_nsec = times[1].tv_nsec = 0; int rc = c_utimes(filename, times); if (rc != 0) { qCWarning(lcFileSystem) << "Error setting mtime for" << filename From d5b61b2eba8a289f561f3d40794e1211d722b131 Mon Sep 17 00:00:00 2001 From: Tamino Bauknecht Date: Wed, 6 Dec 2023 20:54:39 +0100 Subject: [PATCH 27/34] discovery: Check if symlink for server entries Signed-off-by: Tamino Bauknecht --- src/libsync/discovery.cpp | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/libsync/discovery.cpp b/src/libsync/discovery.cpp index 454af3f878edd..177f8dd096539 100644 --- a/src/libsync/discovery.cpp +++ b/src/libsync/discovery.cpp @@ -627,7 +627,8 @@ void ProcessDirectoryJob::processFileAnalyzeRemoteInfo(const SyncFileItemPtr &it item->_isShared = serverEntry.remotePerm.hasPermission(RemotePermissions::IsShared) || serverEntry.sharedByMe; item->_sharedByMe = serverEntry.sharedByMe; item->_lastShareStateFetchedTimestamp = QDateTime::currentMSecsSinceEpoch(); - item->_type = serverEntry.isDirectory ? ItemTypeDirectory : ItemTypeFile; + item->_type = serverEntry.isDirectory ? ItemTypeDirectory : + (serverEntry.isSymLink ? ItemTypeSoftLink : ItemTypeFile); item->_etag = serverEntry.etag; item->_directDownloadUrl = serverEntry.directDownloadUrl; item->_directDownloadCookies = serverEntry.directDownloadCookies; From d36b7b9c977f0a8d14b676ca215056bfc907b425 Mon Sep 17 00:00:00 2001 From: Tamino Bauknecht Date: Wed, 6 Dec 2023 21:04:29 +0100 Subject: [PATCH 28/34] OwncloudPropagator: Fix condition for deletion of previous remote item The issue was introduced in "libsync/common: Store actual type for items". Additionally, the variable/parameter "directoriesToRemove" was renamed to "remoteItemsToRemove" since it could now also be a file or symlink. Signed-off-by: Tamino Bauknecht --- src/libsync/owncloudpropagator.cpp | 23 ++++++++++++----------- src/libsync/owncloudpropagator.h | 4 ++-- 2 files changed, 14 insertions(+), 13 deletions(-) diff --git a/src/libsync/owncloudpropagator.cpp b/src/libsync/owncloudpropagator.cpp index e2410f1714eb7..acbbeaabfc156 100644 --- a/src/libsync/owncloudpropagator.cpp +++ b/src/libsync/owncloudpropagator.cpp @@ -531,13 +531,13 @@ void OwncloudPropagator::start(SyncFileItemVector &&items) _rootJob.reset(new PropagateRootDirectory(this)); QStack> directories; directories.push(qMakePair(QString(), _rootJob.data())); - QVector directoriesToRemove; + QVector remoteItemsToRemove; QString removedDirectory; QString maybeConflictDirectory; foreach (const SyncFileItemPtr &item, items) { if (!removedDirectory.isEmpty() && item->_file.startsWith(removedDirectory)) { // this is an item in a directory which is going to be removed. - auto *delDirJob = qobject_cast(directoriesToRemove.first()); + auto *delDirJob = qobject_cast(remoteItemsToRemove.first()); const auto isNewDirectory = item->isDirectory() && (item->_instruction == CSYNC_INSTRUCTION_NEW || item->_instruction == CSYNC_INSTRUCTION_TYPE_CHANGE); @@ -585,19 +585,19 @@ void OwncloudPropagator::start(SyncFileItemVector &&items) if (item->isDirectory()) { startDirectoryPropagation(item, directories, - directoriesToRemove, + remoteItemsToRemove, removedDirectory, items); } else if (!directories.top().second->_item->_isFileDropDetected) { startFilePropagation(item, directories, - directoriesToRemove, + remoteItemsToRemove, removedDirectory, maybeConflictDirectory); } } - foreach (PropagatorJob *it, directoriesToRemove) { + foreach (PropagatorJob *it, remoteItemsToRemove) { _rootJob->appendDirDeletionJob(it); } @@ -609,7 +609,7 @@ void OwncloudPropagator::start(SyncFileItemVector &&items) void OwncloudPropagator::startDirectoryPropagation(const SyncFileItemPtr &item, QStack> &directories, - QVector &directoriesToRemove, + QVector &remoteItemsToRemove, QString &removedDirectory, const SyncFileItemVector &items) { @@ -633,7 +633,7 @@ void OwncloudPropagator::startDirectoryPropagation(const SyncFileItemPtr &item, if (item->_instruction == CSYNC_INSTRUCTION_REMOVE) { // We do the removal of directories at the end, because there might be moves from // these directories that will happen later. - directoriesToRemove.prepend(directoryPropagationJob.get()); + remoteItemsToRemove.prepend(directoryPropagationJob.get()); removedDirectory = item->_file + "/"; // We should not update the etag of parent directories of the removed directory @@ -676,15 +676,16 @@ void OwncloudPropagator::startDirectoryPropagation(const SyncFileItemPtr &item, void OwncloudPropagator::startFilePropagation(const SyncFileItemPtr &item, QStack > &directories, - QVector &directoriesToRemove, + QVector &remoteItemsToRemove, QString &removedDirectory, QString &maybeConflictDirectory) { - if (item->_instruction == CSYNC_INSTRUCTION_TYPE_CHANGE && item->_type == ItemTypeDirectory) { - // will delete directories, so defer execution + if (item->_instruction == CSYNC_INSTRUCTION_TYPE_CHANGE) { + // will delete previous remote item (since directories cannot be overwritten), + // so defer execution auto job = createJob(item); if (job) { - directoriesToRemove.prepend(job); + remoteItemsToRemove.prepend(job); } removedDirectory = item->_file + "/"; } else { diff --git a/src/libsync/owncloudpropagator.h b/src/libsync/owncloudpropagator.h index 440b98551e51f..3921785699498 100644 --- a/src/libsync/owncloudpropagator.h +++ b/src/libsync/owncloudpropagator.h @@ -437,13 +437,13 @@ class OWNCLOUDSYNC_EXPORT OwncloudPropagator : public QObject void startDirectoryPropagation(const SyncFileItemPtr &item, QStack> &directories, - QVector &directoriesToRemove, + QVector &remoteItemsToRemove, QString &removedDirectory, const SyncFileItemVector &items); void startFilePropagation(const SyncFileItemPtr &item, QStack> &directories, - QVector &directoriesToRemove, + QVector &remoteItemsToRemove, QString &removedDirectory, QString &maybeConflictDirectory); From 5bcf57a22df42c8f9003bb82982348c547716bf1 Mon Sep 17 00:00:00 2001 From: Tamino Bauknecht Date: Wed, 6 Dec 2023 21:13:00 +0100 Subject: [PATCH 29/34] OwncloudPropagator: Only download symlink if enabled by option Signed-off-by: Tamino Bauknecht --- src/libsync/owncloudpropagator.cpp | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/src/libsync/owncloudpropagator.cpp b/src/libsync/owncloudpropagator.cpp index acbbeaabfc156..1527234777f5d 100644 --- a/src/libsync/owncloudpropagator.cpp +++ b/src/libsync/owncloudpropagator.cpp @@ -364,9 +364,13 @@ PropagateItemJob *OwncloudPropagator::createJob(const SyncFileItemPtr &item) } //fall through case CSYNC_INSTRUCTION_SYNC: if (item->_direction != SyncFileItem::Up) { - auto job = new PropagateDownloadFile(this, item); - job->setDeleteExistingFolder(deleteExisting); - return job; + if (item->_type != ItemTypeSoftLink || _syncOptions._synchronizeSymlinks) { + auto job = new PropagateDownloadFile(this, item); + job->setDeleteExistingFolder(deleteExisting); + return job; + } else { + return nullptr; + } } else { if (deleteExisting || !isDelayedUploadItem(item)) { auto job = createUploadJob(item, deleteExisting); From fa16c3376163b8e5ab92d41f92915cd11099f9af Mon Sep 17 00:00:00 2001 From: Tamino Bauknecht Date: Wed, 6 Dec 2023 22:30:17 +0100 Subject: [PATCH 30/34] PropagateDownloadFile: Allow symlink to be larger than 8kB Signed-off-by: Tamino Bauknecht --- src/libsync/propagatedownload.cpp | 33 ++++++++++++++++++++++++++----- src/libsync/propagatedownload.h | 1 + 2 files changed, 29 insertions(+), 5 deletions(-) diff --git a/src/libsync/propagatedownload.cpp b/src/libsync/propagatedownload.cpp index 2bb1423829838..c4bbd7e9f1ed0 100644 --- a/src/libsync/propagatedownload.cpp +++ b/src/libsync/propagatedownload.cpp @@ -33,6 +33,7 @@ #include #include #include +#include #ifdef Q_OS_UNIX #include @@ -290,12 +291,29 @@ qint64 GETFileJob::writeToDevice(const QByteArray &data) return _device->write(data); } +bool GETFileJob::writeSymlink(const QString &symlinkTarget) +{ + auto file_device = qobject_cast(_device); + if (!file_device) { + qCWarning(lcGetJob) << "Unable to create symlink on given device!"; + return false; + } + file_device->close(); + FileSystem::remove(file_device->fileName()); + if (!QFile::link(symlinkTarget, file_device->fileName())) { + qCWarning(lcGetJob) << "Error creating symlink '" << file_device->fileName() << '\''; + return false; + } + return true; +} + void GETFileJob::slotReadyRead() { if (!reply()) return; int bufferSize = qMin(1024 * 8ll, reply()->bytesAvailable()); QByteArray buffer(bufferSize, Qt::Uninitialized); + std::optional symlinkTarget; while (reply()->bytesAvailable() > 0 && _saveBodyToFile) { if (_bandwidthChoked) { @@ -323,11 +341,11 @@ void GETFileJob::slotReadyRead() if (reply()->hasRawHeader("OC-File-Type") && reply()->rawHeader("OC-File-Type").toInt() == ItemTypeSoftLink) { - // TODO: handle non-Unix OS - auto file_device = static_cast(_device); - file_device->close(); - FileSystem::remove(file_device->fileName()); - QFile::link(QString(buffer), file_device->fileName()); + if (symlinkTarget) { + *symlinkTarget += buffer; + } else { + symlinkTarget = buffer; + } } else { const qint64 writtenBytes = writeToDevice(QByteArray::fromRawData(buffer.constData(), readBytes)); if (writtenBytes != readBytes) { @@ -340,6 +358,11 @@ void GETFileJob::slotReadyRead() } } + if (symlinkTarget && !writeSymlink(*symlinkTarget)) { + reply()->abort(); + return; + } + if (reply()->isFinished() && (reply()->bytesAvailable() == 0 || !_saveBodyToFile)) { qCDebug(lcGetJob) << "Actually finished!"; if (_bandwidthManager) { diff --git a/src/libsync/propagatedownload.h b/src/libsync/propagatedownload.h index 2a5fea962ce50..976a501261642 100644 --- a/src/libsync/propagatedownload.h +++ b/src/libsync/propagatedownload.h @@ -115,6 +115,7 @@ class OWNCLOUDSYNC_EXPORT GETFileJob : public AbstractNetworkJob protected: virtual qint64 writeToDevice(const QByteArray &data); + virtual bool writeSymlink(const QString &symlinkTarget); signals: void finishedSignal(); From f779b3e11d406cb263fa90cae8c5357908a06baf Mon Sep 17 00:00:00 2001 From: Tamino Bauknecht Date: Fri, 8 Dec 2023 01:49:56 +0100 Subject: [PATCH 31/34] PropagateDownloadFile/FileSystem: Preserve permissions for symlinks This commit will preserve the permissions set on the symlink if a new file is downloaded in the same place and the symlink is overwritten. That fixes a bug which caused the downloaded file to become unreadable if the symlink was broken and no permissions could be retrieved. Signed-off-by: Tamino Bauknecht --- src/libsync/filesystem.cpp | 27 +++++++++++++++++++++++++++ src/libsync/filesystem.h | 7 +++++++ src/libsync/propagatedownload.cpp | 11 ++++++----- 3 files changed, 40 insertions(+), 5 deletions(-) diff --git a/src/libsync/filesystem.cpp b/src/libsync/filesystem.cpp index 042da06df18dc..357cee5b9d13d 100644 --- a/src/libsync/filesystem.cpp +++ b/src/libsync/filesystem.cpp @@ -14,6 +14,8 @@ #include "filesystem.h" +#include + #include "common/utility.h" #include #include @@ -140,6 +142,31 @@ qint64 FileSystem::getSize(const QString &filename) return info.size(); } +QFile::Permissions FileSystem::getPermissions(const QString &filename) +{ + using qtStdFilePermissionsMapping = std::pair; + constexpr std::array qtStdFilePermissionsMappings = { + std::make_pair(QFileDevice::ReadOwner, std::filesystem::perms::owner_read), + {QFileDevice::WriteOwner, std::filesystem::perms::owner_write}, + {QFileDevice::ExeOwner, std::filesystem::perms::owner_exec}, + {QFileDevice::ReadGroup, std::filesystem::perms::group_read}, + {QFileDevice::WriteGroup, std::filesystem::perms::group_write}, + {QFileDevice::ExeGroup, std::filesystem::perms::group_exec}, + {QFileDevice::ReadOther, std::filesystem::perms::others_read}, + {QFileDevice::WriteOther, std::filesystem::perms::others_write}, + {QFileDevice::ExeOther, std::filesystem::perms::others_exec}}; + + auto fileStatus = std::filesystem::symlink_status(filename.toStdString()); + auto permissions = fileStatus.permissions(); + + QFile::Permissions resultPermissions; + for (auto [qtPermissionFlag, stdPermissionFlag] : qtStdFilePermissionsMappings) { + auto isPermissionSet = (permissions & stdPermissionFlag) != std::filesystem::perms::none; + resultPermissions.setFlag(qtPermissionFlag, isPermissionSet); + } + return resultPermissions; +} + // Code inspired from Qt5's QDir::removeRecursively bool FileSystem::removeRecursively(const QString &path, const std::function &onDeleted, QStringList *errors) { diff --git a/src/libsync/filesystem.h b/src/libsync/filesystem.h index 602f74bb7f2bc..0cdaaac2dec96 100644 --- a/src/libsync/filesystem.h +++ b/src/libsync/filesystem.h @@ -63,6 +63,13 @@ namespace FileSystem { */ qint64 OWNCLOUDSYNC_EXPORT getSize(const QString &filename); + /** + * @brief Get permissions for a file + * + * If the file is a symlink, the permissions for the symlink are returned. + */ + QFile::Permissions OWNCLOUDSYNC_EXPORT getPermissions(const QString &filename); + /** * @brief Retrieve a file inode with csync */ diff --git a/src/libsync/propagatedownload.cpp b/src/libsync/propagatedownload.cpp index c4bbd7e9f1ed0..6ad074eb71113 100644 --- a/src/libsync/propagatedownload.cpp +++ b/src/libsync/propagatedownload.cpp @@ -1108,7 +1108,8 @@ namespace { // Anonymous namespace for the recall feature static void preserveGroupOwnership(const QString &fileName, const QFileInfo &fi) { #ifdef Q_OS_UNIX - int chownErr = chown(fileName.toLocal8Bit().constData(), -1, fi.groupId()); + int chownErr = fchownat(AT_FDCWD, fileName.toLocal8Bit().constData(), -1, + fi.groupId(), AT_SYMLINK_NOFOLLOW); if (chownErr) { // TODO: Consider further error handling! qCWarning(lcPropagateDownload) << QString("preserveGroupOwnership: chown error %1: setting group %2 failed on file %3").arg(chownErr).arg(fi.groupId()).arg(fileName); @@ -1220,11 +1221,11 @@ void PropagateDownloadFile::downloadFinished() auto previousFileExists = FileSystem::fileExists(filename) && _item->_instruction != CSYNC_INSTRUCTION_CASE_CLASH_CONFLICT; if (previousFileExists) { // Preserve the existing file permissions. - const auto existingFile = QFileInfo{filename}; - if (existingFile.permissions() != _tmpFile.permissions()) { - _tmpFile.setPermissions(existingFile.permissions()); + auto previousPermissions = FileSystem::getPermissions(filename); + if (previousPermissions != FileSystem::getPermissions(_tmpFile.fileName())) { + _tmpFile.setPermissions(previousPermissions); } - preserveGroupOwnership(_tmpFile.fileName(), existingFile); + preserveGroupOwnership(_tmpFile.fileName(), QFileInfo(filename)); // Make the file a hydrated placeholder if possible const auto result = propagator()->syncOptions()._vfs->convertToPlaceholder(_tmpFile.fileName(), *_item, filename); From 5265ed426a99ae135cdd1249d355a0c3bed5b600 Mon Sep 17 00:00:00 2001 From: Tamino Bauknecht Date: Fri, 8 Dec 2023 01:53:11 +0100 Subject: [PATCH 32/34] PropagateDownloadFile/FileSystem: Improve previous commit (by mainly undoing it) Instead of copying the permissions on the symlink, use the default permissions in that case. Otherwise, the newly downloaded files might become executable since symlinks often have with all permissions set. Signed-off-by: Tamino Bauknecht --- src/libsync/filesystem.cpp | 27 --------------------------- src/libsync/filesystem.h | 7 ------- src/libsync/propagatedownload.cpp | 10 +++++----- 3 files changed, 5 insertions(+), 39 deletions(-) diff --git a/src/libsync/filesystem.cpp b/src/libsync/filesystem.cpp index 357cee5b9d13d..042da06df18dc 100644 --- a/src/libsync/filesystem.cpp +++ b/src/libsync/filesystem.cpp @@ -14,8 +14,6 @@ #include "filesystem.h" -#include - #include "common/utility.h" #include #include @@ -142,31 +140,6 @@ qint64 FileSystem::getSize(const QString &filename) return info.size(); } -QFile::Permissions FileSystem::getPermissions(const QString &filename) -{ - using qtStdFilePermissionsMapping = std::pair; - constexpr std::array qtStdFilePermissionsMappings = { - std::make_pair(QFileDevice::ReadOwner, std::filesystem::perms::owner_read), - {QFileDevice::WriteOwner, std::filesystem::perms::owner_write}, - {QFileDevice::ExeOwner, std::filesystem::perms::owner_exec}, - {QFileDevice::ReadGroup, std::filesystem::perms::group_read}, - {QFileDevice::WriteGroup, std::filesystem::perms::group_write}, - {QFileDevice::ExeGroup, std::filesystem::perms::group_exec}, - {QFileDevice::ReadOther, std::filesystem::perms::others_read}, - {QFileDevice::WriteOther, std::filesystem::perms::others_write}, - {QFileDevice::ExeOther, std::filesystem::perms::others_exec}}; - - auto fileStatus = std::filesystem::symlink_status(filename.toStdString()); - auto permissions = fileStatus.permissions(); - - QFile::Permissions resultPermissions; - for (auto [qtPermissionFlag, stdPermissionFlag] : qtStdFilePermissionsMappings) { - auto isPermissionSet = (permissions & stdPermissionFlag) != std::filesystem::perms::none; - resultPermissions.setFlag(qtPermissionFlag, isPermissionSet); - } - return resultPermissions; -} - // Code inspired from Qt5's QDir::removeRecursively bool FileSystem::removeRecursively(const QString &path, const std::function &onDeleted, QStringList *errors) { diff --git a/src/libsync/filesystem.h b/src/libsync/filesystem.h index 0cdaaac2dec96..602f74bb7f2bc 100644 --- a/src/libsync/filesystem.h +++ b/src/libsync/filesystem.h @@ -63,13 +63,6 @@ namespace FileSystem { */ qint64 OWNCLOUDSYNC_EXPORT getSize(const QString &filename); - /** - * @brief Get permissions for a file - * - * If the file is a symlink, the permissions for the symlink are returned. - */ - QFile::Permissions OWNCLOUDSYNC_EXPORT getPermissions(const QString &filename); - /** * @brief Retrieve a file inode with csync */ diff --git a/src/libsync/propagatedownload.cpp b/src/libsync/propagatedownload.cpp index 6ad074eb71113..f3553da7749f0 100644 --- a/src/libsync/propagatedownload.cpp +++ b/src/libsync/propagatedownload.cpp @@ -1220,12 +1220,12 @@ void PropagateDownloadFile::downloadFinished() auto previousFileExists = FileSystem::fileExists(filename) && _item->_instruction != CSYNC_INSTRUCTION_CASE_CLASH_CONFLICT; if (previousFileExists) { - // Preserve the existing file permissions. - auto previousPermissions = FileSystem::getPermissions(filename); - if (previousPermissions != FileSystem::getPermissions(_tmpFile.fileName())) { - _tmpFile.setPermissions(previousPermissions); + // Preserve the existing file permissions (except it was a symlink) + const auto existingFile = QFileInfo{filename}; + if (!existingFile.isSymLink() && existingFile.permissions() != _tmpFile.permissions()) { + _tmpFile.setPermissions(existingFile.permissions()); } - preserveGroupOwnership(_tmpFile.fileName(), QFileInfo(filename)); + preserveGroupOwnership(_tmpFile.fileName(), existingFile); // Make the file a hydrated placeholder if possible const auto result = propagator()->syncOptions()._vfs->convertToPlaceholder(_tmpFile.fileName(), *_item, filename); From c61a574599ea822cea602fe83371e76c85cbdfe1 Mon Sep 17 00:00:00 2001 From: Tamino Bauknecht Date: Sun, 10 Dec 2023 11:14:14 +0100 Subject: [PATCH 33/34] ConflictDialog: Fix size and mtime for symlinks in conflict resolution Signed-off-by: Tamino Bauknecht --- src/gui/conflictdialog.cpp | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/src/gui/conflictdialog.cpp b/src/gui/conflictdialog.cpp index bae7b190f957a..a96c44836d1a9 100644 --- a/src/gui/conflictdialog.cpp +++ b/src/gui/conflictdialog.cpp @@ -16,6 +16,7 @@ #include "ui_conflictdialog.h" #include "conflictsolver.h" +#include "filesystem.h" #include #include @@ -132,9 +133,10 @@ void ConflictDialog::updateWidgets() const auto fileUrl = QUrl::fromLocalFile(filename).toString(); linkLabel->setText(QStringLiteral("%2").arg(fileUrl).arg(linkText)); - const auto info = QFileInfo(filename); - mtimeLabel->setText(info.lastModified().toString()); - sizeLabel->setText(locale().formattedDataSize(info.size())); + const auto lastModified = QDateTime::fromTime_t(FileSystem::getModTime(filename)); + const auto fileSize = FileSystem::getSize(filename); + mtimeLabel->setText(lastModified.toString()); + sizeLabel->setText(locale().formattedDataSize(fileSize)); const auto mime = mimeDb.mimeTypeForFile(filename); if (QIcon::hasThemeIcon(mime.iconName())) { From 38f5dac4dfd092a7fc38fb2b752a6aafd33f5b20 Mon Sep 17 00:00:00 2001 From: Tamino Bauknecht Date: Sun, 10 Dec 2023 13:06:28 +0100 Subject: [PATCH 34/34] ActivityListModel: Modify ignore text to not include symlinks --- src/gui/tray/activitylistmodel.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/gui/tray/activitylistmodel.cpp b/src/gui/tray/activitylistmodel.cpp index 09336e0c8a443..86d0aa1a4217a 100644 --- a/src/gui/tray/activitylistmodel.cpp +++ b/src/gui/tray/activitylistmodel.cpp @@ -613,7 +613,7 @@ void ActivityListModel::addIgnoredFileToList(const Activity &newActivity) bool duplicate = false; if (_listOfIgnoredFiles.size() == 0) { _notificationIgnoredFiles = newActivity; - _notificationIgnoredFiles._subject = tr("Files from the ignore list as well as symbolic links are not synced."); + _notificationIgnoredFiles._subject = tr("Files from the ignore list are not synced."); addEntriesToActivityList({_notificationIgnoredFiles}); _listOfIgnoredFiles.append(newActivity); return;