From: Christian Kamm Date: Tue, 5 Jan 2016 10:58:18 +0000 (+0100) Subject: Windows: Fix deleting and replacing of read-only files #4308 X-Git-Tag: archive/raspbian/3.16.7-1_deb13u1+rpi1~1^2~1254^2~114 X-Git-Url: https://dgit.raspbian.org/?a=commitdiff_plain;h=2662203fb7277d5d1a57cef556279f8cd3b974f5;p=nextcloud-desktop.git Windows: Fix deleting and replacing of read-only files #4308 --- diff --git a/src/gui/folder.cpp b/src/gui/folder.cpp index 4f262c232..e55c0e14f 100644 --- a/src/gui/folder.cpp +++ b/src/gui/folder.cpp @@ -570,7 +570,7 @@ int Folder::slotDiscardDownloadProgress() foreach (const SyncJournalDb::DownloadInfo & deleted_info, deleted_infos) { const QString tmppath = folderpath.filePath(deleted_info._tmpfile); qDebug() << "Deleting temporary file: " << tmppath; - QFile::remove(tmppath); + FileSystem::remove(tmppath); } return deleted_infos.size(); } diff --git a/src/libsync/filesystem.cpp b/src/libsync/filesystem.cpp index 15bb001f5..3cd2781cf 100644 --- a/src/libsync/filesystem.cpp +++ b/src/libsync/filesystem.cpp @@ -310,6 +310,11 @@ bool FileSystem::uncheckedRenameReplace(const QString& originFileName, } #else //Q_OS_WIN + // You can not overwrite a read-only file on windows. + if (!QFileInfo(destinationFileName).isWritable()) { + setFileReadOnly(destinationFileName, false); + } + BOOL ok; QString orig = longWinPath(originFileName); QString dest = longWinPath(destinationFileName); @@ -564,4 +569,23 @@ QString FileSystem::makeConflictFileName(const QString &fn, const QDateTime &dt) return conflictFileName; } +bool FileSystem::remove(const QString &fileName, QString *errorString) +{ +#ifdef Q_OS_WIN + // You cannot delete a read-only file on windows, but we want to + // allow that. + if (!QFileInfo(fileName).isWritable()) { + setFileReadOnly(fileName, false); + } +#endif + QFile f(fileName); + if (!f.remove()) { + if (errorString) { + *errorString = f.errorString(); + } + return false; + } + return true; +} + } // namespace OCC diff --git a/src/libsync/filesystem.h b/src/libsync/filesystem.h index 877687aa2..addab46c5 100644 --- a/src/libsync/filesystem.h +++ b/src/libsync/filesystem.h @@ -147,6 +147,14 @@ bool uncheckedRenameReplace(const QString &originFileName, const QString &destinationFileName, QString *errorString); +/** + * Removes a file. + * + * Equivalent to QFile::remove(), except on Windows, where it will also + * successfully remove read-only files. + */ +bool OWNCLOUDSYNC_EXPORT remove(const QString &fileName, QString *errorString = 0); + /** * Replacement for QFile::open(ReadOnly) followed by a seek(). * This version sets a more permissive sharing mode on Windows. diff --git a/src/libsync/propagatedownload.cpp b/src/libsync/propagatedownload.cpp index 689caa2c3..f19dce997 100644 --- a/src/libsync/propagatedownload.cpp +++ b/src/libsync/propagatedownload.cpp @@ -325,7 +325,7 @@ void PropagateDownloadFileQNAM::start() if (progressInfo._valid) { // if the etag has changed meanwhile, remove the already downloaded part. if (progressInfo._etag != _item->_etag) { - QFile::remove(_propagator->getFilePath(progressInfo._tmpfile)); + FileSystem::remove(_propagator->getFilePath(progressInfo._tmpfile)); _propagator->_journal->setDownloadInfo(_item->_file, SyncJournalDb::DownloadInfo()); } else { tmpFileName = progressInfo._tmpfile; @@ -452,7 +452,7 @@ void PropagateDownloadFileQNAM::slotGetFinished() // used a bad range header or the file's not on the server anymore. if (_tmpFile.size() == 0 || badRangeHeader || fileNotFound) { _tmpFile.close(); - _tmpFile.remove(); + FileSystem::remove(_tmpFile.fileName()); _propagator->_journal->setDownloadInfo(_item->_file, SyncJournalDb::DownloadInfo()); } @@ -517,7 +517,7 @@ void PropagateDownloadFileQNAM::slotGetFinished() // Strange bug with broken webserver or webfirewall https://github.com/owncloud/client/issues/3373#issuecomment-122672322 // This happened when trying to resume a file. The Content-Range header was files, Content-Length was == 0 qDebug() << bodySize << _item->_size << _tmpFile.size() << job->resumeStart(); - _tmpFile.remove(); + FileSystem::remove(_tmpFile.fileName()); done(SyncFileItem::SoftError, QLatin1String("Broken webserver returning empty content length for non-empty file on resume")); return; } @@ -546,7 +546,7 @@ void PropagateDownloadFileQNAM::slotGetFinished() void PropagateDownloadFileQNAM::slotChecksumFail( const QString& errMsg ) { - _tmpFile.remove(); + FileSystem::remove(_tmpFile.fileName()); _propagator->_anotherSyncNeeded = true; done(SyncFileItem::SoftError, errMsg ); // tr("The file downloaded with a broken checksum, will be redownloaded.")); } @@ -588,10 +588,9 @@ static void handleRecallFile(const QString &fn) QString fpath = thisDir.filePath(line); QString rpath = makeRecallFileName(fpath); - // if previously recalled file exists then remove it (copy will not overwrite it) - QFile(rpath).remove(); qDebug() << "Copy recall file: " << fpath << " -> " << rpath; - QFile::copy(fpath,rpath); + QString error; + FileSystem::uncheckedRenameReplace(fpath, rpath, &error); } } } // end namespace diff --git a/src/libsync/propagatorjobs.cpp b/src/libsync/propagatorjobs.cpp index 24754615b..1e431bda2 100644 --- a/src/libsync/propagatorjobs.cpp +++ b/src/libsync/propagatorjobs.cpp @@ -66,18 +66,12 @@ bool PropagateLocalRemove::removeRecursively(const QString& path) if (isDir) { ok = removeRecursively(path + QLatin1Char('/') + di.fileName()); // recursive } else { -#ifdef Q_OS_WIN - // On Windows, write only files cannot be deleted. (#4277) - if (!di.fileInfo().isWritable()) { - FileSystem::setFileReadOnlyWeak(di.filePath(),false); - } -#endif - QFile f(di.filePath()); - ok = f.remove(); + QString removeError; + ok = FileSystem::remove(di.filePath(), &removeError); if (!ok) { _error += PropagateLocalRemove::tr("Error removing '%1': %2;"). - arg(QDir::toNativeSeparators(f.fileName()), f.errorString()) + " "; - qDebug() << "Error removing " << f.fileName() << ':' << f.errorString(); + arg(QDir::toNativeSeparators(di.filePath()), removeError) + " "; + qDebug() << "Error removing " << di.filePath() << ':' << removeError; } } if (success && !ok) { @@ -130,9 +124,10 @@ void PropagateLocalRemove::start() return; } } else { - QFile file(filename); - if (FileSystem::fileExists(filename) && !file.remove()) { - done(SyncFileItem::NormalError, file.errorString()); + QString removeError; + if (FileSystem::fileExists(filename) + && !FileSystem::remove(filename, &removeError)) { + done(SyncFileItem::NormalError, removeError); return; } } @@ -154,18 +149,11 @@ void PropagateLocalMkdir::start() // we need to delete the file first. QFileInfo fi(newDirStr); if (_deleteExistingFile && fi.exists() && fi.isFile()) { -#ifdef Q_OS_WIN - // On Windows, write only files cannot be deleted. - if (!fi.isWritable()) { - FileSystem::setFileReadOnlyWeak(newDirStr, false); - } -#endif - - QFile f(newDirStr); - if (!f.remove()) { + QString removeError; + if (!FileSystem::remove(newDirStr, &removeError)) { done( SyncFileItem::NormalError, tr("could not delete file %1, error: %2") - .arg(newDirStr, f.errorString())); + .arg(newDirStr, removeError)); return; } } diff --git a/src/libsync/syncengine.cpp b/src/libsync/syncengine.cpp index cf99c6365..0fa7dc880 100644 --- a/src/libsync/syncengine.cpp +++ b/src/libsync/syncengine.cpp @@ -254,7 +254,7 @@ void SyncEngine::deleteStaleDownloadInfos() foreach (const SyncJournalDb::DownloadInfo & deleted_info, deleted_infos) { const QString tmppath = _propagator->getFilePath(deleted_info._tmpfile); qDebug() << "Deleting stale temporary file: " << tmppath; - QFile::remove(tmppath); + FileSystem::remove(tmppath); } }