From 1b393cafd28500825f13f6959535bf31e123bbf5 Mon Sep 17 00:00:00 2001 From: Lena Biliaieva Date: Tue, 3 Sep 2024 18:01:34 +0200 Subject: [PATCH] Network: Add maxConcurrentStreams setting to QHttp2Configuration MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Task-number: QTBUG-75087 Change-Id: I0e42aad6e4e1f534a1070835d1b3cd37486663de Reviewed-by: Mårten Nordheim --- src/network/access/qhttp2configuration.cpp | 30 +++++++++++++++++++++- src/network/access/qhttp2configuration.h | 3 +++ src/network/access/qhttp2connection.cpp | 28 ++++++++++++++++---- src/network/access/qhttp2connection_p.h | 12 +++++++-- 4 files changed, 65 insertions(+), 8 deletions(-) diff --git a/src/network/access/qhttp2configuration.cpp b/src/network/access/qhttp2configuration.cpp index b718ddc7559..95a462396d8 100644 --- a/src/network/access/qhttp2configuration.cpp +++ b/src/network/access/qhttp2configuration.cpp @@ -66,6 +66,8 @@ public: unsigned maxFrameSize = Http2::minPayloadLimit; // Initial (default) value of 16Kb. + unsigned maxConcurrentStreams = Http2::maxConcurrentStreams; + bool pushEnabled = false; // TODO: for now those two below are noop. bool huffmanCompressionEnabled = true; @@ -253,6 +255,31 @@ unsigned QHttp2Configuration::maxFrameSize() const return d->maxFrameSize; } +/*! + \since 6.9 + + Sets \a value as the maximum number of concurrent streams that + will be advertised to the peer when sending SETTINGS frame. + + \sa maxConcurrentStreams() +*/ +void QHttp2Configuration::setMaxConcurrentStreams(unsigned value) +{ + d->maxConcurrentStreams = value; +} + +/*! + \since 6.9 + + Returns the maximum number of concurrent streams. + + \sa setMaxConcurrentStreams() +*/ +unsigned QHttp2Configuration::maxConcurrentStreams() const +{ + return d->maxConcurrentStreams; +} + /*! Swaps this configuration with the \a other configuration. */ @@ -284,7 +311,8 @@ bool QHttp2Configuration::isEqual(const QHttp2Configuration &other) const noexce return d->pushEnabled == other.d->pushEnabled && d->huffmanCompressionEnabled == other.d->huffmanCompressionEnabled && d->sessionWindowSize == other.d->sessionWindowSize - && d->streamWindowSize == other.d->streamWindowSize; + && d->streamWindowSize == other.d->streamWindowSize + && d->maxConcurrentStreams == other.d->maxConcurrentStreams; } QT_END_NAMESPACE diff --git a/src/network/access/qhttp2configuration.h b/src/network/access/qhttp2configuration.h index ae08b664d45..0ee78e462d5 100644 --- a/src/network/access/qhttp2configuration.h +++ b/src/network/access/qhttp2configuration.h @@ -39,6 +39,9 @@ public: bool setMaxFrameSize(unsigned size); unsigned maxFrameSize() const; + void setMaxConcurrentStreams(unsigned value); + unsigned maxConcurrentStreams() const; + void swap(QHttp2Configuration &other) noexcept; private: diff --git a/src/network/access/qhttp2connection.cpp b/src/network/access/qhttp2connection.cpp index ef7b8a28eb7..b5a85740def 100644 --- a/src/network/access/qhttp2connection.cpp +++ b/src/network/access/qhttp2connection.cpp @@ -819,7 +819,7 @@ QHttp2Connection *QHttp2Connection::createUpgradedConnection(QIODevice *socket, connection->m_connectionType = QHttp2Connection::Type::Client; // HTTP2 connection is already established and request was sent, so stream 1 // is already 'active' and is closed for any further outgoing data. - QHttp2Stream *stream = connection->createStreamInternal().unwrap(); + QHttp2Stream *stream = connection->createLocalStreamInternal().unwrap(); Q_ASSERT(stream->streamID() == 1); stream->setState(QHttp2Stream::State::HalfClosedLocal); connection->m_upgradedConnection = true; @@ -885,16 +885,16 @@ QH2Expected QHttp2Connectio Q_ASSERT(m_connectionType == Type::Client); // This overload is just for clients if (m_nextStreamID > lastValidStreamID) return { QHttp2Connection::CreateStreamError::StreamIdsExhausted }; - return createStreamInternal(); + return createLocalStreamInternal(); } QH2Expected -QHttp2Connection::createStreamInternal() +QHttp2Connection::createLocalStreamInternal() { if (m_goingAway) return { QHttp2Connection::CreateStreamError::ReceivedGOAWAY }; const quint32 streamID = m_nextStreamID; - if (size_t(m_maxConcurrentStreams) <= size_t(numActiveLocalStreams())) + if (size_t(m_peerMaxConcurrentStreams) <= size_t(numActiveLocalStreams())) return { QHttp2Connection::CreateStreamError::MaxConcurrentStreamsReached }; if (QHttp2Stream *ptr = createStreamInternal_impl(streamID)) { @@ -1217,6 +1217,7 @@ void QHttp2Connection::setH2Configuration(QHttp2Configuration config) maxSessionReceiveWindowSize = qint32(m_config.sessionReceiveWindowSize()); pushPromiseEnabled = m_config.serverPushEnabled(); streamInitialReceiveWindowSize = qint32(m_config.streamReceiveWindowSize()); + m_maxConcurrentStreams = m_config.maxConcurrentStreams(); encoder.setCompressStrings(m_config.huffmanCompressionEnabled()); } @@ -1413,9 +1414,19 @@ void QHttp2Connection::handleHEADERS() const bool isRemotelyInitiatedStream = isClient ^ isClientInitiatedStream; if (isRemotelyInitiatedStream && streamID > m_lastIncomingStreamID) { + bool streamCountIsOk = size_t(m_maxConcurrentStreams) > size_t(numActiveRemoteStreams()); QHttp2Stream *newStream = createStreamInternal_impl(streamID); Q_ASSERT(newStream); m_lastIncomingStreamID = streamID; + + if (!streamCountIsOk) { + newStream->setState(QHttp2Stream::State::Open); + newStream->streamError(PROTOCOL_ERROR, QLatin1String("Max concurrent streams reached")); + + emit incomingStreamErrorOccured(CreateStreamError::MaxConcurrentStreamsReached); + return; + } + qCDebug(qHttp2ConnectionLog, "[%p] Created new incoming stream %d", this, streamID); emit newIncomingStream(newStream); } else if (auto it = m_streams.constFind(streamID); it == m_streams.cend()) { @@ -1627,6 +1638,7 @@ void QHttp2Connection::handlePUSH_PROMISE() if ((reservedID & 1) || reservedID <= m_lastIncomingStreamID || reservedID > lastValidStreamID) return connectionError(PROTOCOL_ERROR, "PUSH_PROMISE with invalid promised stream ID"); + bool streamCountIsOk = size_t(m_maxConcurrentStreams) > size_t(numActiveRemoteStreams()); // RFC 9113, 6.6: A receiver MUST treat the receipt of a PUSH_PROMISE that promises an // illegal stream identifier (Section 5.1.1) as a connection error auto *stream = createStreamInternal_impl(reservedID); @@ -1635,6 +1647,12 @@ void QHttp2Connection::handlePUSH_PROMISE() m_lastIncomingStreamID = reservedID; stream->setState(QHttp2Stream::State::ReservedRemote); + if (!streamCountIsOk) { + stream->streamError(PROTOCOL_ERROR, QLatin1String("Max concurrent streams reached")); + emit incomingStreamErrorOccured(CreateStreamError::MaxConcurrentStreamsReached); + return; + } + // "ignoring a PUSH_PROMISE frame causes the stream state to become // indeterminate" - let's send RST_STREAM frame with REFUSE_STREAM code. if (!pushPromiseEnabled) { @@ -1955,7 +1973,7 @@ bool QHttp2Connection::acceptSetting(Http2::Settings identifier, quint32 newValu case Settings::MAX_CONCURRENT_STREAMS_ID: { qCDebug(qHttp2ConnectionLog, "[%p] Received SETTINGS MAX_CONCURRENT_STREAMS %d", this, newValue); - m_maxConcurrentStreams = newValue; + m_peerMaxConcurrentStreams = newValue; break; } case Settings::MAX_FRAME_SIZE_ID: { diff --git a/src/network/access/qhttp2connection_p.h b/src/network/access/qhttp2connection_p.h index 76c5cedc341..a0c4bbc9d93 100644 --- a/src/network/access/qhttp2connection_p.h +++ b/src/network/access/qhttp2connection_p.h @@ -249,6 +249,8 @@ public: bool isGoingAway() const noexcept { return m_goingAway; } quint32 maxConcurrentStreams() const noexcept { return m_maxConcurrentStreams; } + quint32 peerMaxConcurrentStreams() const noexcept { return m_peerMaxConcurrentStreams; } + quint32 maxHeaderListSize() const noexcept { return m_maxHeaderListSize; } bool isUpgradedConnection() const noexcept { return m_upgradedConnection; } @@ -263,6 +265,8 @@ Q_SIGNALS: void errorOccurred(Http2::Http2Error errorCode, const QString &errorString); void receivedGOAWAY(Http2::Http2Error errorCode, quint32 lastStreamID); void receivedEND_STREAM(quint32 streamID); + void incomingStreamErrorOccured(CreateStreamError error); + public Q_SLOTS: bool sendPing(); bool sendPing(QByteArrayView data); @@ -273,7 +277,7 @@ private: friend class QHttp2Stream; [[nodiscard]] QIODevice *getSocket() const { return qobject_cast(parent()); } - QH2Expected createStreamInternal(); + QH2Expected createLocalStreamInternal(); QHttp2Stream *createStreamInternal_impl(quint32 streamID); bool isInvalidStream(quint32 streamID) noexcept; @@ -351,11 +355,15 @@ private: // This is how many concurrent streams our peer allows us, 100 is the // initial value, can be updated by the server's SETTINGS frame(s): - quint32 m_maxConcurrentStreams = Http2::maxConcurrentStreams; + quint32 m_peerMaxConcurrentStreams = Http2::maxConcurrentStreams; // While we allow sending SETTTINGS_MAX_CONCURRENT_STREAMS to limit our peer, // it's just a hint and we do not actually enforce it (and we can continue // sending requests and creating streams while maxConcurrentStreams allows). + // This is how many concurrent streams we allow our peer to create + // This value is specified in QHttp2Configuration when creating the connection + quint32 m_maxConcurrentStreams = Http2::maxConcurrentStreams; + // This is our (client-side) maximum possible receive window size, we set // it in a ctor from QHttp2Configuration, it does not change after that. // The default is 64Kb: