QUrl: make sure setPort(nonnegative) is taken as part of authority

There were a couple of corner cases where doing setPort() would result
in QUrl thinking that an authority was not present. Since the full URL
parsing implies that a host is always present if the authority is
present, then we also imply that setting the port number makes the host
be present too.

Change-Id: I69f37f9304f24709a823fffd14e67c12da18d69f
Reviewed-by: David Faure <david.faure@kdab.com>
This commit is contained in:
Thiago Macieira 2017-09-21 13:59:05 -07:00
parent 4e339a5ac1
commit 9574436666
2 changed files with 41 additions and 8 deletions

View File

@ -1037,6 +1037,7 @@ inline void QUrlPrivate::setAuthority(const QString &auth, int from, int end, QU
{
sectionIsPresent &= ~Authority;
sectionIsPresent |= Host;
port = -1;
// we never actually _loop_
while (from != end) {
@ -1061,10 +1062,8 @@ inline void QUrlPrivate::setAuthority(const QString &auth, int from, int end, QU
}
}
if (colonIndex == end - 1) {
// found a colon but no digits after it
port = -1;
} else if (uint(colonIndex) < uint(end)) {
if (uint(colonIndex) < uint(end) - 1) {
// found a colon with digits after it
unsigned long x = 0;
for (int i = colonIndex + 1; i < end; ++i) {
ushort c = auth.at(i).unicode();
@ -1083,8 +1082,6 @@ inline void QUrlPrivate::setAuthority(const QString &auth, int from, int end, QU
if (mode == QUrl::StrictMode)
break;
}
} else {
port = -1;
}
setHost(auth, from, qMin<uint>(end, colonIndex), mode);
@ -1644,8 +1641,7 @@ inline QUrlPrivate::ErrorCode QUrlPrivate::validityError(QString *source, int *p
if (path.isEmpty())
return NoError;
if (path.at(0) == QLatin1Char('/')) {
if (sectionIsPresent & QUrlPrivate::Authority || port != -1 ||
path.length() == 1 || path.at(1) != QLatin1Char('/'))
if (hasAuthority() || path.length() == 1 || path.at(1) != QLatin1Char('/'))
return NoError;
if (source) {
*source = path;
@ -2474,6 +2470,8 @@ void QUrl::setPort(int port)
}
d->port = port;
if (port != -1)
d->sectionIsPresent |= QUrlPrivate::Host;
}
/*!

View File

@ -2080,6 +2080,11 @@ void tst_QUrl::isValid()
QVERIFY(!url.isValid());
QVERIFY(url.toString().isEmpty());
QVERIFY(url.errorString().contains("Path component starts with '//' and authority is absent"));
// should disappear if we set a port
url.setPort(80);
QVERIFY(url.isValid());
QCOMPARE(url.toString(), QString("http://:80//example.com"));
}
{
@ -2088,6 +2093,13 @@ void tst_QUrl::isValid()
QVERIFY(!url.isValid());
QVERIFY(url.toString().isEmpty());
QVERIFY(url.errorString().contains("':' before any '/'"));
// this specific error disappears if we set anything in the authority,
// but then we run into another error
url.setPort(80);
QVERIFY(!url.isValid());
QVERIFY(url.toString().isEmpty());
QVERIFY(url.errorString().contains("Path component is relative and authority is present"));
}
{
@ -2827,6 +2839,29 @@ void tst_QUrl::setPort()
QCOMPARE(url.port(), -1);
QVERIFY(url.errorString().contains("out of range"));
}
{
QUrl reference("//:80");
QUrl piecewise;
piecewise.setPort(80);
QCOMPARE(piecewise, reference);
}
{
// setAuthority must clear the port
QUrl url("http://example.com:80");
url.setAuthority("example.org");
QCOMPARE(url.port(), -1);
QCOMPARE(url.toString(), QString("http://example.org"));
}
{
// setAuthority must clear the port
QUrl url("http://example.com:80");
url.setAuthority(QString());
QCOMPARE(url.port(), -1);
QCOMPARE(url.toString(), QString("http:"));
}
}
void tst_QUrl::port_data()