QUrl: re-fix the setPath("//path") case leading to scheme://path

Commits aba336c2b4 (in Qt 5.2) and
aba336c2b4 (in 5.6) both tried to deal
with this problem, with different levels of success. This is the third
attempt (and hopefully the charm).

Instead of modifying the path that the user provides, go straight ahead
and declare it invalid. This is supported by RFC 3986, which declares
this expansion impossible:

   relative-part = "//" authority path-abempty
                 / path-absolute
                 / path-noscheme
                 / path-empty
   path-abempty  = *( "/" segment )
   path-absolute = "/" [ segment-nz *( "/" segment ) ]
   path-noscheme = segment-nz-nc *( "/" segment )

The "path-abempty" and "path-noscheme" cases are the two issues we
already handle. This commit adds the third one: path-absolute, which
requires that the first segment of the path be of non-zero length.

That is, it is now possible again to have http://example.com//path
constructed piece-wise, without it producing http://example.com/path.
Additionally, it catches the case of http://example.com//path parsed
from full URL then followed by setAuthority("").

Change-Id: I69f37f9304f24709a823fffd14e67a5e7212ddcd
Reviewed-by: David Faure <david.faure@kdab.com>
bb10
Thiago Macieira 2017-09-21 13:27:51 -07:00
parent 70ee08951a
commit f62768d046
2 changed files with 36 additions and 12 deletions

View File

@ -499,9 +499,10 @@ public:
InvalidFragmentError = Fragment << 8,
// the following two cases are only possible in combination
// with presence/absence of the authority and scheme. See validityError().
// the following three cases are only possible in combination with
// presence/absence of the path, authority and scheme. See validityError().
AuthorityPresentAndPathIsRelative = Authority << 8 | Path << 8 | 0x10000,
AuthorityAbsentAndPathIsDoubleSlash,
RelativeUrlPathContainsColonBeforeSlash = Scheme << 8 | Authority << 8 | Path << 8 | 0x10000,
NoError = 0
@ -1627,19 +1628,32 @@ inline QUrlPrivate::ErrorCode QUrlPrivate::validityError(QString *source, int *p
return error->code;
}
// There are two more cases of invalid URLs that QUrl recognizes and they
// There are three more cases of invalid URLs that QUrl recognizes and they
// are only possible with constructed URLs (setXXX methods), not with
// parsing. Therefore, they are tested here.
//
// The two cases are a non-empty path that doesn't start with a slash and:
// Two cases are a non-empty path that doesn't start with a slash and:
// - with an authority
// - without an authority, without scheme but the path with a colon before
// the first slash
// The third case is an empty authority and a non-empty path that starts
// with "//".
// Those cases are considered invalid because toString() would produce a URL
// that wouldn't be parsed back to the same QUrl.
if (path.isEmpty() || path.at(0) == QLatin1Char('/'))
if (path.isEmpty())
return NoError;
if (path.at(0) == QLatin1Char('/')) {
if (sectionIsPresent & QUrlPrivate::Authority || port != -1 ||
path.length() == 1 || path.at(1) != QLatin1Char('/'))
return NoError;
if (source) {
*source = path;
*position = 0;
}
return AuthorityAbsentAndPathIsDoubleSlash;
}
if (sectionIsPresent & QUrlPrivate::Host) {
if (source) {
*source = path;
@ -2514,10 +2528,7 @@ void QUrl::setPath(const QString &path, ParsingMode mode)
mode = TolerantMode;
}
int from = 0;
while (from < data.length() - 2 && data.midRef(from, 2) == QLatin1String("//"))
++from;
d->setPath(data, from, data.length());
d->setPath(data, 0, data.length());
// optimized out, since there is no path delimiter
// if (path.isNull())
@ -3989,6 +4000,8 @@ static QString errorMessage(QUrlPrivate::ErrorCode errorCode, const QString &err
case QUrlPrivate::AuthorityPresentAndPathIsRelative:
return QStringLiteral("Path component is relative and authority is present");
case QUrlPrivate::AuthorityAbsentAndPathIsDoubleSlash:
return QStringLiteral("Path component starts with '//' and authority is absent");
case QUrlPrivate::RelativeUrlPathContainsColonBeforeSlash:
return QStringLiteral("Relative URL's path component contains ':' before any '/'");
}

View File

@ -2074,6 +2074,14 @@ void tst_QUrl::isValid()
QVERIFY(url.errorString().contains("Path component is relative and authority is present"));
}
{
QUrl url("http:");
url.setPath("//example.com");
QVERIFY(!url.isValid());
QVERIFY(url.toString().isEmpty());
QVERIFY(url.errorString().contains("Path component starts with '//' and authority is absent"));
}
{
QUrl url;
url.setPath("http://example.com");
@ -3649,12 +3657,12 @@ void tst_QUrl::setComponents_data()
QTest::newRow("path-%3A-before-slash") << QUrl()
<< int(Path) << "c%3A/" << Tolerant << true
<< PrettyDecoded << "c%3A/" << "c%3A/";
QTest::newRow("path-doubleslash") << QUrl("trash:/")
QTest::newRow("path-doubleslash") << QUrl("http://example.com")
<< int(Path) << "//path" << Tolerant << true
<< PrettyDecoded << "/path" << "trash:/path";
<< PrettyDecoded << "//path" << "http://example.com//path";
QTest::newRow("path-withdotdot") << QUrl("file:///tmp")
<< int(Path) << "//tmp/..///root/." << Tolerant << true
<< PrettyDecoded << "/tmp/..///root/." << "file:///tmp/..///root/.";
<< PrettyDecoded << "//tmp/..///root/." << "file:////tmp/..///root/.";
// the other fields can be present and be empty
// that is, their delimiters would be present, but there would be nothing to one side
@ -3777,6 +3785,9 @@ void tst_QUrl::setComponents_data()
QTest::newRow("invalid-path-2") << QUrl("http://example.com")
<< int(Path) << "relative" << Strict << false
<< PrettyDecoded << "relative" << "";
QTest::newRow("invalid-path-3") << QUrl("trash:/")
<< int(Path) << "//path" << Tolerant << false
<< PrettyDecoded << "//path" << "";
// -- test bad percent encoding --
// unnecessary to test the scheme, since percent-decoding is not performed in it;