HTTP header may be damaged - fix, unit test

"HTTP/1.1 100 CONTINUE\r\n"
If the header from a server is splitted between two packets
the first packet contains "HTTP/1.1 100" and the second one
contains " CONTINUE\r\n", one space (0x20) is skipped. After
processing the line looks in this way "HTTP/1.1 100CONTINUE".
QHttpNetworkReplyPrivate::readStatus(QAbstractSocket *socket)
is called twice, if a http header is splitted as above.
The function always removes whitespace from the beginning of a packet,
even if it is the second part of a http header.

QHttpNetworkReply returns QNetworkReply::RemoteHostClosedError
due to damaged http header during processing.

Improvement of unit test.

Task-number: QTBUG-27161

Change-Id: Ifc2949f62473209b4032185effbf5078b4130cda
Reviewed-by: Qt Doc Bot <qt_docbot@qt-project.org>
Reviewed-by: Shane Kearns <shane.kearns@accenture.com>

diff --git a/src/network/access/qhttpnetworkreply.cpp b/src/network/access/qhttpnetworkreply.cpp
index 1420511..83838fb 100644 (file)
--- a/src/network/access/qhttpnetworkreply.cpp
+++ b/src/network/access/qhttpnetworkreply.cpp
@@ -411,7 +411,7 @@ qint64 QHttpNetworkReplyPrivate::readStatus(QAbstractSocket *socket)
             return -1; // unexpected EOF
         else if (haveRead == 0)
             break; // read more later
-        else if (haveRead == 1 && bytes == 0 && (c == 11 || c == '\n' || c == '\r' || c == ' ' || c == 31))
+        else if (haveRead == 1 && fragment.size() == 0 && (c == 11 || c == '\n' || c == '\r' || c == ' ' || c == 31))
             continue; // Ignore all whitespace that was trailing froma previous request on that socket
 
         bytes++;

diff --git a/tests/auto/network/access/qnetworkreply/tst_qnetworkreply.cpp b/tests/auto/network/access/qnetworkreply/tst_qnetworkreply.cpp
index 968f25f..d6de1f9 100644 (file)
--- a/tests/auto/network/access/qnetworkreply/tst_qnetworkreply.cpp
+++ b/tests/auto/network/access/qnetworkreply/tst_qnetworkreply.cpp
@@ -380,6 +380,9 @@ private Q_SLOTS:
     void qtbug18232gzipContentLengthZero();
     void qtbug22660gzipNoContentLengthEmptyContent();
 
+    void qtbug27161httpHeaderMayBeDamaged_data();
+    void qtbug27161httpHeaderMayBeDamaged();
+
     void synchronousRequest_data();
     void synchronousRequest();
 #ifndef QT_NO_SSL
@@ -6445,6 +6448,77 @@ void tst_QNetworkReply::qtbug22660gzipNoContentLengthEmptyContent()
     QCOMPARE(reply->readAll(), QByteArray());
 }
 
+class QtBug27161Helper : public QObject {
+    Q_OBJECT
+public:
+    QtBug27161Helper(MiniHttpServer & server, const QByteArray & data):
+        m_server(server),
+        m_data(data)
+    {
+        connect(&m_server, SIGNAL(newConnection()), this, SLOT(newConnectionSlot()));
+    }
+public slots:
+    void newConnectionSlot(){
+        connect(m_server.client, SIGNAL(bytesWritten(qint64)), this, SLOT(bytesWrittenSlot()));
+    }
+
+    void bytesWrittenSlot(){
+        disconnect(m_server.client, SIGNAL(bytesWritten(qint64)), this, SLOT(bytesWrittenSlot()));
+        m_Timer.singleShot(100, this, SLOT(timeoutSlot()));
+    }
+
+    void timeoutSlot(){
+        m_server.doClose = true;
+        // we need to emulate the bytesWrittenSlot call if the data is empty.
+        if (m_data.size() == 0)
+            QMetaObject::invokeMethod(&m_server, "bytesWrittenSlot", Qt::QueuedConnection);
+        else
+            m_server.client->write(m_data);
+    }
+
+private:
+    MiniHttpServer & m_server;
+    QByteArray m_data;
+    QTimer m_Timer;
+};
+
+void tst_QNetworkReply::qtbug27161httpHeaderMayBeDamaged_data(){
+    QByteArray response("HTTP/1.0 200 OK\r\nServer: bogus\r\nContent-Length: 3\r\n\r\nABC");
+    QTest::addColumn<QByteArray>("firstPacket");
+    QTest::addColumn<QByteArray>("secondPacket");
+
+    for (int i = 1; i < response.size(); i++){
+        QByteArray dataTag("Iteration: ");
+        dataTag.append(QByteArray::number(i - 1));
+        QTest::newRow(dataTag.constData()) << response.left(i) << response.mid(i);
+    }
+}
+
+/*
+ * Purpose of this test is to check whether a content from server is parsed correctly
+ * if it is splitted into two parts.
+ */
+void tst_QNetworkReply::qtbug27161httpHeaderMayBeDamaged(){
+    QFETCH(QByteArray, firstPacket);
+    QFETCH(QByteArray, secondPacket);
+    MiniHttpServer server(firstPacket);
+    server.doClose = false;
+    QtBug27161Helper helper(server, secondPacket);
+
+    QNetworkRequest request(QUrl("http://localhost:" + QString::number(server.serverPort())));
+    QNetworkReplyPtr reply(manager.get(request));
+
+    QVERIFY(waitForFinish(reply) == Success);
+
+    QVERIFY(reply->isFinished());
+    QCOMPARE(reply->error(), QNetworkReply::NoError);
+    QCOMPARE(reply->size(), qint64(3));
+    QCOMPARE(reply->header(QNetworkRequest::ContentLengthHeader).toLongLong(), qint64(3));
+    QCOMPARE(reply->rawHeader("Content-length"), QByteArray("3"));
+    QCOMPARE(reply->rawHeader("Server"), QByteArray("bogus"));
+    QCOMPARE(reply->readAll(), QByteArray("ABC"));
+}
+
 void tst_QNetworkReply::synchronousRequest_data()
 {
     QTest::addColumn<QUrl>("url");