From 10d7da6168f704260c19c5a20d974a7c0b7f1441 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?M=C3=A5rten=20Nordheim?= Date: Tue, 23 Jul 2024 12:32:23 +0200 Subject: [PATCH] Http2: Use the enum for error codes and fix bugs The code was all initially taken out of the QNAM protocol handler, so it was using QNetworkReply error-values somewhat liberally. That means it would emit quint32s with values from two different enums and no good way to tell them apart. Instead, just use the Http2Error enum for everything. Pick-to: 6.7 Change-Id: I31be4e3ac8f0439478c22b173cd8830550d16ddb Reviewed-by: Dennis Oberst Reviewed-by: Alexey Edelev (cherry picked from commit 28dd5e0a716039a058b9156509909c2da639909a) Reviewed-by: Qt Cherry-pick Bot --- src/network/access/http2/http2protocol_p.h | 2 +- src/network/access/qhttp2connection.cpp | 43 +++++++++++----------- src/network/access/qhttp2connection_p.h | 12 +++--- 3 files changed, 28 insertions(+), 29 deletions(-) diff --git a/src/network/access/http2/http2protocol_p.h b/src/network/access/http2/http2protocol_p.h index f0f18d1dd54..1936d272b03 100644 --- a/src/network/access/http2/http2protocol_p.h +++ b/src/network/access/http2/http2protocol_p.h @@ -132,7 +132,7 @@ enum class FrameStatus goodFrame }; -enum Http2Error +enum Http2Error : quint32 { // Old-style enum to avoid excessive name // qualification ... diff --git a/src/network/access/qhttp2connection.cpp b/src/network/access/qhttp2connection.cpp index 258aeb43623..eb3f9d9d8b4 100644 --- a/src/network/access/qhttp2connection.cpp +++ b/src/network/access/qhttp2connection.cpp @@ -74,7 +74,7 @@ QHttp2Stream::~QHttp2Stream() noexcept = default; */ /*! - \fn void QHttp2Stream::errorOccurred(quint32 errorCode, const QString &errorString) + \fn void QHttp2Stream::errorOccurred(Http2::Http2Error errorCode, const QString &errorString) This signal is emitted when the stream has encountered an error. The \a errorCode parameter is the HTTP/2 error code, and the \a errorString @@ -196,7 +196,7 @@ QHttp2Stream::~QHttp2Stream() noexcept = default; Returns the buffer containing the data received from the remote peer. */ -void QHttp2Stream::finishWithError(quint32 errorCode, const QString &message) +void QHttp2Stream::finishWithError(Http2::Http2Error errorCode, const QString &message) { qCDebug(qHttp2ConnectionLog, "[%p] stream %u finished with error: %ls (error code: %u)", getConnection(), m_streamID, qUtf16Printable(message), errorCode); @@ -204,12 +204,12 @@ void QHttp2Stream::finishWithError(quint32 errorCode, const QString &message) emit errorOccurred(errorCode, message); } -void QHttp2Stream::finishWithError(quint32 errorCode) +void QHttp2Stream::finishWithError(Http2::Http2Error errorCode) { - QNetworkReply::NetworkError error = QNetworkReply::NoError; + QNetworkReply::NetworkError ignored = QNetworkReply::NoError; QString message; - qt_error(errorCode, error, message); - finishWithError(error, message); + qt_error(errorCode, ignored, message); + finishWithError(errorCode, message); } /*! @@ -219,7 +219,7 @@ void QHttp2Stream::finishWithError(quint32 errorCode) Returns \c false if the stream is closed or idle, also if it fails to send the RST_STREAM frame. Otherwise, returns \c true. */ -bool QHttp2Stream::sendRST_STREAM(quint32 errorCode) +bool QHttp2Stream::sendRST_STREAM(Http2::Http2Error errorCode) { if (m_state == State::Closed || m_state == State::Idle) return false; @@ -230,7 +230,7 @@ bool QHttp2Stream::sendRST_STREAM(quint32 errorCode) QHttp2Connection *connection = getConnection(); FrameWriter &frameWriter = connection->frameWriter; frameWriter.start(FrameType::RST_STREAM, FrameFlag::EMPTY, m_streamID); - frameWriter.append(errorCode); + frameWriter.append(quint32(errorCode)); return frameWriter.write(*connection->getSocket()); } @@ -358,7 +358,7 @@ void QHttp2Stream::internalSendDATA() if (!frameWriter.write(*socket)) { qCDebug(qHttp2ConnectionLog, "[%p] stream %u, failed to write to socket", connection, m_streamID); - finishWithError(QNetworkReply::ProtocolFailure, "failed to write to socket"_L1); + finishWithError(INTERNAL_ERROR, "failed to write to socket"_L1); return; } @@ -575,7 +575,7 @@ void QHttp2Stream::handleDATA(const Frame &inboundFrame) "[%p] stream %u, received DATA frame with payload size %u, " "but recvWindow is %d, sending FLOW_CONTROL_ERROR", connection, m_streamID, inboundFrame.payloadSize(), m_recvWindow); - finishWithError(QNetworkReply::ProtocolFailure, "flow control error"_L1); + finishWithError(FLOW_CONTROL_ERROR, "flow control error"_L1); sendRST_STREAM(FLOW_CONTROL_ERROR); return; } @@ -620,7 +620,7 @@ void QHttp2Stream::handleRST_STREAM(const Frame &inboundFrame) m_uploadDevice = nullptr; m_uploadByteDevice = nullptr; } - finishWithError(*m_RST_STREAM_code, ""_L1); + finishWithError(Http2Error(*m_RST_STREAM_code)); } void QHttp2Stream::handleWINDOW_UPDATE(const Frame &inboundFrame) @@ -633,7 +633,7 @@ void QHttp2Stream::handleWINDOW_UPDATE(const Frame &inboundFrame) "[%p] stream %u, received WINDOW_UPDATE frame with invalid delta %u, sending " "PROTOCOL_ERROR", getConnection(), m_streamID, delta); - finishWithError(QNetworkReply::ProtocolFailure, "invalid WINDOW_UPDATE delta"_L1); + finishWithError(PROTOCOL_ERROR, "invalid WINDOW_UPDATE delta"_L1); sendRST_STREAM(PROTOCOL_ERROR); return; } @@ -701,7 +701,7 @@ void QHttp2Stream::handleWINDOW_UPDATE(const Frame &inboundFrame) */ /*! - \fn void QHttp2Connection::receivedGOAWAY(quint32 errorCode, quint32 lastStreamID) + \fn void QHttp2Connection::receivedGOAWAY(Http2::Http2Error errorCode, quint32 lastStreamID) This signal is emitted when the connection has received a GOAWAY frame. The \a errorCode parameter is the HTTP/2 error code, and the \a lastStreamID @@ -935,7 +935,7 @@ bool QHttp2Connection::serverCheckClientPreface() qCDebug(qHttp2ConnectionLog, "[%p] Peer sent valid client preface", this); m_waitingForClientPreface = false; if (!sendServerPreface()) { - connectionError(Http2::INTERNAL_ERROR, "Failed to send server preface"); + connectionError(INTERNAL_ERROR, "Failed to send server preface"); return false; } return true; @@ -1075,7 +1075,7 @@ void QHttp2Connection::handleConnectionClosure() for (auto it = m_streams.begin(), end = m_streams.end(); it != end; ++it) { auto stream = it.value(); if (stream && stream->isActive()) - stream->finishWithError(QNetworkReply::RemoteHostClosedError, errorString); + stream->finishWithError(PROTOCOL_ERROR, errorString); } } @@ -1101,12 +1101,11 @@ void QHttp2Connection::connectionError(Http2Error errorCode, const char *message m_goingAway = true; sendGOAWAY(errorCode); - const auto error = qt_error(errorCode); auto messageView = QLatin1StringView(message); for (QHttp2Stream *stream : std::as_const(m_streams)) { if (stream && stream->isActive()) - stream->finishWithError(error, messageView); + stream->finishWithError(errorCode, messageView); } closeSession(); @@ -1186,12 +1185,12 @@ bool QHttp2Connection::sendWINDOW_UPDATE(quint32 streamID, quint32 delta) return frameWriter.write(*getSocket()); } -bool QHttp2Connection::sendGOAWAY(quint32 errorCode) +bool QHttp2Connection::sendGOAWAY(Http2::Http2Error errorCode) { frameWriter.start(FrameType::GOAWAY, FrameFlag::EMPTY, Http2PredefinedParameters::connectionStreamID); frameWriter.append(quint32(m_lastIncomingStreamID)); - frameWriter.append(errorCode); + frameWriter.append(quint32(errorCode)); return frameWriter.write(*getSocket()); } @@ -1481,7 +1480,7 @@ void QHttp2Connection::handleGOAWAY() const uchar *const src = inboundFrame.dataBegin(); quint32 lastStreamID = qFromBigEndian(src); - const quint32 errorCode = qFromBigEndian(src + 4); + const Http2Error errorCode = Http2Error(qFromBigEndian(src + 4)); if (!lastStreamID) { // "The last stream identifier can be set to 0 if no @@ -1595,7 +1594,7 @@ void QHttp2Connection::handleContinuedHEADERS() // We can receive HEADERS on streams initiated by our requests // (these streams are in halfClosedLocal or open state) or // remote-reserved streams from a server's PUSH_PROMISE. - stream->finishWithError(QNetworkReply::ProtocolFailure, + stream->finishWithError(PROTOCOL_ERROR, "HEADERS on invalid stream"_L1); stream->sendRST_STREAM(CANCEL); return; @@ -1694,7 +1693,7 @@ bool QHttp2Connection::acceptSetting(Http2::Settings identifier, quint32 newValu qint32 sum = 0; if (qAddOverflow(stream->m_sendWindow, delta, &sum)) { stream->sendRST_STREAM(PROTOCOL_ERROR); - stream->finishWithError(QNetworkReply::ProtocolFailure, + stream->finishWithError(PROTOCOL_ERROR, "SETTINGS window overflow"_L1); continue; } diff --git a/src/network/access/qhttp2connection_p.h b/src/network/access/qhttp2connection_p.h index e524c791a9c..028860c0d68 100644 --- a/src/network/access/qhttp2connection_p.h +++ b/src/network/access/qhttp2connection_p.h @@ -120,7 +120,7 @@ public: Q_SIGNALS: void headersReceived(const HPack::HttpHeader &headers, bool endStream); void headersUpdated(); - void errorOccurred(quint32 errorCode, const QString &errorString); + void errorOccurred(Http2::Http2Error errorCode, const QString &errorString); void stateChanged(QHttp2Stream::State newState); void promisedStreamReceived(quint32 newStreamID); void uploadBlocked(); @@ -131,7 +131,7 @@ Q_SIGNALS: void uploadFinished(); public Q_SLOTS: - bool sendRST_STREAM(quint32 errorCode); + bool sendRST_STREAM(Http2::Http2Error errorCode); bool sendHEADERS(const HPack::HttpHeader &headers, bool endStream, quint8 priority = DefaultPriority); void sendDATA(QIODevice *device, bool endStream); @@ -169,8 +169,8 @@ private: void handleRST_STREAM(const Http2::Frame &inboundFrame); void handleWINDOW_UPDATE(const Http2::Frame &inboundFrame); - void finishWithError(quint32 errorCode, const QString &message); - void finishWithError(quint32 errorCode); + void finishWithError(Http2::Http2Error errorCode, const QString &message); + void finishWithError(Http2::Http2Error errorCode); // Keep it const since it never changes after creation const quint32 m_streamID = 0; @@ -245,7 +245,7 @@ Q_SIGNALS: void settingsFrameReceived(); void pingFrameRecived(QHttp2Connection::PingState state); void errorOccurred(Http2::Http2Error errorCode, const QString &errorString); - void receivedGOAWAY(quint32 errorCode, quint32 lastStreamID); + void receivedGOAWAY(Http2::Http2Error errorCode, quint32 lastStreamID); public Q_SLOTS: bool sendPing(); bool sendPing(QByteArrayView data); @@ -275,7 +275,7 @@ private: bool sendServerPreface(); bool serverCheckClientPreface(); bool sendWINDOW_UPDATE(quint32 streamID, quint32 delta); - bool sendGOAWAY(quint32 errorCode); + bool sendGOAWAY(Http2::Http2Error errorCode); bool sendSETTINGS_ACK(); void handleDATA();