From 214fbc658bc78ad39e0fb33daf2a3a3fa621b02b Mon Sep 17 00:00:00 2001 From: Thiago Macieira Date: Mon, 5 Aug 2024 17:48:15 -0700 Subject: [PATCH] QUrl::resolved: switch to using qt_normalizePathSegments Commit 4b1547adc9b195e6acc90471fc48dec7ee0c429d rewrote mergePathSegments() but got one thing wrong: that ".." on a "//" should only remove one slash, not all of them. That behavior was introduced by commit eaf4438b3511c8380b9b691b656a87a60e342e2 to match what browsers do. QUrl will use the local file behavior if the URL scheme is "file", unlike the browsers. Task-number: QTBUG-120396 Change-Id: I8a96935cf6c742259c9dfffd17e8fd3cfde46e25 Reviewed-by: David Faure (cherry picked from commit e7bcf41c0b23d83cfb31f966454945c705589a99) Reviewed-by: Ahmad Samir (cherry picked from commit 2ce3153b83615f9fad7e87bfcf548f9167f5d54c) --- src/corelib/io/qdir.cpp | 2 +- src/corelib/io/qurl.cpp | 109 +----------------------- tests/auto/corelib/io/qurl/tst_qurl.cpp | 23 ++--- 3 files changed, 15 insertions(+), 119 deletions(-) diff --git a/src/corelib/io/qdir.cpp b/src/corelib/io/qdir.cpp index 46aee5c895a..606d2a97c78 100644 --- a/src/corelib/io/qdir.cpp +++ b/src/corelib/io/qdir.cpp @@ -73,7 +73,7 @@ static qsizetype rootLength(QStringView name, bool allowUncPaths) return len > 2 && name.at(2) == u'/' ? 3 : 2; } #endif - if (name.at(0) == u'/') + if (len && name.at(0) == u'/') return 1; return 0; } diff --git a/src/corelib/io/qurl.cpp b/src/corelib/io/qurl.cpp index d0f0d957eb8..e867b2d27ea 100644 --- a/src/corelib/io/qurl.cpp +++ b/src/corelib/io/qurl.cpp @@ -911,7 +911,7 @@ inline void QUrlPrivate::appendPath(QString &appendTo, QUrl::FormattingOptions o { QString thePath = path; if (options & QUrl::NormalizePathSegments) { - thePath = qt_normalizePathSegments(path, isLocalFile() ? QDirPrivate::DefaultNormalization : QDirPrivate::RemotePath); + qt_normalizePathSegments(&thePath, isLocalFile() ? QDirPrivate::DefaultNormalization : QDirPrivate::RemotePath); } QStringView thePathView(thePath); @@ -1524,111 +1524,6 @@ inline QString QUrlPrivate::mergePaths(const QString &relativePath) const return newPath; } -/* - From http://www.ietf.org/rfc/rfc3986.txt, 5.2.4: Remove dot segments - - Removes unnecessary ../ and ./ from the path. Used for normalizing - the URL. - - This code has a Qt-specific extension to handle empty path segments (a.k.a. - multiple slashes like "a//b"). We try to keep them wherever possible - because with some protocols they are meaningful, but we still consider them - to be a single directory transition for "." or ".." (e.g., "a/b//c" + - "../" is "a/"). See tst_QUrl::resolved() for the expected behavior. -*/ -static void removeDotsFromPath(QString *path) -{ - // The input buffer is initialized with the now-appended path - // components and the output buffer is initialized to the empty - // string. - const QChar *in = path->constBegin(); - - // Scan the input for a "." or ".." segment. If there isn't any, then we - // don't need to modify this path at all. - qsizetype i = 0, n = path->size(); - for (bool lastWasSlash = true; i < n; ++i) { - if (lastWasSlash && in[i] == u'.') { - if (i + 1 == n || in[i + 1] == u'/') - break; - if (in[i + 1] == u'.' && (i + 2 == n || in[i + 2] == u'/')) - break; - } - lastWasSlash = in[i] == u'/'; - } - if (i == n) - return; - - QChar *out = path->data(); - const QChar *end = out + path->size(); - out += i; - in = out; - - // We implement a modified algorithm compared to RFC 3986, for efficiency. - do { -#if 0 // to see in the debugger - QStringView output(path->constBegin(), out); - QStringView input(in, end); -#endif - // First, copy any preceding slashes, so we can look at the segment's - // content. - while (in < end && in[0] == u'/') { - *out++ = *in++; - - // Note: we may exit this loop with in == end, in which case we - // *shouldn't* dereference *in. But since we are pointing to a - // detached, non-empty QString, we know there's a u'\0' at the end. - } - - // Is this path segment either "." or ".."? - enum { Nothing, Dot, DotDot } type = Nothing; - if (in[0] == u'.') { - if (in + 1 == end || in[1] == u'/') - type = Dot; - else if (in[1] == u'.' && (in + 2 == end || in[2] == u'/')) - type = DotDot; - } - if (type != Nothing) { - // If it is either, we skip it and remove any preceding slashes (if - // any) from the output. If it is "..", we remove the segment - // before that and its preceding slashes (if any) too. - const QChar *start = path->constBegin(); - if (type == DotDot) { - while (out > start && *--out != u'/') - ; - while (out > start && *--out == u'/') - ; - ++in; // the first dot - } - - in += 2; // one dot and either one slash or the terminating null - while (out > start && *--out != u'/') - ; - - // And then replace the segment with "/", unless it would make a - // relative path become absolute. - if (out != start) { - // Replacing with a slash won't make the path absolute. - *out++ = u'/'; - } else if (*start == u'/') { - // The path is already absolute. - ++out; - } else { - // The path is relative, so we must skip any follow-on slashes - // to make sure the next iteration of the loop won't copy them, - // which would make the path become absolute. - while (in < end && *in == u'/') - ++in; - } - continue; - } - - // If it is neither, then we copy this segment. - while (in < end && in->unicode() != '/') - *out++ = *in++; - } while (in < end); - path->truncate(out - path->constBegin()); -} - // Authority-less URLs cannot have paths starting with double slashes (see // QUrlPrivate::validityError). We refuse to turn a valid URL into invalid by // way of QUrl::resolved(). @@ -2817,7 +2712,7 @@ QUrl QUrl::resolved(const QUrl &relative) const else t.d->sectionIsPresent &= ~QUrlPrivate::Fragment; - removeDotsFromPath(&t.d->path); + qt_normalizePathSegments(&t.d->path, isLocalFile() ? QDirPrivate::DefaultNormalization : QDirPrivate::RemotePath); if (!t.d->hasAuthority()) fixupNonAuthorityPath(&t.d->path); diff --git a/tests/auto/corelib/io/qurl/tst_qurl.cpp b/tests/auto/corelib/io/qurl/tst_qurl.cpp index c4a5c077c99..30c2c61d855 100644 --- a/tests/auto/corelib/io/qurl/tst_qurl.cpp +++ b/tests/auto/corelib/io/qurl/tst_qurl.cpp @@ -600,7 +600,7 @@ void tst_QUrl::setUrl() QUrl url2("../../////kdebase/konqueror"); QCOMPARE(url.resolved(url2).toString(), - QString::fromLatin1("file:///usr/local/src/kde2/////kdebase/konqueror")); + QString::fromLatin1("file:///usr/local/src/kde2/kdebase/konqueror")); } { @@ -929,16 +929,16 @@ void tst_QUrl::resolving_data() QTest::newRow(".-on-//") << "http://a/b/c//" << "." << "http://a/b/c//"; QTest::newRow("./-on-//") << "http://a/b/c//" << "./" << "http://a/b/c//"; QTest::newRow(".//-on-//") << "http://a/b/c//" << ".//" << "http://a/b/c///"; // weird but correct - QTest::newRow("..-on-//") << "http://a/b/c//" << ".." << "http://a/b/"; - QTest::newRow("../-on-//") << "http://a/b/c//" << "../" << "http://a/b/"; - QTest::newRow("..//-on-//") << "http://a/b/c//" << "..//" << "http://a/b//"; - QTest::newRow("../g-on-//") << "http://a/b/c//" << "../g" << "http://a/b/g"; - QTest::newRow("..//g-on-//") << "http://a/b/c//" << "..//g" << "http://a/b//g"; - QTest::newRow("../..-on-//") << "http://a/b/c//" << "../.." << "http://a/"; - QTest::newRow("../../-on-//") << "http://a/b/c//" << "../../" << "http://a/"; - QTest::newRow("../..//-on-//") << "http://a/b/c//" << "../..//" << "http://a//"; - QTest::newRow("../../g-on-//") << "http://a/b/c//" << "../../g" << "http://a/g"; - QTest::newRow("../..//g-on-//") << "http://a/b/c//" << "../..//g" << "http://a//g"; + QTest::newRow("..-on-//") << "http://a/b/c//" << ".." << "http://a/b/c/"; + QTest::newRow("../-on-//") << "http://a/b/c//" << "../" << "http://a/b/c/"; + QTest::newRow("..//-on-//") << "http://a/b/c//" << "..//" << "http://a/b/c//"; + QTest::newRow("../g-on-//") << "http://a/b/c//" << "../g" << "http://a/b/c/g"; + QTest::newRow("..//g-on-//") << "http://a/b/c//" << "..//g" << "http://a/b/c//g"; + QTest::newRow("../..-on-//") << "http://a/b/c//" << "../.." << "http://a/b/"; + QTest::newRow("../../-on-//") << "http://a/b/c//" << "../../" << "http://a/b/"; + QTest::newRow("../..//-on-//") << "http://a/b/c//" << "../..//" << "http://a/b//"; + QTest::newRow("../../g-on-//") << "http://a/b/c//" << "../../g" << "http://a/b/g"; + QTest::newRow("../..//g-on-//") << "http://a/b/c//" << "../..//g" << "http://a/b//g"; // 5.4.2 Abnormal Examples (http://www.ietf.org/rfc/rfc3986.txt) @@ -4432,6 +4432,7 @@ void tst_QUrl::normalizeRemotePaths() QCOMPARE(url.adjusted(QUrl::NormalizePathSegments).toString(), expected); QCOMPARE(url.adjusted(QUrl::NormalizePathSegments | QUrl::RemoveFilename).toString(), expectedNoFilename); + QCOMPARE(url.resolved(QUrl(".")).toString(), expectedNoFilename); } QTEST_MAIN(tst_QUrl)