Windows: Fix deleting and replacing of read-only files #4308

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();
 }

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

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.

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

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
+            QString removeError;
-            // On Windows, write only files cannot be deleted. (#4277)
+            ok = FileSystem::remove(di.filePath(), &removeError);
-            if (!di.fileInfo().isWritable()) {
-                FileSystem::setFileReadOnlyWeak(di.filePath(),false);
-            }
-#endif
-            QFile f(di.filePath());
-            ok = f.remove();
             if (!ok) {
                 _error += PropagateLocalRemove::tr("Error removing '%1': %2;").
-                    arg(QDir::toNativeSeparators(f.fileName()), f.errorString()) + " ";
+                    arg(QDir::toNativeSeparators(di.filePath()), removeError) + " ";
-                qDebug() << "Error removing " << f.fileName() << ':' << f.errorString();
+                qDebug() << "Error removing " << di.filePath() << ':' << removeError;
             }
         }
         if (success && !ok) {
@@ -130,9 +124,10 @@ void PropagateLocalRemove::start()
             return;
         }
     } else {
-        QFile file(filename);
+        QString removeError;
-        if (FileSystem::fileExists(filename) && !file.remove()) {
+        if (FileSystem::fileExists(filename)
-            done(SyncFileItem::NormalError, file.errorString());
+                && !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
+        QString removeError;
-        // On Windows, write only files cannot be deleted.
+        if (!FileSystem::remove(newDirStr, &removeError)) {
-        if (!fi.isWritable()) {
-            FileSystem::setFileReadOnlyWeak(newDirStr, false);
-        }
-#endif
-
-        QFile f(newDirStr);
-        if (!f.remove()) {
             done( SyncFileItem::NormalError,
                   tr("could not delete file %1, error: %2")
-                  .arg(newDirStr, f.errorString()));
+                  .arg(newDirStr, removeError));
             return;
         }
     }

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);
     }
 }