Skip to content

Commit

Permalink
QUrl::resolved: switch to using qt_normalizePathSegments
Browse files Browse the repository at this point in the history 
Commit 4b1547a 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 eaf4438 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 <david.faure@kdab.com>
(cherry picked from commit e7bcf41)
Reviewed-by: Ahmad Samir <a.samirh78@gmail.com>
(cherry picked from commit 2ce3153)
  • Loading branch information
@thiagomacieira
thiagomacieira committed Sep 16, 2024
1 parent e4b2983 commit 214fbc6
Show file tree
Hide file tree
Showing 3 changed files with 15 additions and 119 deletions.
2 changes: 1 addition & 1 deletion src/corelib/io/qdir.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
Expand Down
109 changes: 2 additions & 107 deletions src/corelib/io/qurl.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down Expand Up @@ -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().
Expand Down Expand Up @@ -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);

Expand Down
23 changes: 12 additions & 11 deletions tests/auto/corelib/io/qurl/tst_qurl.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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"));
}

{
Expand Down Expand Up @@ -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)

Expand Down Expand Up @@ -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)
Expand Down

0 comments on commit 214fbc6

Please sign in to comment.