From 87022c9a4ab00a9faaf6bcdaa8d5884962ccf305 Mon Sep 17 00:00:00 2001
From: Thiago Macieira <thiago.macieira@nokia.com>
Date: Wed, 13 May 2009 12:25:36 +0200
Subject: Fix handling of garbage data sent by the HTTP server instead of a
proper HTTP reply.
If the server's reply doesn't start with "HTTP/", then the reply is
not valid. This could happen if we connected via HTTP to an HTTPS
server. It could also happen with an Icecast server (reply ICY/x.y).
Task-number: 248838
Reviewed-by: Markus Goetz
---
src/network/access/qhttpnetworkconnection.cpp | 14 +++++++--
src/network/access/qhttpnetworkreply.cpp | 11 ++++++-
tests/auto/qnetworkreply/tst_qnetworkreply.cpp | 42 ++++++++++++++++++++++++++
3 files changed, 64 insertions(+), 3 deletions(-)
diff --git a/src/network/access/qhttpnetworkconnection.cpp b/src/network/access/qhttpnetworkconnection.cpp
index 5940fba..f558f2b 100644
--- a/src/network/access/qhttpnetworkconnection.cpp
+++ b/src/network/access/qhttpnetworkconnection.cpp
@@ -529,10 +529,20 @@ void QHttpNetworkConnectionPrivate::receiveReply(QAbstractSocket *socket, QHttpN
QHttpNetworkReplyPrivate::ReplyState state = reply ? reply->d_func()->state : QHttpNetworkReplyPrivate::AllDoneState;
switch (state) {
case QHttpNetworkReplyPrivate::NothingDoneState:
- case QHttpNetworkReplyPrivate::ReadingStatusState:
- bytes += reply->d_func()->readStatus(socket);
+ case QHttpNetworkReplyPrivate::ReadingStatusState: {
+ qint64 statusBytes = reply->d_func()->readStatus(socket);
+ if (statusBytes == -1) {
+ // error reading the status, close the socket and emit error
+ socket->close();
+ reply->d_func()->errorString = errorDetail(QNetworkReply::ProtocolFailure, socket);
+ emit reply->finishedWithError(QNetworkReply::ProtocolFailure, reply->d_func()->errorString);
+ QMetaObject::invokeMethod(q, "_q_startNextRequest", Qt::QueuedConnection);
+ break;
+ }
+ bytes += statusBytes;
channels[i].lastStatus = reply->d_func()->statusCode;
break;
+ }
case QHttpNetworkReplyPrivate::ReadingHeaderState:
bytes += reply->d_func()->readHeader(socket);
if (reply->d_func()->state == QHttpNetworkReplyPrivate::ReadingDataState) {
diff --git a/src/network/access/qhttpnetworkreply.cpp b/src/network/access/qhttpnetworkreply.cpp
index fe3f6af..1b41e1e 100644
--- a/src/network/access/qhttpnetworkreply.cpp
+++ b/src/network/access/qhttpnetworkreply.cpp
@@ -409,6 +409,9 @@ qint64 QHttpNetworkReplyPrivate::readStatus(QAbstractSocket *socket)
if (fragment.endsWith('\r')) {
fragment.truncate(fragment.length()-1);
}
+ if (!fragment.startsWith("HTTP/"))
+ return -1;
+
parseStatus(fragment);
state = ReadingHeaderState;
fragment.clear(); // next fragment
@@ -418,7 +421,13 @@ qint64 QHttpNetworkReplyPrivate::readStatus(QAbstractSocket *socket)
bytes += socket->read(&c, 1);
fragment.append(c);
}
+
+ // is this a valid reply?
+ if (fragment.length() >= 5 && !fragment.startsWith("HTTP/"))
+ return -1;
+
}
+
return bytes;
}
@@ -660,4 +669,4 @@ void QHttpNetworkReply::ignoreSslErrors()
QT_END_NAMESPACE
-#endif // QT_NO_HTTP
\ No newline at end of file
+#endif // QT_NO_HTTP
diff --git a/tests/auto/qnetworkreply/tst_qnetworkreply.cpp b/tests/auto/qnetworkreply/tst_qnetworkreply.cpp
index 104b788..c97fcf3 100644
--- a/tests/auto/qnetworkreply/tst_qnetworkreply.cpp
+++ b/tests/auto/qnetworkreply/tst_qnetworkreply.cpp
@@ -172,6 +172,8 @@ private Q_SLOTS:
void ioGetFromHttpsWithIgnoreSslErrors();
void ioGetFromHttpsWithSslHandshakeError();
#endif
+ void ioGetFromHttpBrokenServer_data();
+ void ioGetFromHttpBrokenServer();
void ioGetWithManyProxies_data();
void ioGetWithManyProxies();
@@ -1887,6 +1889,46 @@ void tst_QNetworkReply::ioGetFromHttpsWithSslHandshakeError()
}
#endif
+void tst_QNetworkReply::ioGetFromHttpBrokenServer_data()
+{
+ QTest::addColumn<QByteArray>("dataToSend");
+ QTest::addColumn<bool>("doDisconnect");
+
+ QTest::newRow("no-newline") << QByteArray("Hello World") << false;
+ QTest::newRow("just-newline") << QByteArray("\r\n") << false;
+ QTest::newRow("just-2newline") << QByteArray("\r\n\r\n") << false;
+ QTest::newRow("with-newlines") << QByteArray("Long first line\r\nLong second line") << false;
+ QTest::newRow("with-newlines2") << QByteArray("\r\nSecond line") << false;
+ QTest::newRow("with-newlines3") << QByteArray("ICY\r\nSecond line") << false;
+
+ QTest::newRow("empty+disconnect") << QByteArray() << true;
+
+ QTest::newRow("no-newline+disconnect") << QByteArray("Hello World") << true;
+ QTest::newRow("just-newline+disconnect") << QByteArray("\r\n") << true;
+ QTest::newRow("just-2newline+disconnect") << QByteArray("\r\n\r\n") << true;
+ QTest::newRow("with-newlines+disconnect") << QByteArray("Long first line\r\nLong second line") << true;
+ QTest::newRow("with-newlines2+disconnect") << QByteArray("\r\nSecond line") << true;
+ QTest::newRow("with-newlines3+disconnect") << QByteArray("ICY\r\nSecond line") << true;
+}
+
+void tst_QNetworkReply::ioGetFromHttpBrokenServer()
+{
+ QFETCH(QByteArray, dataToSend);
+ QFETCH(bool, doDisconnect);
+ MiniHttpServer server(dataToSend);
+ server.doClose = doDisconnect;
+
+ QNetworkRequest request(QUrl("http://localhost:" + QString::number(server.serverPort())));
+ QNetworkReplyPtr reply = manager.get(request);
+
+ connect(reply, SIGNAL(finished()), &QTestEventLoop::instance(), SLOT(exitLoop()));
+ QTestEventLoop::instance().enterLoop(10);
+ QVERIFY(!QTestEventLoop::instance().timeout());
+
+ QCOMPARE(reply->url(), request.url());
+ QVERIFY(reply->error() != QNetworkReply::NoError);
+}
+
void tst_QNetworkReply::ioGetWithManyProxies_data()
{
QTest::addColumn<QList<QNetworkProxy> >("proxyList");
--
cgit v0.12