diff --git a/autotests/jobtest.h b/autotests/jobtest.h --- a/autotests/jobtest.h +++ b/autotests/jobtest.h @@ -97,6 +97,8 @@ void moveDestAlreadyExistsAutoRename_data(); void moveDestAlreadyExistsAutoRename(); + void safeOverwrite(); + void safeOverwrite_data(); void moveAndOverwrite(); void moveOverSymlinkToSelf(); void createSymlink(); diff --git a/autotests/jobtest.cpp b/autotests/jobtest.cpp --- a/autotests/jobtest.cpp +++ b/autotests/jobtest.cpp @@ -1721,6 +1721,53 @@ } } +void JobTest::safeOverwrite_data() +{ + QTest::addColumn("destFileExists"); + + QTest::newRow("dest file exists") << true; + QTest::newRow("dest file doesn't exist") << false; +} + +void JobTest::safeOverwrite() +{ +#ifdef Q_OS_WIN + QSKIP("Test skipped on Windows"); +#endif + + QFETCH(bool, destFileExists); + const QString srcDir = homeTmpDir() + "overwrite"; + const QString srcFile = srcDir + "/testfile"; + const QString destDir = otherTmpDir() + "overwrite_other"; + const QString destFile = destDir + "/testfile"; + const QString destPartFile = destFile + ".part"; + + createTestDirectory(srcDir); + createTestDirectory(destDir); + + QVERIFY(QFile::resize(srcFile, 1000000)); //~1MB + if (!destFileExists) { + QVERIFY(QFile::remove(destFile)); + } + + KIO::FileCopyJob *job = KIO::file_move(QUrl::fromLocalFile(srcFile), QUrl::fromLocalFile(destFile), -1, KIO::HideProgressInfo | KIO::Overwrite); + job->setUiDelegate(nullptr); + QSignalSpy spyTotalSize(job, &KIO::Job::totalSize); + connect(job, &KIO::Job::totalSize, this, [destFileExists, destPartFile](KJob *job, qulonglong totalSize) { + Q_UNUSED(job); + Q_UNUSED(totalSize); + QCOMPARE(destFileExists, QFile::exists(destPartFile)); + }); + QVERIFY(job->exec()); + QVERIFY(QFile::exists(destFile)); + QVERIFY(!QFile::exists(srcFile)); + QVERIFY(!QFile::exists(destPartFile)); + QCOMPARE(spyTotalSize.count(), 1); + + QDir(srcDir).removeRecursively(); + QDir(destDir).removeRecursively(); +} + void JobTest::moveAndOverwrite() { const QString sourceFile = homeTmpDir() + "fileFromHome"; @@ -2000,20 +2047,27 @@ void JobTest::cancelCopyAndCleanDest_data() { QTest::addColumn("suspend"); + QTest::addColumn("overwrite"); - QTest::newRow("suspend job") << true; - QTest::newRow("don't suspend job") << false; + QTest::newRow("suspend job (without overwrite flag)") << true << false; + QTest::newRow("don't suspend job (without overwrite flag)") << false << false; + +#ifndef Q_OS_WIN + QTest::newRow("suspend job (with overwrite flag)") << true << true; + QTest::newRow("don't suspend job (with overwrite flag)") << false << true; +#endif } void JobTest::cancelCopyAndCleanDest() { QFETCH(bool, suspend); + QFETCH(bool, overwrite); const QString baseDir = homeTmpDir(); const QString srcTemplate = baseDir + QStringLiteral("testfile_XXXXXX"); const QString destFile = baseDir + QStringLiteral("testfile_copy"); - // change file size + QTemporaryFile f(srcTemplate); if (!f.open()) { qFatal("Couldn't open %s", qPrintable(f.fileName())); @@ -2023,17 +2077,23 @@ f.close(); QCOMPARE(f.size(), 10000000); //~10MB - KIO::FileCopyJob *copyJob = KIO::file_copy(QUrl::fromLocalFile(f.fileName()), QUrl::fromLocalFile(destFile), -1, KIO::HideProgressInfo); + if (overwrite) { + createTestFile(destFile); + } + + KIO::JobFlag m_overwriteFlag = overwrite ? KIO::Overwrite : KIO::DefaultFlags; + KIO::FileCopyJob *copyJob = KIO::file_copy(QUrl::fromLocalFile(f.fileName()), QUrl::fromLocalFile(destFile), -1, KIO::HideProgressInfo | m_overwriteFlag); copyJob->setUiDelegate(nullptr); QSignalSpy spyProcessedSize(copyJob, &KIO::Job::processedSize); - connect(copyJob, &KIO::Job::processedSize, this, [destFile, suspend](KJob *job, qulonglong processedSize) { + connect(copyJob, &KIO::Job::processedSize, this, [destFile, suspend, overwrite](KJob *job, qulonglong processedSize) { if (processedSize > 0) { - QVERIFY(QFile::exists(destFile)); + const QString destToCheck = (!overwrite) ? destFile : destFile + QStringLiteral(".part"); + QVERIFY(QFile::exists(destToCheck)); if (suspend) { job->suspend(); } job->kill(); - QVERIFY(!QFile::exists(destFile)); + QVERIFY(!QFile::exists(destToCheck)); } }); QVERIFY(!copyJob->exec()); diff --git a/src/core/filecopyjob.cpp b/src/core/filecopyjob.cpp --- a/src/core/filecopyjob.cpp +++ b/src/core/filecopyjob.cpp @@ -574,7 +574,11 @@ // made as part of move operation then delete the dest only if // source file is intact (m_delJob == NULL). if (d->m_bFileWriteInProgress && d->m_copyJob && d->m_dest.isLocalFile()) { + if (d->m_flags & Overwrite) { + QFile::remove(d->m_dest.toLocalFile() + QStringLiteral(".part")); + } else { QFile::remove(d->m_dest.toLocalFile()); + } } return Job::doKill(); diff --git a/src/ioslaves/file/file_unix.cpp b/src/ioslaves/file/file_unix.cpp --- a/src/ioslaves/file/file_unix.cpp +++ b/src/ioslaves/file/file_unix.cpp @@ -141,9 +141,10 @@ // qDebug() << "copy(): " << srcUrl << " -> " << destUrl << ", mode=" << _mode; const QString src = srcUrl.toLocalFile(); - const QString dest = destUrl.toLocalFile(); + QString dest = destUrl.toLocalFile(); QByteArray _src(QFile::encodeName(src)); QByteArray _dest(QFile::encodeName(dest)); + QByteArray _dest_backup; QT_STATBUF buff_src; #if HAVE_POSIX_ACL @@ -180,24 +181,28 @@ return; } - if (!(_flags & KIO::Overwrite)) { - error(KIO::ERR_FILE_ALREADY_EXIST, dest); - return; - } - - // If the destination is a symlink and overwrite is TRUE, - // remove the symlink first to prevent the scenario where - // the symlink actually points to current source! - if ((_flags & KIO::Overwrite) && ((buff_dest.st_mode & QT_STAT_MASK) == QT_STAT_LNK)) { - //qDebug() << "copy(): LINK DESTINATION"; - if (!QFile::remove(dest)) { - if (auto err = execWithElevatedPrivilege(DEL, {_dest}, errno)) { - if (!err.wasCanceled()) { - error(KIO::ERR_CANNOT_DELETE_ORIGINAL, dest); + if (_flags & KIO::Overwrite) { + // If the destination is a symlink and overwrite is TRUE, + // remove the symlink first to prevent the scenario where + // the symlink actually points to current source! + if ((buff_dest.st_mode & QT_STAT_MASK) == QT_STAT_LNK) { + //qDebug() << "copy(): LINK DESTINATION"; + if (!QFile::remove(dest)) { + if (auto err = execWithElevatedPrivilege(DEL, {_dest}, errno)) { + if (!err.wasCanceled()) { + error(KIO::ERR_CANNOT_DELETE_ORIGINAL, dest); + } + return; } - return; } + } else if ((buff_dest.st_mode & QT_STAT_MASK) == QT_STAT_REG) { + _dest_backup = _dest; + dest.append(QStringLiteral(".part")); + _dest = QFile::encodeName(dest); } + } else { + error(KIO::ERR_FILE_ALREADY_EXIST, dest); + return; } } @@ -255,6 +260,7 @@ #ifdef USE_SENDFILE bool use_sendfile = buff_src.st_size < 0x7FFFFFFF; #endif + bool existing_dest_delete_attempted = false; while (1) { #ifdef USE_SENDFILE if (use_sendfile) { @@ -278,6 +284,11 @@ if (use_sendfile) { // qDebug() << "sendfile() error:" << strerror(errno); if (errno == ENOSPC) { // disk full + if (!_dest_backup.isEmpty() && !existing_dest_delete_attempted) { + ::unlink(_dest_backup.constData()); + existing_dest_delete_attempted = true; + continue; + } error(KIO::ERR_DISK_FULL, dest); } else { error(KIO::ERR_SLAVE_DEFINED, @@ -307,6 +318,11 @@ #endif if (dest_file.write(buffer, n) != n) { if (dest_file.error() == QFileDevice::ResourceError) { // disk full + if (!_dest_backup.isEmpty() && !existing_dest_delete_attempted) { + ::unlink(_dest_backup.constData()); + existing_dest_delete_attempted = true; + continue; + } error(KIO::ERR_DISK_FULL, dest); } else { qCWarning(KIO_FILE) << "Couldn't write[2]. Error:" << dest_file.errorString(); @@ -393,6 +409,16 @@ } } + if (!_dest_backup.isEmpty()) { + if (::unlink(_dest_backup.constData()) == -1) { + qCWarning(KIO_FILE) << "Couldn't remove original dest" << _dest_backup << "(" << strerror(errno) << ")"; + } + + if (::rename(_dest.constData(), _dest_backup.constData()) == -1) { + qCWarning(KIO_FILE) << "Couldn't rename" << _dest << "to" << _dest_backup << "(" << strerror(errno) << ")"; + } + } + processedSize(buff_src.st_size); finished(); }