From e4ada4e19ec0bcb2ff1d2c35316a85067be7793d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?M=C3=A5rten=20Nordheim?= Date: Tue, 15 Nov 2022 14:12:34 +0100 Subject: QWebSocket: honor subprotocols specified with setRawHeader We would error out with a ConnectionRejected if the server accepted one of the protocols specified directly in the header since we did not consider those at all. Fixes: QTBUG-108276 Change-Id: Ifbb316c9d4871fd764e03c74caefa10f5b757155 Reviewed-by: Timur Pocheptsov (cherry picked from commit 69e2b30057003c0ed17c3478d60fea249546a168) Reviewed-by: Qt Cherry-pick Bot --- src/websockets/qwebsocket_p.cpp | 30 +++++++++++++++++++--- src/websockets/qwebsocket_p.h | 2 ++ src/websockets/qwebsocket_wasm_p.cpp | 9 +------ .../qwebsocketserver/tst_qwebsocketserver.cpp | 28 +++++++++++++++----- 4 files changed, 50 insertions(+), 19 deletions(-) diff --git a/src/websockets/qwebsocket_p.cpp b/src/websockets/qwebsocket_p.cpp index 18efa59..8ad72c3 100644 --- a/src/websockets/qwebsocket_p.cpp +++ b/src/websockets/qwebsocket_p.cpp @@ -318,6 +318,24 @@ void QWebSocketPrivate::_q_updateSslConfiguration() #endif +QStringList QWebSocketPrivate::requestedSubProtocols() const +{ + auto subprotocolsRequestedInRawHeader = [this]() { + QStringList protocols; + QByteArray rawProtocols = m_request.rawHeader("Sec-WebSocket-Protocol"); + QLatin1StringView rawProtocolsView(rawProtocols); + const QStringList &optionsProtocols = m_options.subprotocols(); + for (auto &&entry : rawProtocolsView.tokenize(u',', Qt::SkipEmptyParts)) { + if (QLatin1StringView trimmed = entry.trimmed(); !trimmed.isEmpty()) { + if (!optionsProtocols.contains(trimmed)) + protocols << trimmed; + } + } + return protocols; + }; + return m_options.subprotocols() + subprotocolsRequestedInRawHeader(); +} + /*! Called from QWebSocketServer \internal @@ -1004,8 +1022,7 @@ void QWebSocketPrivate::processHandshake(QTcpSocket *pSocket) #endif const QString protocol = QString::fromLatin1(parser.combinedHeaderValue( QByteArrayLiteral("sec-websocket-protocol"))); - - if (!protocol.isEmpty() && !handshakeOptions().subprotocols().contains(protocol)) { + if (!protocol.isEmpty() && !requestedSubProtocols().contains(protocol)) { setErrorString(QWebSocket::tr("WebSocket server has chosen protocol %1 which has not been " "requested") .arg(protocol)); @@ -1098,9 +1115,14 @@ void QWebSocketPrivate::processStateChanged(QAbstractSocket::SocketState socketS QList > headers; const auto headerList = m_request.rawHeaderList(); - for (const QByteArray &key : headerList) + for (const QByteArray &key : headerList) { + // protocols handled separately below + if (key.compare("Sec-WebSocket-Protocol", Qt::CaseInsensitive) == 0) + continue; headers << qMakePair(QString::fromLatin1(key), QString::fromLatin1(m_request.rawHeader(key))); + } + const QStringList subProtocols = requestedSubProtocols(); const auto format = QUrl::RemoveScheme | QUrl::RemoveUserInfo | QUrl::RemovePath | QUrl::RemoveQuery @@ -1110,7 +1132,7 @@ void QWebSocketPrivate::processStateChanged(QAbstractSocket::SocketState socketS host, origin(), QString(), - m_options.subprotocols(), + subProtocols, m_key, headers); if (handshake.isEmpty()) { diff --git a/src/websockets/qwebsocket_p.h b/src/websockets/qwebsocket_p.h index cddb6b3..7e293fe 100644 --- a/src/websockets/qwebsocket_p.h +++ b/src/websockets/qwebsocket_p.h @@ -152,6 +152,8 @@ private: void enableMasking(bool enable); void setErrorString(const QString &errorString); + QStringList requestedSubProtocols() const; + void socketDestroyed(QObject *socket); void processData(); diff --git a/src/websockets/qwebsocket_wasm_p.cpp b/src/websockets/qwebsocket_wasm_p.cpp index 4435889..d783f90 100644 --- a/src/websockets/qwebsocket_wasm_p.cpp +++ b/src/websockets/qwebsocket_wasm_p.cpp @@ -173,15 +173,8 @@ void QWebSocketPrivate::open(const QNetworkRequest &request, // required for some use cases like MQTT. // add user subprotocol options - QStringList protocols = handshakeOptions().subprotocols(); - + QStringList protocols = requestedSubProtocols(); QByteArray secProto; - if (request.hasRawHeader("Sec-WebSocket-Protocol")) { - secProto = request.rawHeader("Sec-WebSocket-Protocol"); - if (!protocols.contains(secProto)) { - protocols.append(QString::fromLatin1(secProto)); - } - } if (!protocols.isEmpty()) { // comma-separated list of protocol strings, no spaces secProto = protocols.join(QStringLiteral(",")).toLatin1(); diff --git a/tests/auto/websockets/qwebsocketserver/tst_qwebsocketserver.cpp b/tests/auto/websockets/qwebsocketserver/tst_qwebsocketserver.cpp index d6dd356..0fda305 100644 --- a/tests/auto/websockets/qwebsocketserver/tst_qwebsocketserver.cpp +++ b/tests/auto/websockets/qwebsocketserver/tst_qwebsocketserver.cpp @@ -356,20 +356,28 @@ void tst_QWebSocketServer::tst_connectivity() void tst_QWebSocketServer::tst_protocols_data() { QTest::addColumn("clientProtocols"); + QTest::addColumn("headerProtocols"); QTest::addColumn("expectedProtocol"); - QTest::addRow("none") << QStringList {} << QString {}; + + QTest::addRow("none") << QStringList{} << QStringList{} << QString{}; QTest::addRow("same order as server") - << QStringList { "chat", "superchat" } << QStringLiteral("chat"); + << QStringList{ "chat", "superchat" } << QStringList{} << QStringLiteral("chat"); QTest::addRow("different order from server") - << QStringList { "superchat", "chat" } << QStringLiteral("superchat"); - QTest::addRow("unsupported protocol") << QStringList { "foo" } << QString {}; + << QStringList{ "superchat", "chat" } << QStringList{} << QStringLiteral("superchat"); + QTest::addRow("unsupported protocol") << QStringList{ "foo" } << QStringList{} << QString{}; QTest::addRow("mixed supported/unsupported protocol") - << QStringList { "foo", "chat" } << QStringLiteral("chat"); + << QStringList{ "foo", "chat" } << QStringList{} << QStringLiteral("chat"); + + QTest::addRow("same order as server, in header") + << QStringList{} << QStringList{ "chat", "superchat" } << QStringLiteral("chat"); + QTest::addRow("specified in options and in header") + << QStringList{ "superchat" } << QStringList{ "chat" } << QStringLiteral("superchat"); } void tst_QWebSocketServer::tst_protocols() { QFETCH(QStringList, clientProtocols); + QFETCH(QStringList, headerProtocols); QFETCH(QString, expectedProtocol); QWebSocketServer server(QString(), QWebSocketServer::NonSecureMode); @@ -380,9 +388,15 @@ void tst_QWebSocketServer::tst_protocols() QVERIFY(server.listen()); QWebSocketHandshakeOptions opt; + QNetworkRequest request(server.serverUrl()); + if (!headerProtocols.isEmpty()) { + QString protocols = headerProtocols.join(u','); + request.setRawHeader("Sec-WebSocket-Protocol", protocols.toUtf8()); + } opt.setSubprotocols(clientProtocols); + QWebSocket client; - client.open(server.serverUrl(), opt); + client.open(request, opt); QTRY_COMPARE(client.state(), QAbstractSocket::ConnectedState); QTRY_COMPARE(newConnectionSpy.size(), 1); @@ -392,7 +406,7 @@ void tst_QWebSocketServer::tst_protocols() QCOMPARE(client.subprotocol(), expectedProtocol); QCOMPARE(serverSocket->subprotocol(), expectedProtocol); - QCOMPARE(serverSocket->handshakeOptions().subprotocols(), clientProtocols); + QCOMPARE(serverSocket->handshakeOptions().subprotocols(), clientProtocols + headerProtocols); } void tst_QWebSocketServer::tst_preSharedKey() -- cgit v1.2.1