From 8c5736e68f9619a6f2f223c7363a820e528d089c Mon Sep 17 00:00:00 2001 From: Thiago Macieira Date: Sun, 2 Feb 2025 10:45:49 -0800 Subject: [PATCH] QUrl: avoid going up from the drive path on Windows file URLs On Windows, using a URL of "file:///c:/" as a base to be resolved with "../" should not result in the Windows drive being removed. [ChangeLog][QtCore][QUrl] Fixed a bug (regression from 6.7) where resolving a base URL of an absolute file path containing a Windows drive could result in said drive being removed (e.g., resolving "file:///c:/" with "../" would result in "file:///"). Fixes: QTBUG-133402 Pick-to: 6.8 Change-Id: I58286b9c5e5d02363f0efffdb06f983b560340df Reviewed-by: David Faure (cherry picked from commit 340c9d88ab353e201f117d64609fa5f7d2fa2b21) Reviewed-by: Qt Cherry-pick Bot --- src/corelib/io/qdir.cpp | 45 +++++++++++----- src/corelib/io/qdir_p.h | 1 + src/corelib/io/qurl.cpp | 18 ++++--- tests/auto/corelib/io/qurl/tst_qurl.cpp | 69 +++++++++++++++++++++++++ 4 files changed, 111 insertions(+), 22 deletions(-) diff --git a/src/corelib/io/qdir.cpp b/src/corelib/io/qdir.cpp index 58fccc2e7c2..227aa936318 100644 --- a/src/corelib/io/qdir.cpp +++ b/src/corelib/io/qdir.cpp @@ -52,23 +52,40 @@ static QString driveSpec(const QString &path) #endif // Return the length of the root part of an absolute path, for use by cleanPath(), cd(). -static qsizetype rootLength(QStringView name) +static qsizetype rootLength(QStringView name, QDirPrivate::PathNormalizations flags) { + constexpr bool UseWindowsRules = false // So we don't #include #if defined(Q_OS_WIN) - const qsizetype len = name.size(); - // Handle possible UNC paths which start with double slash - if (name.startsWith("//"_L1)) { - // Server name '//server/path' is part of the prefix. - const qsizetype nextSlash = name.indexOf(u'/', 2); - return nextSlash >= 0 ? nextSlash + 1 : len; - } - if (len >= 2 && name.at(1) == u':') { - // Handle a possible drive letter - return len > 2 && name.at(2) == u'/' ? 3 : 2; - } + || true #endif + ; + const qsizetype len = name.size(); + char16_t firstChar = len > 0 ? name.at(0).unicode() : u'\0'; + char16_t secondChar = len > 1 ? name.at(1).unicode() : u'\0'; + if constexpr (UseWindowsRules) { + // Handle possible UNC paths which start with double slash + bool urlMode = flags.testAnyFlags(QDirPrivate::UrlNormalizationMode); + if (firstChar == u'/' && secondChar == u'/' && !urlMode) { + // Server name '//server/path' is part of the prefix. + const qsizetype nextSlash = name.indexOf(u'/', 2); + return nextSlash >= 0 ? nextSlash + 1 : len; + } - return name.startsWith(u'/') ? 1 : 0; + // Handle a possible drive letter + qsizetype driveLength = 2; + if (firstChar == u'/' && urlMode && len > 2 && name.at(2) == u':') { + // Drive-in-URL-Path mode, e.g. "/c:" or "/c:/autoexec.bat" + ++driveLength; + secondChar = u':'; + } + if (secondChar == u':') { + if (len > driveLength && name.at(driveLength) == u'/') + return driveLength + 1; // absolute drive path, e.g. "c:/config.sys" + return driveLength; // relative drive path, e.g. "c:" or "d:swapfile.sys" + } + } + + return firstChar == u'/' ? 1 : 0; } //************* QDirPrivate @@ -2217,7 +2234,7 @@ bool QDir::match(const QString &filter, const QString &fileName) bool qt_normalizePathSegments(QString *path, QDirPrivate::PathNormalizations flags) { const bool isRemote = flags.testAnyFlag(QDirPrivate::RemotePath); - const qsizetype prefixLength = rootLength(*path); + const qsizetype prefixLength = rootLength(*path, flags); // RFC 3986 says: "The input buffer is initialized with the now-appended // path components and the output buffer is initialized to the empty diff --git a/src/corelib/io/qdir_p.h b/src/corelib/io/qdir_p.h index aebb14b98bd..a18c95e8a79 100644 --- a/src/corelib/io/qdir_p.h +++ b/src/corelib/io/qdir_p.h @@ -29,6 +29,7 @@ class QDirPrivate : public QSharedData public: enum PathNormalization { DefaultNormalization = 0x00, + UrlNormalizationMode = 0x01, RemotePath = 0x02, }; Q_DECLARE_FLAGS(PathNormalizations, PathNormalization) diff --git a/src/corelib/io/qurl.cpp b/src/corelib/io/qurl.cpp index 1e1a992719b..f03e53a2f16 100644 --- a/src/corelib/io/qurl.cpp +++ b/src/corelib/io/qurl.cpp @@ -550,6 +550,13 @@ public: inline bool isLocalFile() const { return flags & IsLocalFile; } QString toLocalFile(QUrl::FormattingOptions options) const; + bool normalizePathSegments(QString *path) const + { + QDirPrivate::PathNormalizations mode = QDirPrivate::UrlNormalizationMode; + if (!isLocalFile()) + mode |= QDirPrivate::RemotePath; + return qt_normalizePathSegments(path, mode); + } QString mergePaths(const QString &relativePath) const; QAtomicInt ref; @@ -910,11 +917,8 @@ inline void QUrlPrivate::appendPassword(QString &appendTo, QUrl::FormattingOptio inline void QUrlPrivate::appendPath(QString &appendTo, QUrl::FormattingOptions options, Section appendingTo) const { QString thePath = path; - if (options & QUrl::NormalizePathSegments) { - qt_normalizePathSegments( - &thePath, - isLocalFile() ? QDirPrivate::DefaultNormalization : QDirPrivate::RemotePath); - } + if (options & QUrl::NormalizePathSegments) + normalizePathSegments(&thePath); QStringView thePathView(thePath); if (options & QUrl::RemoveFilename) { @@ -2714,9 +2718,7 @@ QUrl QUrl::resolved(const QUrl &relative) const else t.d->sectionIsPresent &= ~QUrlPrivate::Fragment; - qt_normalizePathSegments( - &t.d->path, - isLocalFile() ? QDirPrivate::DefaultNormalization : QDirPrivate::RemotePath); + t.d->normalizePathSegments(&t.d->path); if (!t.d->hasAuthority()) { if (t.d->isLocalFile() && t.d->path.startsWith(u'/')) t.d->sectionIsPresent |= QUrlPrivate::Host; diff --git a/tests/auto/corelib/io/qurl/tst_qurl.cpp b/tests/auto/corelib/io/qurl/tst_qurl.cpp index 7b5f86ea2f4..bbe5815cc48 100644 --- a/tests/auto/corelib/io/qurl/tst_qurl.cpp +++ b/tests/auto/corelib/io/qurl/tst_qurl.cpp @@ -61,6 +61,8 @@ private slots: void fromLocalFile(); void fromLocalFileNormalize_data(); void fromLocalFileNormalize(); + void fromLocalFileNormalizeNonRoundtrip_data(); + void fromLocalFileNormalizeNonRoundtrip(); void macTypes(); void relative(); void compat_legacy(); @@ -1424,6 +1426,10 @@ void tst_QUrl::fromLocalFile_data() // Windows absolute details QTest::newRow("windows-drive") << QString::fromLatin1("c:/a.txt") << QString::fromLatin1("file:///c:/a.txt") << QString::fromLatin1("/c:/a.txt"); + // Handling of Windows roots with relative - note, no normalization! + QTest::newRow("windows-drive-above-root") + << QString::fromLatin1("c:/../a.txt") << QString::fromLatin1("file:///c:/../a.txt") << QString::fromLatin1("/c:/../a.txt"); + // Windows UNC paths for (const char *suffix : { "", "/", "/somedir/somefile" }) { const char *pathDescription = @@ -1525,6 +1531,69 @@ void tst_QUrl::fromLocalFileNormalize() QCOMPARE(url.toString(QUrl::NormalizePathSegments), urlWithNormalizedPath); } +void tst_QUrl::fromLocalFileNormalizeNonRoundtrip_data() +{ +#ifdef Q_OS_WIN32 + static constexpr bool IsWindows = true; +#else + static constexpr bool IsWindows = false; +#endif + + QTest::addColumn("input"); + QTest::addColumn("theUrl"); + QTest::addColumn("thePath"); + QTest::addColumn("urlWithNormalizedPath"); + + QTest::newRow("server") << u"//server"_s << u"file://server"_s << QString() << u"file://server"_s; + QTest::newRow("server/..") << u"//server/.."_s << u"file://server/.."_s << u"/.."_s << u"file://server/.."_s; + QTest::newRow("server/share") << u"//server/share"_s << u"file://server/share"_s << u"/share"_s << u"file://server/share"_s; + QTest::newRow("server/share/..") << u"//server/share/.."_s << u"file://server/share/.."_s << u"/share/.."_s << u"file://server/"_s; + + auto addAbsoluteWindowsPathRow = [](const char *name, const QString &input, + const QString &unixNormalized, const QString &windowsNormalized) { + QString thePath = '/' + input; // fromPercentEncoding, but works for now + QString theUrl = "file://" + thePath; + const QString &normalized = IsWindows ? windowsNormalized : unixNormalized; + QTest::newRow(name) << input << theUrl << thePath << normalized; + }; + addAbsoluteWindowsPathRow("relative-drive", "c:", "file:///c:", "file:///c:"); + addAbsoluteWindowsPathRow("absolute-drive", "c:/", "file:///c:/", "file:///c:/"); + addAbsoluteWindowsPathRow("relative-drive/path", "c:autoexec.bat", + "file:///c:autoexec.bat", "file:///c:autoexec.bat"); + addAbsoluteWindowsPathRow("absolute-drive/path", "c:/config.sys", + "file:///c:/config.sys", "file:///c:/config.sys"); + addAbsoluteWindowsPathRow("absolute-drive/path/..", "c:/dos/..", + "file:///c:/", "file:///c:/"); + addAbsoluteWindowsPathRow("absolute-drive/path/../", "c:/dos/../", + "file:///c:/", "file:///c:/"); + + // The drive root should remain for the normalized URLs on Windows + addAbsoluteWindowsPathRow("absolute-drive/..", "c:/..", + "file:///", "file:///c:/.."); + addAbsoluteWindowsPathRow("relative-drive/path/..", "c:dos/..", + "file:///", "file:///c:"); + addAbsoluteWindowsPathRow("relative-drive/path/../", "c:dos/../", + "file:///", "file:///c:"); // Note: trailing / would change meaning! + addAbsoluteWindowsPathRow("relative-drive/path/../..", "c:dos/../..", + "file:///..", "file:///c:.."); + addAbsoluteWindowsPathRow("relative-drive/path/../../", "c:dos/../../", + "file:///../", "file:///c:../"); +} + +void tst_QUrl::fromLocalFileNormalizeNonRoundtrip() +{ + QFETCH(QString, input); + QFETCH(QString, theUrl); + QFETCH(QString, thePath); + QFETCH(QString, urlWithNormalizedPath); + + QUrl url = QUrl::fromLocalFile(input); + + QCOMPARE(url.toString(QUrl::DecodeReserved), theUrl); + QCOMPARE(url.path(), thePath); + QCOMPARE(url.toString(QUrl::NormalizePathSegments), urlWithNormalizedPath); +} + void tst_QUrl::macTypes() { #ifndef Q_OS_DARWIN