File CopyJob-Fix-crash-when-skipping.patch of Package kio

Overview Repositories Revisions Requests Users Attributes Meta

File CopyJob-Fix-crash-when-skipping.patch of Package kio

From 4306dce1e8d9ca61ef5b7246bd602e21a5e63621 Mon Sep 17 00:00:00 2001
From: Ahmad Samir <a.samirh78@gmail.com>
Date: Fri, 6 Sep 2019 16:04:49 +0200
Subject: [CopyJob] Fix crash when copying an already existing dir and pressing
 "Skip"

Summary:
In copyNextFile() if all files have been skipped QList::erase() will
return end() iterator, accessing the element it denotes will cause
a segmentation fault. Make sure the iterator is valid if it's changed
inside the while loop, if we're going to use it before control reaches
the loop condition.

Add a unit test.

BUG: 408350
FIXED-IN: 5.62.0

Test Plan:
kioclient5 copy SOME_DIR_WITH_FILES DEST
kioclient5 copy --interactive SOME_DIR_WITH_FILES DEST

- In the "folder already exists" dialog enable "Apply to all" then hit "Skip"
- Without the patch you'd get a segmentation fault, with the patch the copy
  should finish as expected

All unit tests passed (except kiocore-kacltest, but that's unrelated).

Reviewers: #frameworks, dfaure

Reviewed By: dfaure

Subscribers: kde-frameworks-devel

Tags: #frameworks

Differential Revision: https://phabricator.kde.org/D23708
---
 autotests/jobtest.cpp | 31 +++++++++++++++++++++++++++++++
 autotests/jobtest.h   |  2 ++
 src/core/copyjob.cpp  |  2 +-
 3 files changed, 34 insertions(+), 1 deletion(-)

diff --git a/autotests/jobtest.cpp b/autotests/jobtest.cpp
index 47fdf08..c852934 100644
--- a/autotests/jobtest.cpp
+++ b/autotests/jobtest.cpp
@@ -1732,6 +1732,37 @@ void JobTest::moveDestAlreadyExistsAutoRename(const QString &destDir, bool moveD
     }
 }
 
+void JobTest::copyDirectoryAlreadyExistsSkip()
+{
+    // when copying a directory (which contains at least one file) to some location, and then
+    // copying the same dir to the same location again, and clicking "Skip" there should be no
+    // segmentation fault, bug 408350
+
+    const QString src = homeTmpDir() + "a";
+    createTestDirectory(src);
+    const QString dest = homeTmpDir() + "dest";
+    createTestDirectory(dest);
+
+    QUrl u = QUrl::fromLocalFile(src);
+    QUrl d = QUrl::fromLocalFile(dest);
+
+    KIO::Job *job = KIO::copy(u, d, KIO::HideProgressInfo);
+    job->setUiDelegate(nullptr);
+    QVERIFY(job->exec());
+    QVERIFY(QFile::exists(dest + QStringLiteral("/a/testfile")));
+
+    job = KIO::copy(u, d, KIO::HideProgressInfo);
+    // Simulate the user pressing "Skip" in the dialog.
+    PredefinedAnswerJobUiDelegate extension;
+    extension.m_skipResult = KIO::S_SKIP;
+    job->setUiDelegateExtension(&extension);
+    QVERIFY(job->exec());
+    QVERIFY(QFile::exists(dest + QStringLiteral("/a/testfile")));
+
+    QDir(src).removeRecursively();
+    QDir(dest).removeRecursively();
+}
+
 void JobTest::moveAndOverwrite()
 {
     const QString sourceFile = homeTmpDir() + "fileFromHome";
diff --git a/autotests/jobtest.h b/autotests/jobtest.h
index fd58799..009f3ad 100644
--- a/autotests/jobtest.h
+++ b/autotests/jobtest.h
@@ -97,6 +97,8 @@ private Q_SLOTS:
     void moveDestAlreadyExistsAutoRename_data();
     void moveDestAlreadyExistsAutoRename();
 
+    void copyDirectoryAlreadyExistsSkip();
+
     void moveAndOverwrite();
     void moveOverSymlinkToSelf();
     void createSymlink();
diff --git a/src/core/copyjob.cpp b/src/core/copyjob.cpp
index 5fd3c21..e551f89 100644
--- a/src/core/copyjob.cpp
+++ b/src/core/copyjob.cpp
@@ -1637,7 +1637,7 @@ void CopyJobPrivate::copyNextFile()
             it = files.erase(it);
         }
 
-        if ((*it).size > ((1ul << 32) -1)) { // (1ul << 32 -1) = 4 GB
+        if (it != files.end() && (*it).size > ((1ul << 32) - 1)) { // ((1ul << 32) - 1) = 4 GB
             const auto fileSystem = KFileSystemType::fileSystemType(m_globalDest.toString());
             if (fileSystem == KFileSystemType::Fat) {
                 q->setError(ERR_FILE_TOO_LARGE_FOR_FAT32);
-- 
cgit v1.1

Places

File CopyJob-Fix-crash-when-skipping.patch of Package kio

Places