Skip to content

Commit 4b06a8b

Browse files
TheOneRingTheOneRing
committed
Add unit test for canonicalPath and fix discovered issues
1 parent af8a483 commit 4b06a8b

File tree

5 files changed

+146
-47
lines changed

5 files changed

+146
-47
lines changed

‎src/gui/folderman.cpp‎

Lines changed: 6 additions & 38 deletions
Original file line numberDiff line numberDiff line change
@@ -541,38 +541,6 @@ QString FolderMan::trayTooltipStatusString(
541541
return folderMessage;
542542
}
543543

544-
// canonicalPath returns an empty string if the file does not exist.
545-
// This function also works with files that does not exist and resolve the symlinks in the
546-
// parent directories.
547-
static std::filesystem::path canonicalPath(const std::filesystem::path &p)
548-
{
549-
std::error_code ec;
550-
if (!std::filesystem::exists(p, ec) && !ec) {
551-
const auto parentPath = p.lexically_normal().parent_path();
552-
// last invocation will return /
553-
if (parentPath == p) {
554-
return p;
555-
}
556-
557-
return canonicalPath(parentPath) / p.filename();
558-
}
559-
if (ec) {
560-
qCWarning(lcFolderMan) << "Failed to check existence of path:" << p << ec.message();
561-
}
562-
const auto out = std::filesystem::canonical(p, ec);
563-
if (ec) {
564-
qCWarning(lcFolderMan) << "Failed to canonicalize path:" << p << ec.message();
565-
return p;
566-
}
567-
return out;
568-
}
569-
570-
static QString canonicalPath(const QString &p)
571-
{
572-
// clean path to normalize path back to Qt form
573-
return QDir::cleanPath(FileSystem::fromFilesystemPath(canonicalPath(FileSystem::toFilesystemPath(p))));
574-
}
575-
576544
static QString checkPathForSyncRootMarkingRecursive(const QString &path, FolderMan::NewFolderType folderType, const QUuid &accountUuid)
577545
{
578546
std::pair<QString, QUuid> existingTags = Utility::getDirectorySyncRootMarkings(path);
@@ -659,9 +627,9 @@ QString FolderMan::checkPathValidityForNewFolder(const QString &path, NewFolderT
659627
if (path.isEmpty()) {
660628
return u"Passingg an empty path is not supported"_s;
661629
}
662-
const QString userDir = canonicalPath(path) + QLatin1Char('/');
630+
const QString userDir = FileSystem::canonicalPath(path) + QLatin1Char('/');
663631
for (auto f : _folders) {
664-
const QString folderDir = canonicalPath(f->path()) + QLatin1Char('/');
632+
const QString folderDir = FileSystem::canonicalPath(f->path()) + QLatin1Char('/');
665633

666634
const auto isChild = FileSystem::isChildPathOf2(folderDir, userDir);
667635
if (isChild.testFlag(FileSystem::ChildResult::IsEqual)) {
@@ -701,24 +669,24 @@ QString FolderMan::findGoodPathForNewSyncFolder(
701669
// going to be an acceptable sync folder path for any value of foobar.
702670
// If relativePath is empty, the path is equal to newFolder, and we will find a name in the following loop
703671
QString relativePath;
704-
if (FolderMan::instance()->folderForPath(canonicalPath(normalisedPath), &relativePath) && !relativePath.isEmpty()) {
672+
if (FolderMan::instance()->folderForPath(FileSystem::canonicalPath(normalisedPath), &relativePath) && !relativePath.isEmpty()) {
705673
// Any path with that parent is going to be unacceptable,
706674
// so just keep it as-is.
707-
return canonicalPath(normalisedPath);
675+
return FileSystem::canonicalPath(normalisedPath);
708676
}
709677
// Count attempts and give up eventually
710678
{
711679
QString folder = normalisedPath;
712680
for (int attempt = 2; attempt <= 100; ++attempt) {
713681
if (!QFileInfo::exists(folder) && FolderMan::instance()->checkPathValidityForNewFolder(folder, folderType, accountUuid).isEmpty()) {
714-
return canonicalPath(folder);
682+
return FileSystem::canonicalPath(folder);
715683
}
716684
folder = normalisedPath + QStringLiteral(" (%1)").arg(attempt);
717685
}
718686
}
719687
// we failed to find a non existing path
720688
Q_ASSERT(false);
721-
return canonicalPath(normalisedPath);
689+
return FileSystem::canonicalPath(normalisedPath);
722690
}
723691

724692
bool FolderMan::ignoreHiddenFiles() const

‎src/libsync/common/filesystembase.cpp‎

Lines changed: 13 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -64,11 +64,11 @@ QString FileSystem::fromFilesystemPath(const std::filesystem::path &path)
6464
#ifdef Q_OS_WIN
6565
constexpr std::wstring_view prefix = LR"(\\?\)";
6666
std::wstring nativePath = path.native();
67+
auto view = std::wstring_view(nativePath);
6768
if (nativePath.starts_with(prefix)) {
68-
const auto view = std::wstring_view(nativePath).substr(prefix.size());
69-
return QString::fromWCharArray(view.data(), view.length());
69+
view = view.substr(prefix.size());
7070
}
71-
return QString::fromStdWString(nativePath);
71+
return QDir::fromNativeSeparators(QString::fromWCharArray(view.data(), view.length()));
7272
#elif defined(Q_OS_MACOS)
7373
// based on QFile::decodeName
7474
return QString::fromStdString(path.native()).normalized(QString::NormalizationForm_C);
@@ -86,20 +86,24 @@ QString FileSystem::longWinPath(const QString &inpath)
8686
if (inpath.isEmpty()) {
8787
return inpath;
8888
}
89-
const QString str = QDir::toNativeSeparators(inpath);
89+
QString out = QDir::toNativeSeparators(inpath);
9090
const QLatin1Char sep('\\');
91+
if (out.size() == 2 && out.at(1) == ':'_L1) {
92+
// std::path handles C: incorrectly
93+
out += sep;
94+
}
9195

9296
// we already have a unc path
93-
if (str.startsWith(sep + sep)) {
94-
return str;
97+
if (out.startsWith(sep + sep)) {
98+
return out;
9599
}
96100
// prepend \\?\ and to support long names
97101

98-
if (str.at(0) == sep) {
102+
if (out.at(0) == sep) {
99103
// should not happen as we require the path to be absolute
100-
return QStringLiteral("\\\\?") + str;
104+
return QStringLiteral("\\\\?") + out;
101105
}
102-
return QStringLiteral("\\\\?\\") + str;
106+
return QStringLiteral("\\\\?\\") + out;
103107
#endif
104108
}
105109

‎src/libsync/filesystem.cpp‎

Lines changed: 40 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -176,6 +176,46 @@ bool FileSystem::fileChanged(const std::filesystem::path &path, const FileChange
176176
return false;
177177
}
178178

179+
std::filesystem::path FileSystem::canonicalPath(const std::filesystem::path &p)
180+
{
181+
std::error_code ec;
182+
if (!std::filesystem::exists(p, ec) && !ec) {
183+
const auto normalized = p.lexically_normal();
184+
const auto parentPath = normalized.parent_path();
185+
// last invocation will return /
186+
if (parentPath == p) {
187+
return p;
188+
}
189+
if (normalized.filename().empty()) {
190+
return canonicalPath(parentPath);
191+
} else {
192+
return canonicalPath(parentPath) / normalized.filename();
193+
}
194+
}
195+
if (ec) {
196+
qCWarning(lcFileSystem) << "Failed to check existence of path:" << p << ec.message();
197+
}
198+
const auto out = std::filesystem::canonical(p, ec);
199+
if (ec) {
200+
qCWarning(lcFileSystem) << "Failed to canonicalize path:" << p << ec.message();
201+
return p;
202+
}
203+
#ifdef Q_OS_WIN
204+
// std::filesystem::canonical removes the ucn prefix
205+
const auto ucn = std::wstring(LR"(\\?\)");
206+
Q_ASSERT(!out.native().starts_with(ucn));
207+
return std::filesystem::path(ucn + out.native());
208+
#else
209+
return out;
210+
#endif
211+
}
212+
213+
QString FileSystem::canonicalPath(const QString &p)
214+
{
215+
// clean path to normalize path back to Qt form
216+
return FileSystem::fromFilesystemPath(canonicalPath(FileSystem::toFilesystemPath(p)));
217+
}
218+
179219
qint64 FileSystem::getSize(const std::filesystem::path &filename)
180220
{
181221
std::error_code ec;

‎src/libsync/filesystem.h‎

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -106,6 +106,14 @@ namespace FileSystem {
106106
bool OPENCLOUD_SYNC_EXPORT fileChanged(const std::filesystem::path &path, const FileChangedInfo &previousInfo);
107107

108108

109+
// canonicalPath returns an empty string if the file does not exist.
110+
// This function also works with files that does not exist and resolve the symlinks in the
111+
// parent directories.
112+
std::filesystem::path OPENCLOUD_SYNC_EXPORT canonicalPath(const std::filesystem::path &p);
113+
114+
QString OPENCLOUD_SYNC_EXPORT canonicalPath(const QString &p);
115+
116+
109117
struct RemoveEntry
110118
{
111119
const QString path;

‎test/testutility.cpp‎

Lines changed: 79 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -470,6 +470,85 @@ private Q_SLOTS:
470470
QVERIFY(inode.has_value());
471471
QCOMPARE(fileInfo.inode(), inode.value());
472472
}
473+
474+
void testCanonicalPath()
475+
{
476+
// we compare .native() for std::fiileystem::path, else Qt does actual file comparison
477+
std::error_code ec;
478+
// our build dir might be symlinked, ensure the input path is already canonical
479+
auto path = OCC::FileSystem::fromFilesystemPath(std::filesystem::canonical(qApp->applicationFilePath().toStdString(), ec));
480+
QVERIFY(ec.value() == 0);
481+
QCOMPARE(OCC::FileSystem::canonicalPath(OCC::FileSystem::toFilesystemPath(path)).native(), OCC::FileSystem::toFilesystemPath(path).native());
482+
QCOMPARE(path, OCC::FileSystem::canonicalPath(path));
483+
484+
#ifdef Q_OS_WIN
485+
path = u"C:/"_s;
486+
QCOMPARE(path, OCC::FileSystem::canonicalPath(path));
487+
QCOMPARE(OCC::FileSystem::toFilesystemPath(path).native(), OCC::FileSystem::canonicalPath(OCC::FileSystem::toFilesystemPath(path)).native());
488+
489+
path = u"C:"_s;
490+
QCOMPARE("C:/"_L1, OCC::FileSystem::canonicalPath(path));
491+
QCOMPARE(OCC::FileSystem::toFilesystemPath(path).native(), OCC::FileSystem::canonicalPath(OCC::FileSystem::toFilesystemPath(path)).native());
492+
493+
494+
// test non-existing file, which relies on lexical normalization rather than actual canonical path
495+
path = u"C:/fooo_bar"_s;
496+
QVERIFY(!QFileInfo::exists(path));
497+
QCOMPARE(path, OCC::FileSystem::canonicalPath(path));
498+
QCOMPARE(OCC::FileSystem::toFilesystemPath(path).native(), OCC::FileSystem::canonicalPath(OCC::FileSystem::toFilesystemPath(path)).native());
499+
500+
path = u"C:/fooo_bar/../foo/./../fooo_bar"_s;
501+
QVERIFY(!QFileInfo::exists(path));
502+
QCOMPARE(u"C:/fooo_bar"_s, OCC::FileSystem::canonicalPath(path));
503+
QCOMPARE(
504+
OCC::FileSystem::toFilesystemPath(u"C:/fooo_bar"_s).native(), OCC::FileSystem::canonicalPath(OCC::FileSystem::toFilesystemPath(path)).native());
505+
506+
// test multiple consecutive slashes
507+
path = u"C:///fooo_bar//test///file"_s;
508+
QVERIFY(!QFileInfo::exists(path));
509+
QCOMPARE(u"C:/fooo_bar/test/file"_s, OCC::FileSystem::canonicalPath(path));
510+
QCOMPARE(OCC::FileSystem::toFilesystemPath(u"C:/fooo_bar/test/file"_s).native(),
511+
OCC::FileSystem::canonicalPath(OCC::FileSystem::toFilesystemPath(path)).native());
512+
513+
// test trailing slashes
514+
path = u"C:/fooo_bar///"_s;
515+
QVERIFY(!QFileInfo::exists(path));
516+
QCOMPARE(u"C:/fooo_bar"_s, OCC::FileSystem::canonicalPath(path));
517+
QCOMPARE(
518+
OCC::FileSystem::toFilesystemPath(u"C:/fooo_bar"_s).native(), OCC::FileSystem::canonicalPath(OCC::FileSystem::toFilesystemPath(path)).native());
519+
520+
// test dot segments in middle of path
521+
path = u"C:/fooo_bar/./test/./file"_s;
522+
QVERIFY(!QFileInfo::exists(path));
523+
QCOMPARE(u"C:/fooo_bar/test/file"_s, OCC::FileSystem::canonicalPath(path));
524+
QCOMPARE(OCC::FileSystem::toFilesystemPath(u"C:/fooo_bar/test/file"_s).native(),
525+
OCC::FileSystem::canonicalPath(OCC::FileSystem::toFilesystemPath(path)).native());
526+
527+
// test mixed slashes on Windows
528+
path = u"C:\\fooo_bar/test\\file"_s;
529+
QVERIFY(!QFileInfo::exists(path));
530+
QCOMPARE(u"C:/fooo_bar/test/file"_s, OCC::FileSystem::canonicalPath(path));
531+
QCOMPARE(OCC::FileSystem::toFilesystemPath(u"C:/fooo_bar/test/file"_s).native(),
532+
OCC::FileSystem::canonicalPath(OCC::FileSystem::toFilesystemPath(path)).native());
533+
534+
// test UNC path
535+
path = u"\\\\server\\share\\path"_s;
536+
QCOMPARE(u"//server/share/path"_s, OCC::FileSystem::canonicalPath(path));
537+
QCOMPARE(OCC::FileSystem::toFilesystemPath(u"//server/share/path"_s).native(),
538+
OCC::FileSystem::canonicalPath(OCC::FileSystem::toFilesystemPath(path)).native());
539+
#else
540+
// test non-existing file, which relies on lexical normalization rather than actual canonical path
541+
path = u"/fooo_bar"_s;
542+
QVERIFY(!QFileInfo::exists(path));
543+
QCOMPARE(path, OCC::FileSystem::canonicalPath(path));
544+
QCOMPARE(OCC::FileSystem::toFilesystemPath(path).native(), OCC::FileSystem::canonicalPath(OCC::FileSystem::toFilesystemPath(path)).native());
545+
546+
path = u"/fooo_bar/../foo/./../fooo_bar"_s;
547+
QVERIFY(!QFileInfo::exists(path));
548+
QCOMPARE(u"/fooo_bar"_s, OCC::FileSystem::canonicalPath(path));
549+
QCOMPARE(OCC::FileSystem::toFilesystemPath(u"/fooo_bar"_s).native(), OCC::FileSystem::canonicalPath(OCC::FileSystem::toFilesystemPath(path)).native());
550+
#endif
551+
}
473552
};
474553

475554
QTEST_GUILESS_MAIN(TestUtility)

0 commit comments

Comments
 (0)