Implement ping reply in QHttp2Connection and add test

Fixes: QTBUG-122338
Change-Id: I1e8dfa8a93c45dbe12a628d4d5e79d494d8f6032
Reviewed-by: Øystein Heskestad <oystein.heskestad@qt.io>
Reviewed-by: Mårten Nordheim <marten.nordheim@qt.io>

src/network/access/qhttp2connection.cpp

@@ -9,6 +9,7 @@
 #include <QtCore/private/qiodevice_p.h>
 #include <QtCore/private/qnoncontiguousbytedevice_p.h>
 #include <QtCore/qcoreapplication.h>
+#include <QtCore/QRandomGenerator>
 #include <QtCore/qloggingcategory.h>
 
 #include <algorithm>
@@ -922,6 +923,32 @@ bool QHttp2Connection::serverCheckClientPreface()
     return true;
 }
 
+bool QHttp2Connection::sendPing()
+{
+    std::array<char, 8> data;
+
+    QRandomGenerator gen;
+    gen.generate(data.begin(), data.end());
+    return sendPing(data);
+}
+
+bool QHttp2Connection::sendPing(QByteArrayView data)
+{
+    frameWriter.start(FrameType::PING, FrameFlag::EMPTY, connectionStreamID);
+
+    Q_ASSERT(data.length() == 8);
+    if (!m_lastPingSignature) {
+        m_lastPingSignature = data.toByteArray();
+    } else {
+        qCWarning(qHttp2ConnectionLog, "[%p] No PING is sent while waiting for the previous PING.", this);
+        return false;
+    }
+
+    frameWriter.append((uchar*)data.data(), (uchar*)data.end());
+    frameWriter.write(*getSocket());
+    return true;
+}
+
 /*!
     This function must be called when you have received a readyRead signal
     (or equivalent) from the QIODevice. It will read and process any incoming
@@ -1389,18 +1416,30 @@ void QHttp2Connection::handlePUSH_PROMISE()
 
 void QHttp2Connection::handlePING()
 {
-    // @future[server]
-    // Since we're implementing a client and not
-    // a server, we only reply to a PING, ACKing it.
     Q_ASSERT(inboundFrame.type() == FrameType::PING);
+    Q_ASSERT(inboundFrame.dataSize() == 8);
 
     if (inboundFrame.streamID() != connectionStreamID)
         return connectionError(PROTOCOL_ERROR, "PING on invalid stream");
 
-    if (inboundFrame.flags() & FrameFlag::ACK)
-        return connectionError(PROTOCOL_ERROR, "unexpected PING ACK");
+    if (inboundFrame.flags() & FrameFlag::ACK) {
+        QByteArrayView pingSignature(reinterpret_cast<const char *>(inboundFrame.dataBegin()), 8);
+        if (!m_lastPingSignature.has_value()) {
+            emit pingFrameRecived(PingState::PongNoPingSent);
+            qCWarning(qHttp2ConnectionLog, "[%p] PING with ACK received but no PING was sent.", this);
+        } else if (pingSignature != m_lastPingSignature) {
+            emit pingFrameRecived(PingState::PongSignatureChanged);
+            qCWarning(qHttp2ConnectionLog, "[%p] PING signature does not match the last PING.", this);
+        } else {
+            emit pingFrameRecived(PingState::PongSignatureIdentical);
+        }
+        m_lastPingSignature.reset();
+        return;
+    } else {
+        emit pingFrameRecived(PingState::Ping);
+
+    }
 
-    Q_ASSERT(inboundFrame.dataSize() == 8);
 
     frameWriter.start(FrameType::PING, FrameFlag::ACK, connectionStreamID);
     frameWriter.append(inboundFrame.dataBegin(), inboundFrame.dataBegin() + 8);

src/network/access/qhttp2connection_p.h

@@ -197,6 +197,13 @@ public:
     };
     Q_ENUM(CreateStreamError)
 
+    enum class PingState {
+        Ping,
+        PongSignatureIdentical,
+        PongSignatureChanged,
+        PongNoPingSent, // We got an ACKed ping but had not sent any
+    };
+
     // For a pre-established connection:
     [[nodiscard]] static QHttp2Connection *
     createUpgradedConnection(QIODevice *socket, const QHttp2Configuration &config);
@@ -232,9 +239,12 @@ Q_SIGNALS:
     void errorReceived(/*@future: add as needed?*/); // Connection errors only, no stream-specific errors
     void connectionClosed();
     void settingsFrameReceived();
+    void pingFrameRecived(PingState state);
     void errorOccurred(Http2::Http2Error errorCode, const QString &errorString);
     void receivedGOAWAY(quint32 errorCode, quint32 lastStreamID);
 public Q_SLOTS:
+    bool sendPing();
+    bool sendPing(QByteArrayView data);
     void handleReadyRead();
     void handleConnectionClosure();
 
@@ -295,6 +305,7 @@ private:
     QHash<quint32, QPointer<QHttp2Stream>> m_streams;
     QHash<QUrl, quint32> m_promisedStreams;
     QVarLengthArray<quint32> m_resetStreamIDs;
+    std::optional<QByteArray> m_lastPingSignature = std::nullopt;
     quint32 m_nextStreamID = 1;
 
     // Peer's max frame size (this min is the default value

tests/auto/network/access/qhttp2connection/tst_qhttp2connection.cpp

@@ -20,6 +20,7 @@ private slots:
     void construct();
     void constructStream();
     void testSETTINGSFrame();
+    void testPING();
     void connectToServer();
     void WINDOW_UPDATE();
 
@@ -257,6 +258,35 @@ void tst_QHttp2Connection::testSETTINGSFrame()
     }
 }
 
+void tst_QHttp2Connection::testPING()
+{
+    auto [client, server] = makeFakeConnectedSockets();
+    auto connection = makeHttp2Connection(client.get(), {}, Client);
+    auto serverConnection = makeHttp2Connection(server.get(), {}, Server);
+
+    QVERIFY(waitForSettingsExchange(connection, serverConnection));
+
+    QSignalSpy serverPingSpy{ serverConnection, &QHttp2Connection::pingFrameRecived };
+    QSignalSpy clientPingSpy{ connection, &QHttp2Connection::pingFrameRecived };
+
+    QByteArray data{"pingpong"};
+    connection->sendPing(data);
+
+    QVERIFY(serverPingSpy.wait());
+    QVERIFY(clientPingSpy.wait());
+
+    QCOMPARE(serverPingSpy.last().at(0).toInt(), int(QHttp2Connection::PingState::Ping));
+    QCOMPARE(clientPingSpy.last().at(0).toInt(), int(QHttp2Connection::PingState::PongSignatureIdentical));
+
+    serverConnection->sendPing();
+
+    QVERIFY(clientPingSpy.wait());
+    QVERIFY(serverPingSpy.wait());
+
+    QCOMPARE(clientPingSpy.last().at(0).toInt(), int(QHttp2Connection::PingState::Ping));
+    QCOMPARE(serverPingSpy.last().at(0).toInt(), int(QHttp2Connection::PingState::PongSignatureIdentical));
+}
+
 void tst_QHttp2Connection::connectToServer()
 {
     auto [client, server] = makeFakeConnectedSockets();