From f6039b08b3c97193222e97f4529ff066dc765f43 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?M=C3=A5rten=20Nordheim?= Date: Thu, 18 Jan 2024 12:27:08 +0100 Subject: [PATCH] QH2Connection: Fix issue with unity-build/odr The static function appeared in two places, and in a unity-build this fails quite visibly. Change-Id: I60000d01194a2c79ca9c101f2a6d3f77f469f1a7 Reviewed-by: Alexey Edelev Reviewed-by: Nodir Temirkhodjaev (cherry picked from commit a3a48815cc9430d6f5c736a312ea4ea5c0895205) Reviewed-by: Qt Cherry-pick Bot --- src/network/access/http2/hpack.cpp | 43 +++++++++++++++++++ src/network/access/http2/hpack_p.h | 3 ++ src/network/access/qhttp2connection.cpp | 45 +------------------- src/network/access/qhttp2protocolhandler.cpp | 44 +------------------ 4 files changed, 48 insertions(+), 87 deletions(-) diff --git a/src/network/access/http2/hpack.cpp b/src/network/access/http2/hpack.cpp index a2a06e8cd5e..9e970dda53b 100644 --- a/src/network/access/http2/hpack.cpp +++ b/src/network/access/http2/hpack.cpp @@ -504,6 +504,49 @@ void Decoder::handleStreamError(BitIStream &inputStream) // HTTP2 layer will end with session error/COMPRESSION_ERROR. } +std::optional makePromiseKeyUrl(const HttpHeader &requestHeader) +{ + constexpr QByteArrayView names[] = { ":authority", ":method", ":path", ":scheme" }; + enum PseudoHeaderEnum + { + Authority, + Method, + Path, + Scheme + }; + std::array, std::size(names)> pseudoHeaders{}; + for (const auto &field : requestHeader) { + const auto *it = std::find(std::begin(names), std::end(names), QByteArrayView(field.name)); + if (it != std::end(names)) { + const auto index = std::distance(std::begin(names), it); + if (field.value.isEmpty() || pseudoHeaders.at(index).has_value()) + return {}; + pseudoHeaders[index] = field.value; + } + } + + auto optionalIsSet = [](const auto &x) { return x.has_value(); }; + if (!std::all_of(pseudoHeaders.begin(), pseudoHeaders.end(), optionalIsSet)) { + // All four required, HTTP/2 8.1.2.3. + return {}; + } + + const QByteArrayView method = pseudoHeaders[Method].value(); + if (method.compare("get", Qt::CaseInsensitive) != 0 && + method.compare("head", Qt::CaseInsensitive) != 0) { + return {}; + } + + QUrl url; + url.setScheme(QLatin1StringView(pseudoHeaders[Scheme].value())); + url.setAuthority(QLatin1StringView(pseudoHeaders[Authority].value())); + url.setPath(QLatin1StringView(pseudoHeaders[Path].value())); + + if (!url.isValid()) + return {}; + return url; +} + } QT_END_NAMESPACE diff --git a/src/network/access/http2/hpack_p.h b/src/network/access/http2/hpack_p.h index 75693da73cd..b407b819418 100644 --- a/src/network/access/http2/hpack_p.h +++ b/src/network/access/http2/hpack_p.h @@ -18,8 +18,10 @@ #include "hpacktable_p.h" #include +#include #include +#include QT_BEGIN_NAMESPACE @@ -112,6 +114,7 @@ private: FieldLookupTable lookupTable; }; +std::optional makePromiseKeyUrl(const HttpHeader &requestHeader); } QT_END_NAMESPACE diff --git a/src/network/access/qhttp2connection.cpp b/src/network/access/qhttp2connection.cpp index 1d092f6eb73..d2ac5f02908 100644 --- a/src/network/access/qhttp2connection.cpp +++ b/src/network/access/qhttp2connection.cpp @@ -21,49 +21,6 @@ Q_LOGGING_CATEGORY(qHttp2ConnectionLog, "qt.network.http2.connection", QtCritica using namespace Qt::StringLiterals; using namespace Http2; -static std::optional makeUrl(const HPack::HttpHeader &requestHeader) -{ - constexpr QByteArrayView names[] = { ":authority", ":method", ":path", ":scheme" }; - enum PseudoHeaderEnum - { - Authority, - Method, - Path, - Scheme - }; - std::array, std::size(names)> pseudoHeaders{}; - for (const auto &field : requestHeader) { - const auto *it = std::find(std::begin(names), std::end(names), QByteArrayView(field.name)); - if (it != std::end(names)) { - const auto index = std::distance(std::begin(names), it); - if (field.value.isEmpty() || pseudoHeaders.at(index).has_value()) - return {}; - pseudoHeaders[index] = field.value; - } - } - - auto optionalIsSet = [](const auto &x) { return x.has_value(); }; - if (!std::all_of(pseudoHeaders.begin(), pseudoHeaders.end(), optionalIsSet)) { - // All four required, HTTP/2 8.1.2.3. - return {}; - } - - const QByteArrayView method = pseudoHeaders[Method].value(); - if (method.compare("get", Qt::CaseInsensitive) != 0 && - method.compare("head", Qt::CaseInsensitive) != 0) { - return {}; - } - - QUrl url; - url.setScheme(QLatin1StringView(pseudoHeaders[Scheme].value())); - url.setAuthority(QLatin1StringView(pseudoHeaders[Authority].value())); - url.setPath(QLatin1StringView(pseudoHeaders[Path].value())); - - if (!url.isValid()) - return {}; - return url; -} - QHttp2Stream::QHttp2Stream(QHttp2Connection *connection, quint32 streamID) noexcept : QObject(connection), m_streamID(streamID) { @@ -1297,7 +1254,7 @@ void QHttp2Connection::handleContinuedHEADERS() streamIt.value()->handleHEADERS(continuedFrames[0].flags(), decoder.decodedHeader()); break; case FrameType::PUSH_PROMISE: { - std::optional promiseKey = makeUrl(decoder.decodedHeader()); + std::optional promiseKey = HPack::makePromiseKeyUrl(decoder.decodedHeader()); if (!promiseKey) return; // invalid URL/key ! if (m_promisedStreams.contains(*promiseKey)) diff --git a/src/network/access/qhttp2protocolhandler.cpp b/src/network/access/qhttp2protocolhandler.cpp index 4e7c1e1d61b..60f7ea12a6c 100644 --- a/src/network/access/qhttp2protocolhandler.cpp +++ b/src/network/access/qhttp2protocolhandler.cpp @@ -1468,54 +1468,12 @@ quint32 QHttp2ProtocolHandler::allocateStreamID() return streamID; } -static std::optional makeUrl(const HPack::HttpHeader &requestHeader) -{ - constexpr QByteArrayView names[] = { ":authority", ":method", ":path", ":scheme" }; - enum PseudoHeaderEnum - { - Authority, - Method, - Path, - Scheme - }; - std::array, std::size(names)> pseudoHeaders{}; - for (const auto &field : requestHeader) { - const auto it = std::find(std::begin(names), std::end(names), QByteArrayView(field.name)); - if (it != std::end(names)) { - const auto index = std::distance(std::begin(names), it); - if (field.value.isEmpty() || pseudoHeaders.at(index).has_value()) - return {}; - pseudoHeaders[index] = field.value; - } - } - - if (!std::all_of(pseudoHeaders.begin(), pseudoHeaders.end(), [](const auto &x) { return x.has_value();})) { - // All four required, HTTP/2 8.1.2.3. - return {}; - } - - const QByteArrayView method = pseudoHeaders[Method].value(); - if (method.compare("get", Qt::CaseInsensitive) != 0 && - method.compare("head", Qt::CaseInsensitive) != 0) { - return {}; - } - - QUrl url; - url.setScheme(QLatin1StringView(pseudoHeaders[Scheme].value())); - url.setAuthority(QLatin1StringView(pseudoHeaders[Authority].value())); - url.setPath(QLatin1StringView(pseudoHeaders[Path].value())); - - if (!url.isValid()) - return {}; - return url; -} - bool QHttp2ProtocolHandler::tryReserveStream(const Http2::Frame &pushPromiseFrame, const HPack::HttpHeader &requestHeader) { Q_ASSERT(pushPromiseFrame.type() == FrameType::PUSH_PROMISE); - const auto url = makeUrl(requestHeader); + const auto url = HPack::makePromiseKeyUrl(requestHeader); if (!url.has_value()) return false;