From d39f816b64f6036bbb9fc33d9565b429b3e0b56e Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?M=C3=A5rten=20Nordheim?= <marten.nordheim@qt.io>
Date: Tue, 15 Nov 2022 14:16:23 +0100
Subject: QWebSocket: check correct variable in if-statement

It's _usually_ correct, but if all protocols have been filtered
out then it will be an empty string.

Change-Id: I3aa3221b7e36cfd253cd1152b13e57183bf4f905
Reviewed-by: Timur Pocheptsov <timur.pocheptsov@qt.io>
(cherry picked from commit 0fbd3decd0dd694063edd06a174f137ec30d3741)
Reviewed-by: Qt Cherry-pick Bot <cherrypick_bot@qt-project.org>
---
 src/websockets/qwebsocket_p.cpp                    |  2 +-
 .../auto/websockets/qwebsocket/tst_qwebsocket.cpp  | 71 ++++++++++++++++++++++
 2 files changed, 72 insertions(+), 1 deletion(-)

diff --git a/src/websockets/qwebsocket_p.cpp b/src/websockets/qwebsocket_p.cpp
index 1539916..18efa59 100644
--- a/src/websockets/qwebsocket_p.cpp
+++ b/src/websockets/qwebsocket_p.cpp
@@ -1269,7 +1269,7 @@ QString QWebSocketPrivate::createHandShakeRequest(QString resourceName,
         return validProtocols;
     }();
 
-    if (!protocols.isEmpty()) {
+    if (!validProtocols.isEmpty()) {
         handshakeRequest << QStringLiteral("Sec-WebSocket-Protocol: ")
                                 % validProtocols.join(QLatin1String(", "));
     }
diff --git a/tests/auto/websockets/qwebsocket/tst_qwebsocket.cpp b/tests/auto/websockets/qwebsocket/tst_qwebsocket.cpp
index bfc1ea2..ea5c2b1 100644
--- a/tests/auto/websockets/qwebsocket/tst_qwebsocket.cpp
+++ b/tests/auto/websockets/qwebsocket/tst_qwebsocket.cpp
@@ -15,6 +15,8 @@ QT_USE_NAMESPACE
 
 Q_DECLARE_METATYPE(QWebSocketProtocol::Version)
 
+using namespace Qt::StringLiterals;
+
 class EchoServer : public QObject
 {
     Q_OBJECT
@@ -134,6 +136,8 @@ private Q_SLOTS:
     void tst_openRequest_data();
     void tst_openRequest();
     void tst_protocolAccessor();
+    void protocolsHeaderGeneration_data();
+    void protocolsHeaderGeneration();
     void tst_moveToThread();
     void tst_moveToThreadNoWarning();
 #ifndef QT_NO_NETWORKPROXY
@@ -728,6 +732,73 @@ void tst_QWebSocket::tst_protocolAccessor()
     socket.close();
 }
 
+void tst_QWebSocket::protocolsHeaderGeneration_data()
+{
+    QTest::addColumn<QStringList>("subprotocols");
+    QTest::addColumn<int>("numInvalidEntries");
+
+    using QSL = QStringList;
+    QTest::addRow("all-invalid") << QSL{ "hello?", "------,,,,------" } << 2;
+    QTest::addRow("one-valid") << QSL{ "hello?", "ImValid" } << 1;
+    QTest::addRow("all-valid") << QSL{ "hello", "ImValid" } << 0;
+}
+
+// We test that the Sec-WebSocket-Protocol header is generated normally in presence
+// of one or more invalid entries. That is, it should not be included at all
+// if there are no valid entries, and there should be no separators with only
+// one valid entry.
+void tst_QWebSocket::protocolsHeaderGeneration()
+{
+    QFETCH(const QStringList, subprotocols);
+    QFETCH(const int, numInvalidEntries);
+    const bool containsValidEntry = numInvalidEntries != subprotocols.size();
+
+    QTcpServer tcpServer;
+    QVERIFY(tcpServer.listen());
+
+    QWebSocket socket;
+
+    QUrl url = QUrl("ws://127.0.0.1:%1"_L1.arg(QString::number(tcpServer.serverPort())));
+
+    QWebSocketHandshakeOptions options;
+    options.setSubprotocols(subprotocols);
+
+    QCOMPARE(options.subprotocols().size(), subprotocols.size());
+    for (int i = 0; i < numInvalidEntries; ++i) {
+        QTest::ignoreMessage(QtMsgType::QtWarningMsg,
+                QRegularExpression("Ignoring invalid WebSocket subprotocol name \".*\""));
+    }
+    socket.open(url, options);
+
+    QTRY_VERIFY(tcpServer.hasPendingConnections());
+    QTcpSocket *serverSocket = tcpServer.nextPendingConnection();
+    QVERIFY(serverSocket);
+
+    bool hasSeenHeader = false;
+    while (serverSocket->state() == QAbstractSocket::ConnectedState) {
+        if (!serverSocket->canReadLine()) {
+            QTRY_VERIFY2(serverSocket->canReadLine(),
+                    "Reached end-of-data without seeing end-of-header!");
+        }
+        const QByteArray fullLine = serverSocket->readLine();
+        QByteArrayView line = fullLine;
+        if (line == "\r\n") // End-of-Header
+            break;
+        QByteArrayView headerPrefix = "Sec-WebSocket-Protocol:";
+        if (line.size() < headerPrefix.size())
+            continue;
+        if (line.first(headerPrefix.size()).compare(headerPrefix, Qt::CaseInsensitive) != 0)
+            continue;
+        hasSeenHeader = true;
+        QByteArrayView protocols = line.sliced(headerPrefix.size()).trimmed();
+        QVERIFY(!protocols.empty());
+        QCOMPARE(protocols.count(','), subprotocols.size() - numInvalidEntries - 1);
+        // Keep going in case we encounter the header again
+    }
+    QCOMPARE(hasSeenHeader, containsValidEntry);
+    serverSocket->disconnectFromHost();
+}
+
 class WebSocket : public QWebSocket
 {
     Q_OBJECT
-- 
cgit v1.2.1