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