From 305deaca22eff7377c53bcbe73167a756181fc63 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?M=C3=A5rten=20Nordheim?= <marten.nordheim@qt.io>
Date: Wed, 7 Feb 2024 18:56:19 +0100
Subject: [PATCH] Http/2: fix active streams counting

We were looking at all active streams, but that also includes promised
streams.

By the RFC the limitation that our peer specifies only applies to the
number of streams _we_ create, not the total amount of active streams.

More importantly, for the qhttp2protocolhandler it could mean that we
could end up having a few promised streams push the active stream count
over the limit, which could lead us to start more streams than intended
(then bounded by the number of queued requests).

The worst case in this scenario is that a **non-compliant** server
doesn't track how many connections we open and the user has queued
a ton of requests, so we open a ton of streams.

But on the upside: server-push is not widely used.

Change-Id: I2a533472eb9127fd176bb99e9db0518f05c3fffe
Reviewed-by: Timur Pocheptsov <timur.pocheptsov@qt.io>
Reviewed-by: Edward Welbourne <edward.welbourne@qt.io>
Reviewed-by:  Alexey Edelev <alexey.edelev@qt.io>
(cherry picked from commit 5ed736b053eb9d04ecd1a6f2f375cce7fcefe4e6)
Reviewed-by: Qt Cherry-pick Bot <cherrypick_bot@qt-project.org>
---
 src/network/access/qhttp2connection.cpp      | 32 ++++++++++++++++----
 src/network/access/qhttp2connection_p.h      |  5 ++-
 src/network/access/qhttp2protocolhandler.cpp |  5 ++-
 3 files changed, 34 insertions(+), 8 deletions(-)

diff --git a/src/network/access/qhttp2connection.cpp b/src/network/access/qhttp2connection.cpp
index 64e10724fa7..32190d2dee7 100644
--- a/src/network/access/qhttp2connection.cpp
+++ b/src/network/access/qhttp2connection.cpp
@@ -552,7 +552,7 @@ QHttp2Connection::createStreamInternal()
     if (m_goingAway)
         return { QHttp2Connection::CreateStreamError::ReceivedGOAWAY };
     const quint32 streamID = m_nextStreamID;
-    if (size_t(m_maxConcurrentStreams) <= size_t(numActiveStreams()))
+    if (size_t(m_maxConcurrentStreams) <= size_t(numActiveLocalStreams()))
         return { QHttp2Connection::CreateStreamError::MaxConcurrentStreamsReached };
     m_nextStreamID += 2;
     return { createStreamInternal_impl(streamID) };
@@ -570,12 +570,32 @@ QHttp2Stream *QHttp2Connection::createStreamInternal_impl(quint32 streamID)
     return stream;
 }
 
-qsizetype QHttp2Connection::numActiveStreams() const noexcept
+qsizetype QHttp2Connection::numActiveStreamsImpl(quint32 mask) const noexcept
 {
-    return std::count_if(m_streams.cbegin(), m_streams.cend(),
-                         [](const QPointer<QHttp2Stream> &stream) {
-                             return stream && stream->state() == QHttp2Stream::State::Open;
-                         });
+    const auto shouldCount = [mask](const QPointer<QHttp2Stream> &stream) -> bool {
+        return stream && (stream->streamID() & 1) == mask;
+    };
+    return std::count_if(m_streams.cbegin(), m_streams.cend(), shouldCount);
+}
+
+/*!
+    \internal
+    The number of streams the remote peer has started that are still active.
+*/
+qsizetype QHttp2Connection::numActiveRemoteStreams() const noexcept
+{
+    const quint32 RemoteMask = m_connectionType == Type::Client ? 0 : 1;
+    return numActiveStreamsImpl(RemoteMask);
+}
+
+/*!
+    \internal
+    The number of streams we have started that are still active.
+*/
+qsizetype QHttp2Connection::numActiveLocalStreams() const noexcept
+{
+    const quint32 LocalMask = m_connectionType == Type::Client ? 1 : 0;
+    return numActiveStreamsImpl(LocalMask);
 }
 
 QHttp2Stream *QHttp2Connection::getStream(quint32 streamID) const
diff --git a/src/network/access/qhttp2connection_p.h b/src/network/access/qhttp2connection_p.h
index 5d525537e58..4d70ae36771 100644
--- a/src/network/access/qhttp2connection_p.h
+++ b/src/network/access/qhttp2connection_p.h
@@ -20,6 +20,7 @@
 #include <QtCore/qobject.h>
 #include <QtCore/qhash.h>
 #include <QtCore/qvarlengtharray.h>
+#include <QtCore/qxpfunctional.h>
 #include <QtNetwork/qhttp2configuration.h>
 #include <QtNetwork/qtcpsocket.h>
 
@@ -253,7 +254,9 @@ private:
                          const char *message); // Connection failed to be established?
     void setH2Configuration(QHttp2Configuration config);
     void closeSession();
-    qsizetype numActiveStreams() const noexcept;
+    qsizetype numActiveStreamsImpl(quint32 mask) const noexcept;
+    qsizetype numActiveRemoteStreams() const noexcept;
+    qsizetype numActiveLocalStreams() const noexcept;
 
     bool sendClientPreface();
     bool sendSETTINGS();
diff --git a/src/network/access/qhttp2protocolhandler.cpp b/src/network/access/qhttp2protocolhandler.cpp
index d17b62565ee..f2508383d8b 100644
--- a/src/network/access/qhttp2protocolhandler.cpp
+++ b/src/network/access/qhttp2protocolhandler.cpp
@@ -325,7 +325,10 @@ bool QHttp2ProtocolHandler::sendRequest()
         initReplyFromPushPromise(message, key);
     }
 
-    const qint64 streamsToUse = qBound(0, qint64(maxConcurrentStreams) - activeStreams.size(),
+    const auto isClientSide = [](const auto &pair) -> bool { return (pair.first & 1) == 1; };
+    const auto activeClientSideStreams = std::count_if(
+            activeStreams.constKeyValueBegin(), activeStreams.constKeyValueEnd(), isClientSide);
+    const qint64 streamsToUse = qBound(0, qint64(maxConcurrentStreams) - activeClientSideStreams,
                                        requests.size());
     auto it = requests.begin();
     for (qint64 i = 0; i < streamsToUse; ++i) {