Skip to content
New issue

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

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

Already on GitHub? Sign in to your account

set the parent folder read/write always when downloading a new file #7808

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
78 changes: 50 additions & 28 deletions src/libsync/propagatedownload.cpp
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
/*

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

View workflow job for this annotation

GitHub Actions / build

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

File src/libsync/propagatedownload.cpp does not conform to Custom style guidelines. (lines 789, 790, 793, 794, 797, 798)
* Copyright (C) by Olivier Goffart <ogoffart@owncloud.com>
*
* This program is free software; you can redistribute it and/or modify
Expand All @@ -12,7 +12,7 @@
* for more details.
*/

#include "config.h"

Check failure on line 15 in src/libsync/propagatedownload.cpp

View workflow job for this annotation

GitHub Actions / build

src/libsync/propagatedownload.cpp:15:10 [clang-diagnostic-error]

'config.h' file not found
#include "owncloudpropagator_p.h"
#include "propagatedownload.h"
#include "networkjobs.h"
Expand Down Expand Up @@ -490,7 +490,8 @@

// For virtual files just dehydrate or create the file and be done
if (_item->_type == ItemTypeVirtualFileDehydration) {
QString fsPath = propagator()->fullLocalPath(_item->_file);
const auto fsPath = propagator()->fullLocalPath(_item->_file);
makeParentFolderModifiable(fsPath);
if (!FileSystem::verifyFileUnchanged(fsPath, _item->_previousSize, _item->_previousModtime)) {
propagator()->_anotherSyncNeeded = true;
done(SyncFileItem::SoftError, tr("File has changed since discovery"), ErrorCategory::GenericError);
Expand All @@ -499,6 +500,7 @@

qCDebug(lcPropagateDownload) << "dehydrating file" << _item->_file;
if (FileSystem::isLnkFile(fsPath)) {
makeParentFolderModifiable(_tmpFile.fileName());
const auto convertResult = vfs->convertToPlaceholder(fsPath, *_item);
if (!convertResult) {
qCCritical(lcPropagateDownload()) << "error when converting a shortcut file to placeholder" << convertResult.error();
Expand Down Expand Up @@ -532,6 +534,10 @@
}
if (_item->_type == ItemTypeVirtualFile && !propagator()->localFileNameClash(_item->_file)) {
qCDebug(lcPropagateDownload) << "creating virtual file" << _item->_file;

const auto fsPath = propagator()->fullLocalPath(_item->_file);
makeParentFolderModifiable(fsPath);

// do a klaas' case clash check.
if (propagator()->localFileNameClash(_item->_file)) {
done(SyncFileItem::FileNameClash, tr("File %1 can not be downloaded because of a local file name clash!").arg(QDir::toNativeSeparators(_item->_file)), ErrorCategory::GenericError);
Expand Down Expand Up @@ -666,6 +672,7 @@
tmpFileName = createDownloadTmpFileName(_item->_file);
}
_tmpFile.setFileName(propagator()->fullLocalPath(tmpFileName));
makeParentFolderModifiable(_tmpFile.fileName());

_resumeStart = _tmpFile.size();
if (_resumeStart > 0 && _resumeStart == _item->_size) {
Expand All @@ -680,32 +687,6 @@
FileSystem::setFileReadOnly(_tmpFile.fileName(), false);
}

#if !defined(Q_OS_MACOS) || __MAC_OS_X_VERSION_MIN_REQUIRED >= MAC_OS_X_VERSION_10_15
try {
const auto newDirPath = std::filesystem::path{_tmpFile.fileName().toStdWString()};
Q_ASSERT(newDirPath.has_parent_path());
_parentPath = newDirPath.parent_path();
}
catch (const std::filesystem::filesystem_error &e)
{
qCWarning(lcPropagateDownload) << "exception when checking parent folder access rights" << e.what() << e.path1().c_str() << e.path2().c_str();
}
catch (const std::system_error &e)
{
qCWarning(lcPropagateDownload) << "exception when checking parent folder access rights" << e.what();
}
catch (...)
{
qCWarning(lcPropagateDownload) << "exception when checking parent folder access rights";
}

if (FileSystem::isFolderReadOnly(_parentPath)) {
FileSystem::setFolderPermissions(QString::fromStdWString(_parentPath.wstring()), FileSystem::FolderPermissions::ReadWrite);
emit propagator()->touchedFile(QString::fromStdWString(_parentPath.wstring()));
_needParentFolderRestorePermissions = true;
}
#endif

if (!_tmpFile.open(QIODevice::Append | QIODevice::Unbuffered)) {
qCWarning(lcPropagateDownload) << "could not open temporary file" << _tmpFile.fileName();
done(SyncFileItem::NormalError, _tmpFile.errorString(), ErrorCategory::GenericError);
Expand Down Expand Up @@ -786,6 +767,47 @@
_deleteExisting = enabled;
}

void PropagateDownloadFile::done(const SyncFileItem::Status status, const QString &errorString, const ErrorCategory category)
{
#if !defined(Q_OS_MACOS) || __MAC_OS_X_VERSION_MIN_REQUIRED >= MAC_OS_X_VERSION_10_15
if (_needParentFolderRestorePermissions) {
FileSystem::setFolderPermissions(QString::fromStdWString(_parentPath.wstring()), FileSystem::FolderPermissions::ReadOnly);
emit propagator()->touchedFile(QString::fromStdWString(_parentPath.wstring()));
_needParentFolderRestorePermissions = false;
}
#endif
PropagateItemJob::done(status, errorString, category);
}

void PropagateDownloadFile::makeParentFolderModifiable(const QString &fileName)
{
#if !defined(Q_OS_MACOS) || __MAC_OS_X_VERSION_MIN_REQUIRED >= MAC_OS_X_VERSION_10_15
try {
const auto newDirPath = std::filesystem::path{fileName.toStdWString()};
Q_ASSERT(newDirPath.has_parent_path());
_parentPath = newDirPath.parent_path();
}
catch (const std::filesystem::filesystem_error &e)
{
qCWarning(lcPropagateDownload) << "exception when checking parent folder access rights" << e.what() << e.path1().c_str() << e.path2().c_str();
}
catch (const std::system_error &e)
{
qCWarning(lcPropagateDownload) << "exception when checking parent folder access rights" << e.what();
}
catch (...)
{
qCWarning(lcPropagateDownload) << "exception when checking parent folder access rights";
}

if (FileSystem::isFolderReadOnly(_parentPath)) {
FileSystem::setFolderPermissions(QString::fromStdWString(_parentPath.wstring()), FileSystem::FolderPermissions::ReadWrite);
emit propagator()->touchedFile(QString::fromStdWString(_parentPath.wstring()));
_needParentFolderRestorePermissions = true;
}
#endif
}

const char owncloudCustomSoftErrorStringC[] = "owncloud-custom-soft-error-string";
void PropagateDownloadFile::slotGetFinished()
{
Expand Down Expand Up @@ -1181,7 +1203,7 @@
}
}

void PropagateDownloadFile::downloadFinished()

Check warning on line 1206 in src/libsync/propagatedownload.cpp

View workflow job for this annotation

GitHub Actions / build

src/libsync/propagatedownload.cpp:1206:29 [readability-function-cognitive-complexity]

function 'downloadFinished' has cognitive complexity of 54 (threshold 25)
{
ASSERT(!_tmpFile.isOpen());
const auto filename = propagator()->fullLocalPath(_item->_file);
Expand Down Expand Up @@ -1328,7 +1350,7 @@

#if !defined(Q_OS_MACOS) || __MAC_OS_X_VERSION_MIN_REQUIRED >= MAC_OS_X_VERSION_10_15
if (_needParentFolderRestorePermissions) {
FileSystem::setFolderPermissions(QString::fromStdWString(_parentPath.wstring()), FileSystem::FolderPermissions::ReadWrite);
FileSystem::setFolderPermissions(QString::fromStdWString(_parentPath.wstring()), FileSystem::FolderPermissions::ReadOnly);
emit propagator()->touchedFile(QString::fromStdWString(_parentPath.wstring()));
_needParentFolderRestorePermissions = false;
}
Expand Down
5 changes: 5 additions & 0 deletions src/libsync/propagatedownload.h
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@
*/
#pragma once

#include "owncloudlib.h"

Check failure on line 16 in src/libsync/propagatedownload.h

View workflow job for this annotation

GitHub Actions / build

src/libsync/propagatedownload.h:16:10 [clang-diagnostic-error]

'owncloudlib.h' file not found
#include "owncloudpropagator.h"
#include "networkjobs.h"
#include "clientsideencryption.h"
Expand Down Expand Up @@ -220,6 +220,11 @@
*/
void setDeleteExistingFolder(bool enabled);

protected:
void done(const SyncFileItem::Status status, const QString &errorString, const ErrorCategory category) override;

void makeParentFolderModifiable(const QString &fileName);

private slots:
/// Called when ComputeChecksum on the local file finishes,
/// maybe the local and remote checksums are identical?
Expand Down
Loading