summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorRobert Griebl <robert.griebl@pelagicore.com>2017-11-25 02:14:03 +0100
committerRobert Griebl <robert.griebl@pelagicore.com>2017-11-28 11:08:16 +0000
commit704174bcc9f445cdf9d471acfa380bec6feb3576 (patch)
tree89db9ec2e7e76758421592a0ab5af23d8ed3de96
parent534ed7ddbe86c58dc7d59c5c693b027ed8f18737 (diff)
downloadqtwebsockets-704174bcc9f445cdf9d471acfa380bec6feb3576.tar.gz
Make HTTP header parsing RFC-compliant
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 <edward.welbourne@qt.io> Reviewed-by: MÃ¥rten Nordheim <marten.nordheim@qt.io>
-rw-r--r--src/websockets/qwebsocket_p.cpp30
-rw-r--r--src/websockets/qwebsockethandshakerequest.cpp24
-rw-r--r--tests/auto/websockets/handshakerequest/tst_handshakerequest.cpp40
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") +