From 704174bcc9f445cdf9d471acfa380bec6feb3576 Mon Sep 17 00:00:00 2001 From: Robert Griebl Date: Sat, 25 Nov 2017 02:14:03 +0100 Subject: Make HTTP header parsing RFC-compliant MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit All this home-grown parsing should be replaced with code from the qhttp* classes. This would result in a big refactoring though, so this commit at least tries to make the header parsing RFC compliant and not choke on perfectly legal "Header:Value" messages. Change-Id: I14303610dc7187b8d8595568fb221c18e4e0d515 Reviewed-by: Edward Welbourne Reviewed-by: Mårten Nordheim --- src/websockets/qwebsocket_p.cpp | 30 ++++++++++++---- src/websockets/qwebsockethandshakerequest.cpp | 24 +++++++++---- .../handshakerequest/tst_handshakerequest.cpp | 40 ++++++++++++++++++++++ 3 files changed, 81 insertions(+), 13 deletions(-) diff --git a/src/websockets/qwebsocket_p.cpp b/src/websockets/qwebsocket_p.cpp index 1cfa106..9b29114 100644 --- a/src/websockets/qwebsocket_p.cpp +++ b/src/websockets/qwebsocket_p.cpp @@ -964,17 +964,32 @@ void QWebSocketPrivate::processHandshake(QTcpSocket *pSocket) } m_handshakeState = ReadingHeaderState; Q_FALLTHROUGH(); - case ReadingHeaderState: + case ReadingHeaderState: { + // TODO: this should really use the existing code from QHttpNetworkReplyPrivate::parseHeader + auto lastHeader = m_headers.end(); while (pSocket->canReadLine()) { QString headerLine = readLine(pSocket); - const QStringList headerField = headerLine.split(QStringLiteral(": "), - QString::SkipEmptyParts); - if (headerField.size() == 2) { - m_headers.insertMulti(headerField[0].toLower(), headerField[1]); - } - if (headerField.isEmpty()) { + + if (headerLine.isEmpty()) { + // end of headers m_handshakeState = ParsingHeaderState; break; + } else if (headerLine.startsWith(QLatin1Char(' ')) || headerLine.startsWith(QLatin1Char('\t'))) { + // continuation line -- add this to the last header field + if (Q_UNLIKELY(lastHeader == m_headers.end())) { + errorDescription = QWebSocket::tr("Malformed header in response: %1.").arg(headerLine); + break; + } + lastHeader.value().append(QLatin1Char(' ')); + lastHeader.value().append(headerLine.trimmed()); + } else { + int colonPos = headerLine.indexOf(QLatin1Char(':')); + if (Q_UNLIKELY(colonPos <= 0)) { + errorDescription = QWebSocket::tr("Malformed header in response: %1.").arg(headerLine); + break; + } + lastHeader = m_headers.insertMulti(headerLine.left(colonPos).trimmed().toLower(), + headerLine.mid(colonPos + 1).trimmed()); } } @@ -986,6 +1001,7 @@ void QWebSocketPrivate::processHandshake(QTcpSocket *pSocket) return; } Q_FALLTHROUGH(); + } case ParsingHeaderState: { const QString acceptKey = m_headers.value(QStringLiteral("sec-websocket-accept"), QString()); const QString upgrade = m_headers.value(QStringLiteral("upgrade"), QString()); diff --git a/src/websockets/qwebsockethandshakerequest.cpp b/src/websockets/qwebsockethandshakerequest.cpp index e6a626c..bfc8a3d 100644 --- a/src/websockets/qwebsockethandshakerequest.cpp +++ b/src/websockets/qwebsockethandshakerequest.cpp @@ -251,14 +251,26 @@ void QWebSocketHandshakeRequest::readHandshake(QTextStream &textStream, int maxH return; } m_headers.clear(); + // TODO: this should really use the existing code from QHttpNetworkReplyPrivate::parseHeader + auto lastHeader = m_headers.end(); while (!headerLine.isEmpty()) { - const QStringList headerField = headerLine.split(QStringLiteral(": "), - QString::SkipEmptyParts); - if (Q_UNLIKELY(headerField.length() < 2)) { - clear(); - return; + if (headerLine.startsWith(QLatin1Char(' ')) || headerLine.startsWith(QLatin1Char('\t'))) { + // continuation line -- add this to the last header field + if (Q_UNLIKELY(lastHeader == m_headers.end())) { + clear(); + return; + } + lastHeader.value().append(QLatin1Char(' ')); + lastHeader.value().append(headerLine.trimmed()); + } else { + int colonPos = headerLine.indexOf(QLatin1Char(':')); + if (Q_UNLIKELY(colonPos <= 0)) { + clear(); + return; + } + lastHeader = m_headers.insertMulti(headerLine.left(colonPos).trimmed().toLower(), + headerLine.mid(colonPos + 1).trimmed()); } - m_headers.insertMulti(headerField.at(0).toLower(), headerField.at(1)); if (m_headers.size() > maxHeaders) { clear(); return; diff --git a/tests/auto/websockets/handshakerequest/tst_handshakerequest.cpp b/tests/auto/websockets/handshakerequest/tst_handshakerequest.cpp index 4a9603f..230b052 100644 --- a/tests/auto/websockets/handshakerequest/tst_handshakerequest.cpp +++ b/tests/auto/websockets/handshakerequest/tst_handshakerequest.cpp @@ -63,6 +63,7 @@ private Q_SLOTS: void tst_multipleValuesInConnectionHeader(); void tst_multipleVersions(); + void tst_parsingWhitespaceInHeaders(); void tst_qtbug_39355(); void tst_qtbug_48123_data(); @@ -194,6 +195,17 @@ void tst_HandshakeRequest::tst_invalidStream_data() QStringLiteral("Sec-WebSocket-Key: AVDFBDDFF\r\n") + QStringLiteral("Upgrade: websocket,ftp\r\n") + QStringLiteral("Connection: Upgrade\r\n\r\n"); + QTest::newRow("Invalid header - starts with continuation") + << QStringLiteral("GET . HTTP/1.1\r\n Host: foo\r\nSec-WebSocket-Version: 13\r\n") + + QStringLiteral("Sec-WebSocket-Key: AVDFBDDFF\r\n") + + QStringLiteral("Upgrade: websocket\r\n") + + QStringLiteral("Connection: Upgrade\r\n\r\n"); + QTest::newRow("Invalid header - no colon") + << QStringLiteral("GET . HTTP/1.1\r\nHost: foo\r\nSec-WebSocket-Version: 13\r\n") + + QStringLiteral("Sec-WebSocket-Key: AVDFBDDFF\r\n") + + QStringLiteral("Upgrade: websocket\r\n") + + QStringLiteral("X-Custom foo\r\n") + + QStringLiteral("Connection: Upgrade\r\n\r\n"); } void tst_HandshakeRequest::tst_invalidStream() @@ -259,6 +271,34 @@ void tst_HandshakeRequest::tst_multipleValuesInConnectionHeader() QCOMPARE(request.versions().at(0), QWebSocketProtocol::Version13); } +/* + * This is a regression test + * Checks for RFC compliant header parsing + */ +void tst_HandshakeRequest::tst_parsingWhitespaceInHeaders() +{ + //doing extensive QStringLiteral concatenations here, because + //MSVC 2010 complains when using concatenation literal strings about + //concatenation of wide and narrow strings (error C2308) + QString header = QStringLiteral("GET /test HTTP/1.1\r\nHost: ") + + QStringLiteral("foo.com\r\nSec-WebSocket-Version:13\r\n") + + QStringLiteral("Sec-WebSocket-Key: AVD \r\n\tFBDDFF \r\n") + + QStringLiteral("Upgrade:websocket \r\n") + + QStringLiteral("Connection: Upgrade,keepalive\r\n\r\n"); + QByteArray data; + QTextStream textStream(&data); + QWebSocketHandshakeRequest request(80, false); + + textStream << header; + textStream.seek(0); + request.readHandshake(textStream, MAX_HEADERLINE_LENGTH, MAX_HEADERS); + + QVERIFY(request.isValid()); + QCOMPARE(request.key(), QStringLiteral("AVD FBDDFF")); + QCOMPARE(request.versions().length(), 1); + QCOMPARE(request.versions().at(0), QWebSocketProtocol::Version13); +} + void tst_HandshakeRequest::tst_multipleVersions() { QString header = QStringLiteral("GET /test HTTP/1.1\r\nHost: foo.com\r\n") + -- cgit v1.2.1