From 60f4fc8b706db9cbeacd5dc4886a7aa347daafc0 Mon Sep 17 00:00:00 2001 From: Tomasz Duda Date: Thu, 6 Sep 2012 22:15:26 +0200 Subject: [PATCH] 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 Reviewed-by: Shane Kearns --- src/network/access/qhttpnetworkreply.cpp | 2 +- .../access/qnetworkreply/tst_qnetworkreply.cpp | 74 ++++++++++++++++++++++ 2 files changed, 75 insertions(+), 1 deletion(-) diff --git a/src/network/access/qhttpnetworkreply.cpp b/src/network/access/qhttpnetworkreply.cpp index 1420511..83838fb 100644 --- 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 --- 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("firstPacket"); + QTest::addColumn("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("url"); -- 2.7.4