Skip to content

Commit 08e21b5

Browse files
committed
test(sync): cover removeRecursively sibling prefix guard
Extract the deleted directory prefix check into isPathInsideDeletedDir and add a unit test verifying a sibling such as "A/BC" is not treated as a child of "A/B", guarding the journal cleanup after a failed local remove. Assisted-by: Claude Code:claude-opus-4-8
1 parent a95d53a commit 08e21b5

3 files changed

Lines changed: 30 additions & 1 deletion

File tree

src/libsync/propagatorjobs.cpp

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -38,6 +38,11 @@ QByteArray localFileIdFromFullId(const QByteArray &id)
3838
return id.left(8);
3939
}
4040

41+
bool isPathInsideDeletedDir(const QString &path, const QString &deletedDir)
42+
{
43+
return !deletedDir.isEmpty() && path.startsWith(deletedDir + QLatin1Char('/'));
44+
}
45+
4146
/**
4247
* The code will update the database in case of error.
4348
* If everything goes well (no error, returns true), the caller is responsible for removing the entries
@@ -87,7 +92,7 @@ bool PropagateLocalRemove::removeRecursively(const QString &path)
8792
for (const auto &it : deleted) {
8893
if (!it.first.startsWith(propagator()->localPath()))
8994
continue;
90-
if (!deletedDir.isEmpty() && it.first.startsWith(deletedDir + QLatin1Char('/')))
95+
if (isPathInsideDeletedDir(it.first, deletedDir))
9196
continue;
9297
if (it.second) {
9398
deletedDir = it.first;

src/libsync/propagatorjobs.h

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,13 @@ constexpr auto checkSumHeaderC = "OC-Checksum";
1919
constexpr auto contentMd5HeaderC = "Content-MD5";
2020
constexpr auto checksumRecalculateOnServerHeaderC = "X-Recalculate-Hash";
2121

22+
/**
23+
* Whether @p path sits inside the already removed directory @p deletedDir.
24+
* Compares with a trailing slash so a sibling such as "A/BC" is not mistaken
25+
* for a child of "A/B".
26+
*/
27+
[[nodiscard]] OWNCLOUDSYNC_EXPORT bool isPathInsideDeletedDir(const QString &path, const QString &deletedDir);
28+
2229
/**
2330
* @brief Declaration of the other propagation jobs
2431
* @ingroup libsync

test/testsyncmove.cpp

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@
1313
#include "common/result.h"
1414
#include "syncenginetestutils.h"
1515
#include <syncengine.h>
16+
#include <propagatorjobs.h>
1617

1718
using namespace OCC;
1819

@@ -1544,6 +1545,22 @@ private slots:
15441545
QVERIFY(cleared._lockstate._lockToken.isEmpty());
15451546
QVERIFY(!cleared._lockstate._locked);
15461547
}
1548+
1549+
// The journal cleanup after a failed local remove must not treat a sibling as a
1550+
// child: "A/BC" is not inside "A/B".
1551+
void testDeletedDirPrefixDoesNotMatchSibling()
1552+
{
1553+
QVERIFY(isPathInsideDeletedDir(QStringLiteral("A/B/child.txt"), QStringLiteral("A/B")));
1554+
QVERIFY(isPathInsideDeletedDir(QStringLiteral("A/B/sub/child.txt"), QStringLiteral("A/B")));
1555+
1556+
// Sibling sharing the prefix must not be skipped.
1557+
QVERIFY(!isPathInsideDeletedDir(QStringLiteral("A/BC"), QStringLiteral("A/B")));
1558+
QVERIFY(!isPathInsideDeletedDir(QStringLiteral("A/BC/child.txt"), QStringLiteral("A/B")));
1559+
1560+
// The directory itself is not inside itself, and an empty deletedDir matches nothing.
1561+
QVERIFY(!isPathInsideDeletedDir(QStringLiteral("A/B"), QStringLiteral("A/B")));
1562+
QVERIFY(!isPathInsideDeletedDir(QStringLiteral("A/B/child.txt"), QString()));
1563+
}
15471564
};
15481565

15491566
QTEST_GUILESS_MAIN(TestSyncMove)

0 commit comments

Comments
 (0)