parent
c0fbe79b87
commit
de132f808b
@ -0,0 +1,38 @@
|
|||||||
|
From ea63c28efc1d2ecb467b83a34923d12462efa96f Mon Sep 17 00:00:00 2001
|
||||||
|
From: Marc Mutz <marc.mutz@qt.io>
|
||||||
|
Date: Tue, 12 Dec 2023 20:51:56 +0100
|
||||||
|
Subject: [PATCH] HPack: fix a Yoda Condition
|
||||||
|
|
||||||
|
Putting the variable on the LHS of a relational operation makes the
|
||||||
|
expression easier to read. In this case, we find that the whole
|
||||||
|
expression is nonsensical as an overflow protection, because if
|
||||||
|
name.size() + value.size() overflows, the result will exactly _not_
|
||||||
|
be > max() - 32, because UB will have happened.
|
||||||
|
|
||||||
|
To be fixed in a follow-up commit.
|
||||||
|
|
||||||
|
As a drive-by, add parentheses around the RHS.
|
||||||
|
|
||||||
|
Change-Id: I35ce598884c37c51b74756b3bd2734b9aad63c09
|
||||||
|
Reviewed-by: Allan Sandfeld Jensen <allan.jensen@qt.io>
|
||||||
|
(cherry picked from commit 658607a34ead214fbacbc2cca44915655c318ea9)
|
||||||
|
Reviewed-by: Qt Cherry-pick Bot <cherrypick_bot@qt-project.org>
|
||||||
|
(cherry picked from commit 4f7efd41740107f90960116700e3134f5e433867)
|
||||||
|
(cherry picked from commit 13c16b756900fe524f6d9534e8a07aa003c05e0c)
|
||||||
|
(cherry picked from commit 1d4788a39668fb2dc5912a8d9c4272dc40e99f92)
|
||||||
|
(cherry picked from commit 87de75b5cc946d196decaa6aef4792a6cac0b6db)
|
||||||
|
---
|
||||||
|
|
||||||
|
diff --git a/src/network/access/http2/hpacktable.cpp b/src/network/access/http2/hpacktable.cpp
|
||||||
|
index 834214f..ab166a6 100644
|
||||||
|
--- a/src/network/access/http2/hpacktable.cpp
|
||||||
|
+++ b/src/network/access/http2/hpacktable.cpp
|
||||||
|
@@ -63,7 +63,7 @@
|
||||||
|
// 32 octets of overhead."
|
||||||
|
|
||||||
|
const unsigned sum = unsigned(name.size() + value.size());
|
||||||
|
- if (std::numeric_limits<unsigned>::max() - 32 < sum)
|
||||||
|
+ if (sum > (std::numeric_limits<unsigned>::max() - 32))
|
||||||
|
return HeaderSize();
|
||||||
|
return HeaderSize(true, quint32(sum + 32));
|
||||||
|
}
|
@ -0,0 +1,59 @@
|
|||||||
|
From 23c3fc483e8b6e21012a61f0bea884446f727776 Mon Sep 17 00:00:00 2001
|
||||||
|
From: Marc Mutz <marc.mutz@qt.io>
|
||||||
|
Date: Tue, 12 Dec 2023 22:08:07 +0100
|
||||||
|
Subject: [PATCH] HPack: fix incorrect integer overflow check
|
||||||
|
|
||||||
|
This code never worked:
|
||||||
|
|
||||||
|
For the comparison with max() - 32 to trigger, on 32-bit platforms (or
|
||||||
|
Qt 5) signed interger overflow would have had to happen in the
|
||||||
|
addition of the two sizes. The compiler can therefore remove the
|
||||||
|
overflow check as dead code.
|
||||||
|
|
||||||
|
On Qt 6 and 64-bit platforms, the signed integer addition would be
|
||||||
|
very unlikely to overflow, but the following truncation to uint32
|
||||||
|
would yield the correct result only in a narrow 32-value window just
|
||||||
|
below UINT_MAX, if even that.
|
||||||
|
|
||||||
|
Fix by using the proper tool, qAddOverflow.
|
||||||
|
|
||||||
|
Manual conflict resolutions:
|
||||||
|
- qAddOverflow doesn't exist in Qt 5, use private add_overflow
|
||||||
|
predecessor API instead
|
||||||
|
|
||||||
|
Change-Id: I7599f2e75ff7f488077b0c60b81022591005661c
|
||||||
|
Reviewed-by: Allan Sandfeld Jensen <allan.jensen@qt.io>
|
||||||
|
(cherry picked from commit ee5da1f2eaf8932aeca02ffea6e4c618585e29e3)
|
||||||
|
Reviewed-by: Qt Cherry-pick Bot <cherrypick_bot@qt-project.org>
|
||||||
|
(cherry picked from commit debeb8878da2dc706ead04b6072ecbe7e5313860)
|
||||||
|
Reviewed-by: Thiago Macieira <thiago.macieira@intel.com>
|
||||||
|
Reviewed-by: Marc Mutz <marc.mutz@qt.io>
|
||||||
|
(cherry picked from commit 811b9eef6d08d929af8708adbf2a5effb0eb62d7)
|
||||||
|
(cherry picked from commit f931facd077ce945f1e42eaa3bead208822d3e00)
|
||||||
|
(cherry picked from commit 9ef4ca5ecfed771dab890856130e93ef5ceabef5)
|
||||||
|
Reviewed-by: MÃ¥rten Nordheim <marten.nordheim@qt.io>
|
||||||
|
---
|
||||||
|
|
||||||
|
diff --git a/src/network/access/http2/hpacktable.cpp b/src/network/access/http2/hpacktable.cpp
|
||||||
|
index ab166a6..de91fc0 100644
|
||||||
|
--- a/src/network/access/http2/hpacktable.cpp
|
||||||
|
+++ b/src/network/access/http2/hpacktable.cpp
|
||||||
|
@@ -40,6 +40,7 @@
|
||||||
|
#include "hpacktable_p.h"
|
||||||
|
|
||||||
|
#include <QtCore/qdebug.h>
|
||||||
|
+#include <QtCore/private/qnumeric_p.h>
|
||||||
|
|
||||||
|
#include <algorithm>
|
||||||
|
#include <cstddef>
|
||||||
|
@@ -62,7 +63,9 @@
|
||||||
|
// for counting the number of references to the name and value would have
|
||||||
|
// 32 octets of overhead."
|
||||||
|
|
||||||
|
- const unsigned sum = unsigned(name.size() + value.size());
|
||||||
|
+ size_t sum;
|
||||||
|
+ if (add_overflow(size_t(name.size()), size_t(value.size()), &sum))
|
||||||
|
+ return HeaderSize();
|
||||||
|
if (sum > (std::numeric_limits<unsigned>::max() - 32))
|
||||||
|
return HeaderSize();
|
||||||
|
return HeaderSize(true, quint32(sum + 32));
|
@ -0,0 +1,197 @@
|
|||||||
|
diff --git a/src/gui/util/qktxhandler.cpp b/src/gui/util/qktxhandler.cpp
|
||||||
|
index 0d98e97453..6a79e55109 100644
|
||||||
|
--- a/src/gui/util/qktxhandler.cpp
|
||||||
|
+++ b/src/gui/util/qktxhandler.cpp
|
||||||
|
@@ -73,7 +73,7 @@ struct KTXHeader {
|
||||||
|
quint32 bytesOfKeyValueData;
|
||||||
|
};
|
||||||
|
|
||||||
|
-static const quint32 headerSize = sizeof(KTXHeader);
|
||||||
|
+static constexpr quint32 qktxh_headerSize = sizeof(KTXHeader);
|
||||||
|
|
||||||
|
// Currently unused, declared for future reference
|
||||||
|
struct KTXKeyValuePairItem {
|
||||||
|
@@ -103,11 +103,36 @@ struct KTXMipmapLevel {
|
||||||
|
*/
|
||||||
|
};
|
||||||
|
|
||||||
|
-bool QKtxHandler::canRead(const QByteArray &suffix, const QByteArray &block)
|
||||||
|
+static bool qAddOverflow(quint32 v1, quint32 v2, quint32 *r) {
|
||||||
|
+ // unsigned additions are well-defined
|
||||||
|
+ *r = v1 + v2;
|
||||||
|
+ return v1 > quint32(v1 + v2);
|
||||||
|
+}
|
||||||
|
+
|
||||||
|
+// Returns the nearest multiple of 4 greater than or equal to 'value'
|
||||||
|
+static bool nearestMultipleOf4(quint32 value, quint32 *result)
|
||||||
|
+{
|
||||||
|
+ constexpr quint32 rounding = 4;
|
||||||
|
+ *result = 0;
|
||||||
|
+ if (qAddOverflow(value, rounding - 1, result))
|
||||||
|
+ return true;
|
||||||
|
+ *result &= ~(rounding - 1);
|
||||||
|
+ return false;
|
||||||
|
+}
|
||||||
|
+
|
||||||
|
+// Returns a slice with prechecked bounds
|
||||||
|
+static QByteArray safeSlice(const QByteArray& array, quint32 start, quint32 length)
|
||||||
|
{
|
||||||
|
- Q_UNUSED(suffix)
|
||||||
|
+ quint32 end = 0;
|
||||||
|
+ if (qAddOverflow(start, length, &end) || end > quint32(array.length()))
|
||||||
|
+ return {};
|
||||||
|
+ return QByteArray(array.data() + start, length);
|
||||||
|
+}
|
||||||
|
|
||||||
|
- return (qstrncmp(block.constData(), ktxIdentifier, KTX_IDENTIFIER_LENGTH) == 0);
|
||||||
|
+bool QKtxHandler::canRead(const QByteArray &suffix, const QByteArray &block)
|
||||||
|
+{
|
||||||
|
+ Q_UNUSED(suffix);
|
||||||
|
+ return block.startsWith(QByteArray::fromRawData(ktxIdentifier, KTX_IDENTIFIER_LENGTH));
|
||||||
|
}
|
||||||
|
|
||||||
|
QTextureFileData QKtxHandler::read()
|
||||||
|
@@ -115,42 +140,97 @@ QTextureFileData QKtxHandler::read()
|
||||||
|
if (!device())
|
||||||
|
return QTextureFileData();
|
||||||
|
|
||||||
|
- QByteArray buf = device()->readAll();
|
||||||
|
- const quint32 dataSize = quint32(buf.size());
|
||||||
|
- if (dataSize < headerSize || !canRead(QByteArray(), buf)) {
|
||||||
|
- qCDebug(lcQtGuiTextureIO, "Invalid KTX file %s", logName().constData());
|
||||||
|
+ const QByteArray buf = device()->readAll();
|
||||||
|
+ if (size_t(buf.size()) > std::numeric_limits<quint32>::max()) {
|
||||||
|
+ qWarning(lcQtGuiTextureIO, "Too big KTX file %s", logName().constData());
|
||||||
|
+ return QTextureFileData();
|
||||||
|
+ }
|
||||||
|
+
|
||||||
|
+ if (!canRead(QByteArray(), buf)) {
|
||||||
|
+ qWarning(lcQtGuiTextureIO, "Invalid KTX file %s", logName().constData());
|
||||||
|
+ return QTextureFileData();
|
||||||
|
+ }
|
||||||
|
+
|
||||||
|
+ if (buf.size() < qsizetype(qktxh_headerSize)) {
|
||||||
|
+ qWarning(lcQtGuiTextureIO, "Invalid KTX header size in %s", logName().constData());
|
||||||
|
return QTextureFileData();
|
||||||
|
}
|
||||||
|
|
||||||
|
- const KTXHeader *header = reinterpret_cast<const KTXHeader *>(buf.constData());
|
||||||
|
- if (!checkHeader(*header)) {
|
||||||
|
- qCDebug(lcQtGuiTextureIO, "Unsupported KTX file format in %s", logName().constData());
|
||||||
|
+ KTXHeader header;
|
||||||
|
+ memcpy(&header, buf.data(), qktxh_headerSize);
|
||||||
|
+ if (!checkHeader(header)) {
|
||||||
|
+ qWarning(lcQtGuiTextureIO, "Unsupported KTX file format in %s", logName().constData());
|
||||||
|
return QTextureFileData();
|
||||||
|
}
|
||||||
|
|
||||||
|
QTextureFileData texData;
|
||||||
|
texData.setData(buf);
|
||||||
|
|
||||||
|
- texData.setSize(QSize(decode(header->pixelWidth), decode(header->pixelHeight)));
|
||||||
|
- texData.setGLFormat(decode(header->glFormat));
|
||||||
|
- texData.setGLInternalFormat(decode(header->glInternalFormat));
|
||||||
|
- texData.setGLBaseInternalFormat(decode(header->glBaseInternalFormat));
|
||||||
|
-
|
||||||
|
- texData.setNumLevels(decode(header->numberOfMipmapLevels));
|
||||||
|
- quint32 offset = headerSize + decode(header->bytesOfKeyValueData);
|
||||||
|
- const int maxLevels = qMin(texData.numLevels(), 32); // Cap iterations in case of corrupt file.
|
||||||
|
- for (int i = 0; i < maxLevels; i++) {
|
||||||
|
- if (offset + sizeof(KTXMipmapLevel) > dataSize) // Corrupt file; avoid oob read
|
||||||
|
- break;
|
||||||
|
- const KTXMipmapLevel *level = reinterpret_cast<const KTXMipmapLevel *>(buf.constData() + offset);
|
||||||
|
- quint32 levelLen = decode(level->imageSize);
|
||||||
|
- texData.setDataOffset(offset + sizeof(KTXMipmapLevel::imageSize), i);
|
||||||
|
- texData.setDataLength(levelLen, i);
|
||||||
|
- offset += sizeof(KTXMipmapLevel::imageSize) + levelLen + (3 - ((levelLen + 3) % 4));
|
||||||
|
+ texData.setSize(QSize(decode(header.pixelWidth), decode(header.pixelHeight)));
|
||||||
|
+ texData.setGLFormat(decode(header.glFormat));
|
||||||
|
+ texData.setGLInternalFormat(decode(header.glInternalFormat));
|
||||||
|
+ texData.setGLBaseInternalFormat(decode(header.glBaseInternalFormat));
|
||||||
|
+
|
||||||
|
+ texData.setNumLevels(decode(header.numberOfMipmapLevels));
|
||||||
|
+
|
||||||
|
+ const quint32 bytesOfKeyValueData = decode(header.bytesOfKeyValueData);
|
||||||
|
+ quint32 headerKeyValueSize;
|
||||||
|
+ if (qAddOverflow(qktxh_headerSize, bytesOfKeyValueData, &headerKeyValueSize)) {
|
||||||
|
+ qWarning(lcQtGuiTextureIO, "Overflow in size of key value data in header of KTX file %s",
|
||||||
|
+ logName().constData());
|
||||||
|
+ return QTextureFileData();
|
||||||
|
+ }
|
||||||
|
+
|
||||||
|
+ if (headerKeyValueSize >= quint32(buf.size())) {
|
||||||
|
+ qWarning(lcQtGuiTextureIO, "OOB request in KTX file %s", logName().constData());
|
||||||
|
+ return QTextureFileData();
|
||||||
|
+ }
|
||||||
|
+
|
||||||
|
+ // Technically, any number of levels is allowed but if the value is bigger than
|
||||||
|
+ // what is possible in KTX V2 (and what makes sense) we return an error.
|
||||||
|
+ // maxLevels = log2(max(width, height, depth))
|
||||||
|
+ const int maxLevels = (sizeof(quint32) * 8)
|
||||||
|
+ - qCountLeadingZeroBits(std::max(
|
||||||
|
+ { header.pixelWidth, header.pixelHeight, header.pixelDepth }));
|
||||||
|
+
|
||||||
|
+ if (texData.numLevels() > maxLevels) {
|
||||||
|
+ qWarning(lcQtGuiTextureIO, "Too many levels in KTX file %s", logName().constData());
|
||||||
|
+ return QTextureFileData();
|
||||||
|
+ }
|
||||||
|
+
|
||||||
|
+ quint32 offset = headerKeyValueSize;
|
||||||
|
+ for (int level = 0; level < texData.numLevels(); level++) {
|
||||||
|
+ const auto imageSizeSlice = safeSlice(buf, offset, sizeof(quint32));
|
||||||
|
+ if (imageSizeSlice.isEmpty()) {
|
||||||
|
+ qWarning(lcQtGuiTextureIO, "OOB request in KTX file %s", logName().constData());
|
||||||
|
+ return QTextureFileData();
|
||||||
|
+ }
|
||||||
|
+
|
||||||
|
+ const quint32 imageSize = decode(qFromUnaligned<quint32>(imageSizeSlice.data()));
|
||||||
|
+ offset += sizeof(quint32); // overflow checked indirectly above
|
||||||
|
+
|
||||||
|
+ texData.setDataOffset(offset, level);
|
||||||
|
+ texData.setDataLength(imageSize, level);
|
||||||
|
+
|
||||||
|
+ // Add image data and padding to offset
|
||||||
|
+ quint32 padded = 0;
|
||||||
|
+ if (nearestMultipleOf4(imageSize, &padded)) {
|
||||||
|
+ qWarning(lcQtGuiTextureIO, "Overflow in KTX file %s", logName().constData());
|
||||||
|
+ return QTextureFileData();
|
||||||
|
+ }
|
||||||
|
+
|
||||||
|
+ quint32 offsetNext;
|
||||||
|
+ if (qAddOverflow(offset, padded, &offsetNext)) {
|
||||||
|
+ qWarning(lcQtGuiTextureIO, "OOB request in KTX file %s", logName().constData());
|
||||||
|
+ return QTextureFileData();
|
||||||
|
+ }
|
||||||
|
+
|
||||||
|
+ offset = offsetNext;
|
||||||
|
}
|
||||||
|
|
||||||
|
if (!texData.isValid()) {
|
||||||
|
- qCDebug(lcQtGuiTextureIO, "Invalid values in header of KTX file %s", logName().constData());
|
||||||
|
+ qWarning(lcQtGuiTextureIO, "Invalid values in header of KTX file %s",
|
||||||
|
+ logName().constData());
|
||||||
|
return QTextureFileData();
|
||||||
|
}
|
||||||
|
|
||||||
|
@@ -191,7 +271,7 @@ bool QKtxHandler::checkHeader(const KTXHeader &header)
|
||||||
|
(decode(header.numberOfFaces) == 1));
|
||||||
|
}
|
||||||
|
|
||||||
|
-quint32 QKtxHandler::decode(quint32 val)
|
||||||
|
+quint32 QKtxHandler::decode(quint32 val) const
|
||||||
|
{
|
||||||
|
return inverseEndian ? qbswap<quint32>(val) : val;
|
||||||
|
}
|
||||||
|
diff --git a/src/gui/util/qktxhandler_p.h b/src/gui/util/qktxhandler_p.h
|
||||||
|
index f831e59d95..cdf1b2eaf8 100644
|
||||||
|
--- a/src/gui/util/qktxhandler_p.h
|
||||||
|
+++ b/src/gui/util/qktxhandler_p.h
|
||||||
|
@@ -68,7 +68,7 @@ public:
|
||||||
|
|
||||||
|
private:
|
||||||
|
bool checkHeader(const KTXHeader &header);
|
||||||
|
- quint32 decode(quint32 val);
|
||||||
|
+ quint32 decode(quint32 val) const;
|
||||||
|
|
||||||
|
bool inverseEndian = false;
|
||||||
|
};
|
@ -0,0 +1,168 @@
|
|||||||
|
From b1e75376cc3adfc7da5502a277dfe9711f3e0536 Mon Sep 17 00:00:00 2001
|
||||||
|
From: Mårten Nordheim <marten.nordheim@qt.io>
|
||||||
|
Date: Tue, 25 Jun 2024 17:09:35 +0200
|
||||||
|
Subject: [PATCH] HTTP2: Delay any communication until encrypted() can be responded to
|
||||||
|
|
||||||
|
We have the encrypted() signal that lets users do extra checks on the
|
||||||
|
established connection. It is emitted as BlockingQueued, so the HTTP
|
||||||
|
thread stalls until it is done emitting. Users can potentially call
|
||||||
|
abort() on the QNetworkReply at that point, which is passed as a Queued
|
||||||
|
call back to the HTTP thread. That means that any currently queued
|
||||||
|
signal emission will be processed before the abort() call is processed.
|
||||||
|
|
||||||
|
In the case of HTTP2 it is a little special since it is multiplexed and
|
||||||
|
the code is built to start requests as they are available. This means
|
||||||
|
that, while the code worked fine for HTTP1, since one connection only
|
||||||
|
has one request, it is not working for HTTP2, since we try to send more
|
||||||
|
requests in-between the encrypted() signal and the abort() call.
|
||||||
|
|
||||||
|
This patch changes the code to delay any communication until the
|
||||||
|
encrypted() signal has been emitted and processed, for HTTP2 only.
|
||||||
|
It's done by adding a few booleans, both to know that we have to return
|
||||||
|
early and so we can keep track of what events arose and what we need to
|
||||||
|
resume once enough time has passed that any abort() call must have been
|
||||||
|
processed.
|
||||||
|
|
||||||
|
Fixes: QTBUG-126610
|
||||||
|
Pick-to: 6.8 6.7 6.5 6.2 5.15 5.12
|
||||||
|
Change-Id: Ic25a600c278203256e35f541026f34a8783235ae
|
||||||
|
Reviewed-by: Marc Mutz <marc.mutz@qt.io>
|
||||||
|
Reviewed-by: Volker Hilsheimer <volker.hilsheimer@qt.io>
|
||||||
|
---
|
||||||
|
|
||||||
|
diff --git a/src/network/access/qhttp2protocolhandler.cpp b/src/network/access/qhttp2protocolhandler.cpp
|
||||||
|
index 91c41d82..562dfd66 100644
|
||||||
|
--- a/src/network/access/qhttp2protocolhandler.cpp
|
||||||
|
+++ b/src/network/access/qhttp2protocolhandler.cpp
|
||||||
|
@@ -371,12 +371,12 @@ bool QHttp2ProtocolHandler::sendRequest()
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
- if (!prefaceSent && !sendClientPreface())
|
||||||
|
- return false;
|
||||||
|
-
|
||||||
|
if (!requests.size())
|
||||||
|
return true;
|
||||||
|
|
||||||
|
+ if (!prefaceSent && !sendClientPreface())
|
||||||
|
+ return false;
|
||||||
|
+
|
||||||
|
m_channel->state = QHttpNetworkConnectionChannel::WritingState;
|
||||||
|
// Check what was promised/pushed, maybe we do not have to send a request
|
||||||
|
// and have a response already?
|
||||||
|
diff --git a/src/network/access/qhttpnetworkconnectionchannel.cpp b/src/network/access/qhttpnetworkconnectionchannel.cpp
|
||||||
|
index 647ef431..608e7914 100644
|
||||||
|
--- a/src/network/access/qhttpnetworkconnectionchannel.cpp
|
||||||
|
+++ b/src/network/access/qhttpnetworkconnectionchannel.cpp
|
||||||
|
@@ -255,6 +255,10 @@ void QHttpNetworkConnectionChannel::abort()
|
||||||
|
bool QHttpNetworkConnectionChannel::sendRequest()
|
||||||
|
{
|
||||||
|
Q_ASSERT(!protocolHandler.isNull());
|
||||||
|
+ if (waitingForPotentialAbort) {
|
||||||
|
+ needInvokeSendRequest = true;
|
||||||
|
+ return false; // this return value is unused
|
||||||
|
+ }
|
||||||
|
return protocolHandler->sendRequest();
|
||||||
|
}
|
||||||
|
|
||||||
|
@@ -267,21 +271,28 @@ bool QHttpNetworkConnectionChannel::sendRequest()
|
||||||
|
void QHttpNetworkConnectionChannel::sendRequestDelayed()
|
||||||
|
{
|
||||||
|
QMetaObject::invokeMethod(this, [this] {
|
||||||
|
- Q_ASSERT(!protocolHandler.isNull());
|
||||||
|
if (reply)
|
||||||
|
- protocolHandler->sendRequest();
|
||||||
|
+ sendRequest();
|
||||||
|
}, Qt::ConnectionType::QueuedConnection);
|
||||||
|
}
|
||||||
|
|
||||||
|
void QHttpNetworkConnectionChannel::_q_receiveReply()
|
||||||
|
{
|
||||||
|
Q_ASSERT(!protocolHandler.isNull());
|
||||||
|
+ if (waitingForPotentialAbort) {
|
||||||
|
+ needInvokeReceiveReply = true;
|
||||||
|
+ return;
|
||||||
|
+ }
|
||||||
|
protocolHandler->_q_receiveReply();
|
||||||
|
}
|
||||||
|
|
||||||
|
void QHttpNetworkConnectionChannel::_q_readyRead()
|
||||||
|
{
|
||||||
|
Q_ASSERT(!protocolHandler.isNull());
|
||||||
|
+ if (waitingForPotentialAbort) {
|
||||||
|
+ needInvokeReadyRead = true;
|
||||||
|
+ return;
|
||||||
|
+ }
|
||||||
|
protocolHandler->_q_readyRead();
|
||||||
|
}
|
||||||
|
|
||||||
|
@@ -1287,7 +1298,18 @@ void QHttpNetworkConnectionChannel::_q_encrypted()
|
||||||
|
// Similar to HTTP/1.1 counterpart below:
|
||||||
|
const auto &pairs = spdyRequestsToSend.values(); // (request, reply)
|
||||||
|
const auto &pair = pairs.first();
|
||||||
|
+ waitingForPotentialAbort = true;
|
||||||
|
emit pair.second->encrypted();
|
||||||
|
+
|
||||||
|
+ // We don't send or handle any received data until any effects from
|
||||||
|
+ // emitting encrypted() have been processed. This is necessary
|
||||||
|
+ // because the user may have called abort(). We may also abort the
|
||||||
|
+ // whole connection if the request has been aborted and there is
|
||||||
|
+ // no more requests to send.
|
||||||
|
+ QMetaObject::invokeMethod(this,
|
||||||
|
+ &QHttpNetworkConnectionChannel::checkAndResumeCommunication,
|
||||||
|
+ Qt::QueuedConnection);
|
||||||
|
+
|
||||||
|
// In case our peer has sent us its settings (window size, max concurrent streams etc.)
|
||||||
|
// let's give _q_receiveReply a chance to read them first ('invokeMethod', QueuedConnection).
|
||||||
|
QMetaObject::invokeMethod(connection, "_q_startNextRequest", Qt::QueuedConnection);
|
||||||
|
@@ -1305,6 +1327,26 @@ void QHttpNetworkConnectionChannel::_q_encrypted()
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
+void QHttpNetworkConnectionChannel::checkAndResumeCommunication()
|
||||||
|
+{
|
||||||
|
+ Q_ASSERT(connection->connectionType() > QHttpNetworkConnection::ConnectionTypeHTTP);
|
||||||
|
+
|
||||||
|
+ // Because HTTP/2 requires that we send a SETTINGS frame as the first thing we do, and respond
|
||||||
|
+ // to a SETTINGS frame with an ACK, we need to delay any handling until we can ensure that any
|
||||||
|
+ // effects from emitting encrypted() have been processed.
|
||||||
|
+ // This function is called after encrypted() was emitted, so check for changes.
|
||||||
|
+
|
||||||
|
+ if (!reply && spdyRequestsToSend.isEmpty())
|
||||||
|
+ abort();
|
||||||
|
+ waitingForPotentialAbort = false;
|
||||||
|
+ if (needInvokeReadyRead)
|
||||||
|
+ _q_readyRead();
|
||||||
|
+ if (needInvokeReceiveReply)
|
||||||
|
+ _q_receiveReply();
|
||||||
|
+ if (needInvokeSendRequest)
|
||||||
|
+ sendRequest();
|
||||||
|
+}
|
||||||
|
+
|
||||||
|
void QHttpNetworkConnectionChannel::requeueSpdyRequests()
|
||||||
|
{
|
||||||
|
QList<HttpMessagePair> spdyPairs = spdyRequestsToSend.values();
|
||||||
|
diff --git a/src/network/access/qhttpnetworkconnectionchannel_p.h b/src/network/access/qhttpnetworkconnectionchannel_p.h
|
||||||
|
index d8ac3979..eac44464 100644
|
||||||
|
--- a/src/network/access/qhttpnetworkconnectionchannel_p.h
|
||||||
|
+++ b/src/network/access/qhttpnetworkconnectionchannel_p.h
|
||||||
|
@@ -107,6 +107,10 @@ public:
|
||||||
|
QAbstractSocket *socket;
|
||||||
|
bool ssl;
|
||||||
|
bool isInitialized;
|
||||||
|
+ bool waitingForPotentialAbort = false;
|
||||||
|
+ bool needInvokeReceiveReply = false;
|
||||||
|
+ bool needInvokeReadyRead = false;
|
||||||
|
+ bool needInvokeSendRequest = false;
|
||||||
|
ChannelState state;
|
||||||
|
QHttpNetworkRequest request; // current request, only used for HTTP
|
||||||
|
QHttpNetworkReply *reply; // current reply for this request, only used for HTTP
|
||||||
|
@@ -187,6 +191,8 @@ public:
|
||||||
|
void closeAndResendCurrentRequest();
|
||||||
|
void resendCurrentRequest();
|
||||||
|
|
||||||
|
+ void checkAndResumeCommunication();
|
||||||
|
+
|
||||||
|
bool isSocketBusy() const;
|
||||||
|
bool isSocketWriting() const;
|
||||||
|
bool isSocketWaiting() const;
|
@ -0,0 +1,60 @@
|
|||||||
|
From 95064c35826793c5d6a4edff9fa08ad308b047bb Mon Sep 17 00:00:00 2001
|
||||||
|
From: Timur Pocheptsov <timur.pocheptsov@qt.io>
|
||||||
|
Date: Tue, 20 Jul 2021 08:16:28 +0200
|
||||||
|
Subject: [PATCH] H2: emit encrypted for at least the first reply, similar to
|
||||||
|
H1
|
||||||
|
MIME-Version: 1.0
|
||||||
|
Content-Type: text/plain; charset=UTF-8
|
||||||
|
Content-Transfer-Encoding: 8bit
|
||||||
|
|
||||||
|
Fixes: QTBUG-95277
|
||||||
|
Change-Id: I1fe01503376c0d6278e366d7bd31b412b7cc3a69
|
||||||
|
Reviewed-by: Mårten Nordheim <marten.nordheim@qt.io>
|
||||||
|
(cherry picked from commit c23b7886348dc313ccec1a131850a7cce1b429de)
|
||||||
|
---
|
||||||
|
src/network/access/qhttpnetworkconnectionchannel.cpp | 4 ++++
|
||||||
|
tests/auto/network/access/http2/tst_http2.cpp | 9 +++++++++
|
||||||
|
2 files changed, 13 insertions(+)
|
||||||
|
|
||||||
|
diff --git a/src/network/access/qhttpnetworkconnectionchannel.cpp b/src/network/access/qhttpnetworkconnectionchannel.cpp
|
||||||
|
index f1db2744..647ef431 100644
|
||||||
|
--- a/src/network/access/qhttpnetworkconnectionchannel.cpp
|
||||||
|
+++ b/src/network/access/qhttpnetworkconnectionchannel.cpp
|
||||||
|
@@ -1284,6 +1284,10 @@ void QHttpNetworkConnectionChannel::_q_encrypted()
|
||||||
|
connection->connectionType() == QHttpNetworkConnection::ConnectionTypeHTTP2Direct) {
|
||||||
|
// we call setSpdyWasUsed(true) on the replies in the SPDY handler when the request is sent
|
||||||
|
if (spdyRequestsToSend.count() > 0) {
|
||||||
|
+ // Similar to HTTP/1.1 counterpart below:
|
||||||
|
+ const auto &pairs = spdyRequestsToSend.values(); // (request, reply)
|
||||||
|
+ const auto &pair = pairs.first();
|
||||||
|
+ emit pair.second->encrypted();
|
||||||
|
// In case our peer has sent us its settings (window size, max concurrent streams etc.)
|
||||||
|
// let's give _q_receiveReply a chance to read them first ('invokeMethod', QueuedConnection).
|
||||||
|
QMetaObject::invokeMethod(connection, "_q_startNextRequest", Qt::QueuedConnection);
|
||||||
|
diff --git a/tests/auto/network/access/http2/tst_http2.cpp b/tests/auto/network/access/http2/tst_http2.cpp
|
||||||
|
index 6702f25b..95a97902 100644
|
||||||
|
--- a/tests/auto/network/access/http2/tst_http2.cpp
|
||||||
|
+++ b/tests/auto/network/access/http2/tst_http2.cpp
|
||||||
|
@@ -267,6 +267,10 @@ void tst_Http2::singleRequest()
|
||||||
|
request.setAttribute(h2Attribute, QVariant(true));
|
||||||
|
|
||||||
|
auto reply = manager->get(request);
|
||||||
|
+#if QT_CONFIG(ssl)
|
||||||
|
+ QSignalSpy encSpy(reply, &QNetworkReply::encrypted);
|
||||||
|
+#endif // QT_CONFIG(ssl)
|
||||||
|
+
|
||||||
|
connect(reply, &QNetworkReply::finished, this, &tst_Http2::replyFinished);
|
||||||
|
// Since we're using self-signed certificates,
|
||||||
|
// ignore SSL errors:
|
||||||
|
@@ -281,6 +285,11 @@ void tst_Http2::singleRequest()
|
||||||
|
|
||||||
|
QCOMPARE(reply->error(), QNetworkReply::NoError);
|
||||||
|
QVERIFY(reply->isFinished());
|
||||||
|
+
|
||||||
|
+#if QT_CONFIG(ssl)
|
||||||
|
+ if (connectionType == H2Type::h2Alpn || connectionType == H2Type::h2Direct)
|
||||||
|
+ QCOMPARE(encSpy.count(), 1);
|
||||||
|
+#endif // QT_CONFIG(ssl)
|
||||||
|
}
|
||||||
|
|
||||||
|
void tst_Http2::multipleRequests()
|
Loading…
Reference in new issue