From 591a9512a2ebdc132d89f742409b5ad606feaad2 Mon Sep 17 00:00:00 2001 From: Thiago Macieira Date: Mon, 24 Jun 2024 13:40:25 -0700 Subject: [PATCH] QFile::moveToTrash/Unix: ensure we try to remove the proper source We already had code to strip ending slashes, which makes sense if trying to trash directories. In the case of symlinks to directories, that also changes from trashing the directory to trashing the symlink. But had to removeFile() on the same modified path. Fixes: QTBUG-126621 Pick-to: 6.7 Change-Id: I46feca3a447244a8ba19fffd17dc0b56f2b1c68c Reviewed-by: Volker Hilsheimer (cherry picked from commit 3302b0cdc15995fb9e70cf9e911d3ab408691b42) Reviewed-by: Qt Cherry-pick Bot --- src/corelib/io/qfile.cpp | 4 +- src/corelib/io/qfilesystemengine_unix.cpp | 2 +- tests/auto/corelib/io/qfile/tst_qfile.cpp | 76 +++++++++++++++++++++++ 3 files changed, 80 insertions(+), 2 deletions(-) diff --git a/src/corelib/io/qfile.cpp b/src/corelib/io/qfile.cpp index ea594470eae..1ca5eea01e2 100644 --- a/src/corelib/io/qfile.cpp +++ b/src/corelib/io/qfile.cpp @@ -462,7 +462,9 @@ QFile::remove(const QString &fileName) //! [move-to-trash-common] The time for this function to run is independent of the size of the file being trashed. If this function is called on a directory, it may be - proportional to the number of files being trashed. + proportional to the number of files being trashed. If the current + fileName() points to a symbolic link, this function will move the link to + the trash, possibly breaking it, not the target of the link. This function uses the Windows and \macos APIs to perform the trashing on those two operating systems. Elsewhere (Unix systems), this function diff --git a/src/corelib/io/qfilesystemengine_unix.cpp b/src/corelib/io/qfilesystemengine_unix.cpp index 0ca32a6b9f4..8a4072098cb 100644 --- a/src/corelib/io/qfilesystemengine_unix.cpp +++ b/src/corelib/io/qfilesystemengine_unix.cpp @@ -1531,7 +1531,7 @@ bool QFileSystemEngine::moveFileToTrash(const QFileSystemEntry &source, renamed = renameat(op.filesDirFd, op.tempTrashFileName, op.filesDirFd, QFile::encodeName(uniqueTrashedName)) == 0; if (renamed) - removeFile(source, error); // success, delete the original file + removeFile(sourcePath, error); // success, delete the original file } if (!renamed) { error = QSystemError(errno, QSystemError::StandardLibraryError); diff --git a/tests/auto/corelib/io/qfile/tst_qfile.cpp b/tests/auto/corelib/io/qfile/tst_qfile.cpp index d69cc167d63..87736fdfc5a 100644 --- a/tests/auto/corelib/io/qfile/tst_qfile.cpp +++ b/tests/auto/corelib/io/qfile/tst_qfile.cpp @@ -292,6 +292,9 @@ private slots: void moveToTrashDuplicateName(); void moveToTrashOpenFile_data(); void moveToTrashOpenFile(); + void moveToTrashSymlinkToFile(); + void moveToTrashSymlinkToDirectory_data(); + void moveToTrashSymlinkToDirectory(); void moveToTrashXdgSafety(); void stdfilesystem(); @@ -4236,6 +4239,79 @@ void tst_QFile::moveToTrashOpenFile() } } +void tst_QFile::moveToTrashSymlinkToFile() +{ +#if defined(Q_OS_ANDROID) || defined(Q_OS_WEBOS) || defined(Q_OS_VXWORKS) + QSKIP("This platform doesn't implement a trash bin"); +#endif + QTemporaryFile temp(QDir::homePath() + "/tst_qfile.moveToTrashSymlinkFile.XXXXXX"); + QVERIFY2(temp.open(), "Failed to create temporary file: " + temp.errorString().toLocal8Bit()); + + // Create the symlink + const QString linkName = temp.fileName() + ".lnk"; + QVERIFY2(temp.link(linkName), "Failed to create link: " + temp.errorString().toLocal8Bit()); + auto cleanLink = qScopeGuard([&]() { + QFile::remove(linkName); + }); + + // now trash it + QFile symlink(linkName); + QVERIFY(symlink.moveToTrash()); + QCOMPARE_NE(symlink.fileName(), linkName); + + // confirm that the target is still a symlink + QFileInfo fi(symlink.fileName()); + QVERIFY(fi.isSymLink()); + QVERIFY(fi.isFile()); // we used an absolute path, so it should not be broken! + symlink.remove(); + + // confirm that the symlink disappeared but the original file is still present + QVERIFY(QFile::exists(temp.fileName())); + QVERIFY(!QFile::exists(linkName)); + cleanLink.dismiss(); +} + +void tst_QFile::moveToTrashSymlinkToDirectory_data() +{ + QTest::addColumn("appendSlash"); + QTest::newRow("without-slash") << false; + QTest::newRow("with-slash") << true; +} + +void tst_QFile::moveToTrashSymlinkToDirectory() +{ +#if defined(Q_OS_ANDROID) || defined(Q_OS_WEBOS) || defined(Q_OS_VXWORKS) + QSKIP("This platform doesn't implement a trash bin"); +#endif + QFETCH(bool, appendSlash); + QTemporaryDir temp(QDir::homePath() + "/tst_qfile.moveToTrashSymlinkDir.XXXXXX"); + QVERIFY2(temp.isValid(), "Failed to create temporary dir: " + temp.errorString().toLocal8Bit()); + + // Create the symlink + const QString linkName = temp.path() + ".lnk"; + QVERIFY(QFile::link(temp.path(), linkName)); + auto cleanLink = qScopeGuard([&]() { + QFile::remove(linkName); + }); + + // now trash it + QFile symlink(appendSlash ? linkName + u'/' : linkName); + QVERIFY(symlink.moveToTrash()); + QCOMPARE_NE(symlink.fileName(), linkName); + QCOMPARE_NE(symlink.fileName(), linkName + u'/'); + + // confirm that the target is still a symlink + QFileInfo fi(symlink.fileName()); + QVERIFY(fi.isSymLink()); + QVERIFY(fi.isDir()); // we used an absolute path, so it should not be broken! + symlink.remove(); + + // confirm that the symlink disappeared but the original dir is still present + QVERIFY(QFile::exists(temp.path())); + QVERIFY(!QFile::exists(linkName)); + cleanLink.dismiss(); +} + void tst_QFile::moveToTrashXdgSafety() { #if defined(Q_OS_VXWORKS)