QHttp2Connection: Fix handling of MAX_HEADER_TABLE_SIZE setting

We need to, following the RFC, send a Dynamic Table Size Update using
HPack once we have acknowledged the setting as long as the new size is
smaller, or if we decide to use the new, larger size.

It's further complicated by the fact that we need to send this update on
the next 'field block' (anything with headers in it), and we may have
multiple SETTING frames come in, and we need to then acknowledge the
_smallest_ one as well as the _final_ one. This is so the decoder on the
peer's side can know that we have set the smallest size, and trimmed our
tables thusly, before going to the larger size.

This could, for example, be used to clear the table.

Task-number: QTBUG-132277
Pick-to: 6.9 6.8
Change-Id: I2b886e125f2c197bc3d42aa0b2bbd308ed2a687c
Reviewed-by: Timur Pocheptsov <timur.pocheptsov@qt.io>
This commit is contained in:
Mårten Nordheim 2024-12-18 12:53:08 +01:00
parent 1b05fba9ac
commit 57f44d6d88
3 changed files with 137 additions and 1 deletions

View File

@ -552,6 +552,18 @@ bool QHttp2Stream::sendHEADERS(const HPack::HttpHeader &headers, bool endStream,
// Compress in-place:
BitOStream outputStream(frameWriter.outboundFrame().buffer);
// Possibly perform and notify of dynamic table size update:
for (auto &maybePendingTableSizeUpdate : connection->pendingTableSizeUpdates) {
if (!maybePendingTableSizeUpdate)
break; // They are ordered, so if the first one is null, the other one is too.
qCDebug(qHttp2ConnectionLog, "[%p] stream %u, sending dynamic table size update of size %u",
connection, streamID(), *maybePendingTableSizeUpdate);
connection->encoder.setMaxDynamicTableSize(*maybePendingTableSizeUpdate);
connection->encoder.encodeSizeUpdate(outputStream, *maybePendingTableSizeUpdate);
maybePendingTableSizeUpdate.reset();
}
if (connection->m_connectionType == QHttp2Connection::Type::Client) {
if (!connection->encoder.encodeRequest(outputStream, headers))
return false;
@ -1946,7 +1958,22 @@ bool QHttp2Connection::acceptSetting(Http2::Settings identifier, quint32 newValu
connectionError(PROTOCOL_ERROR, "SETTINGS invalid table size");
return false;
}
encoder.setMaxDynamicTableSize(newValue);
if (!pendingTableSizeUpdates[0] && encoder.dynamicTableCapacity() == newValue) {
qCDebug(qHttp2ConnectionLog,
"[%p] Ignoring SETTINGS HEADER_TABLE_SIZE %d (same as current value)", this,
newValue);
break;
}
if (pendingTableSizeUpdates[0].value_or(std::numeric_limits<quint32>::max()) >= newValue) {
pendingTableSizeUpdates[0] = newValue;
pendingTableSizeUpdates[1].reset(); // 0 is the latest _and_ smallest, so we don't need 1
qCDebug(qHttp2ConnectionLog, "[%p] Pending table size update to %u", this, newValue);
} else {
pendingTableSizeUpdates[1] = newValue; // newValue was larger than 0, so it goes to 1
qCDebug(qHttp2ConnectionLog, "[%p] Pending 2nd table size update to %u, smallest is %u",
this, newValue, *pendingTableSizeUpdates[0]);
}
break;
}
case Settings::INITIAL_WINDOW_SIZE_ID: {

View File

@ -329,6 +329,13 @@ private:
HPack::Decoder decoder = HPack::Decoder(HPack::FieldLookupTable::DefaultSize);
HPack::Encoder encoder = HPack::Encoder(HPack::FieldLookupTable::DefaultSize, true);
// If we receive SETTINGS_HEADER_TABLE_SIZE in a SETTINGS frame we have to perform a dynamic
// table size update on the _next_ HEADER block we send.
// Because this only happens on the next block we may have multiple pending updates, so we must
// notify of the _smallest_ one followed by the _final_ one. We keep them sorted in that order.
// @future: keep in mind if we add support for sending PUSH_PROMISE because it is a HEADER block
std::array<std::optional<quint32>, 2> pendingTableSizeUpdates;
QHttp2Configuration m_config;
QHash<quint32, QPointer<QHttp2Stream>> m_streams;
QSet<quint32> m_blockedStreams;

View File

@ -20,6 +20,7 @@ private slots:
void construct();
void constructStream();
void testSETTINGSFrame();
void maxHeaderTableSize();
void testPING();
void testRSTServerSide();
void testRSTClientSide();
@ -270,6 +271,107 @@ void tst_QHttp2Connection::testSETTINGSFrame()
}
}
void tst_QHttp2Connection::maxHeaderTableSize()
{
auto [client, server] = makeFakeConnectedSockets();
auto connection = makeHttp2Connection(client.get(), {}, Client);
auto serverConnection = makeHttp2Connection(server.get(), {}, Server);
QSignalSpy incomingRequestSpy(serverConnection, &QHttp2Connection::newIncomingStream);
QHttp2Stream *clientStream = connection->createStream().unwrap();
QVERIFY(clientStream);
QVERIFY(waitForSettingsExchange(connection, serverConnection));
// Test defaults:
// encoder:
QCOMPARE(connection->encoder.dynamicTableSize(), 0u);
QCOMPARE(connection->encoder.maxDynamicTableCapacity(), 4096u);
QCOMPARE(connection->encoder.dynamicTableCapacity(), 4096u);
QCOMPARE(serverConnection->encoder.dynamicTableSize(), 0u);
QCOMPARE(serverConnection->encoder.maxDynamicTableCapacity(), 4096u);
QCOMPARE(serverConnection->encoder.dynamicTableCapacity(), 4096u);
// decoder:
QCOMPARE(connection->decoder.dynamicTableSize(), 0u);
QCOMPARE(connection->decoder.maxDynamicTableCapacity(), 4096u);
QCOMPARE(connection->decoder.dynamicTableCapacity(), 4096u);
QCOMPARE(serverConnection->decoder.dynamicTableSize(), 0u);
QCOMPARE(serverConnection->decoder.maxDynamicTableCapacity(), 4096u);
QCOMPARE(serverConnection->decoder.dynamicTableCapacity(), 4096u);
// Send a HEADER block with a custom header, to add something to the dynamic table:
HPack::HttpHeader headers = getRequiredHeaders();
headers.emplace_back("x-test", "test");
clientStream->sendHEADERS(headers, true);
QVERIFY(incomingRequestSpy.wait());
// Test that the size has been updated:
// encoder:
QCOMPARE_GT(connection->encoder.dynamicTableSize(), 0u); // now > 0
QCOMPARE(connection->encoder.maxDynamicTableCapacity(), 4096u);
QCOMPARE(connection->encoder.dynamicTableCapacity(), 4096u);
QCOMPARE(serverConnection->encoder.dynamicTableSize(), 0u);
QCOMPARE(serverConnection->encoder.maxDynamicTableCapacity(), 4096u);
QCOMPARE(serverConnection->encoder.dynamicTableCapacity(), 4096u);
// decoder:
QCOMPARE(connection->decoder.dynamicTableSize(), 0u);
QCOMPARE(connection->decoder.maxDynamicTableCapacity(), 4096u);
QCOMPARE(connection->decoder.dynamicTableCapacity(), 4096u);
QCOMPARE_GT(serverConnection->decoder.dynamicTableSize(), 0u); // now > 0
QCOMPARE(serverConnection->decoder.maxDynamicTableCapacity(), 4096u);
QCOMPARE(serverConnection->decoder.dynamicTableCapacity(), 4096u);
const quint32 initialTableSize = connection->encoder.dynamicTableSize();
QCOMPARE(initialTableSize, serverConnection->decoder.dynamicTableSize());
// Notify from server that we want a smaller size, reset to 0 first:
for (quint32 nextSize : {0, 2048}) {
// @note: currently we don't have an API for this so just do it by hand:
serverConnection->waitingForSettingsACK = true;
using namespace Http2;
FrameWriter builder(FrameType::SETTINGS, FrameFlag::EMPTY, connectionStreamID);
builder.append(Settings::HEADER_TABLE_SIZE_ID);
builder.append(nextSize);
builder.write(*server->out);
QVERIFY(QTest::qWaitFor([&]() { return !serverConnection->waitingForSettingsACK; }));
}
// Now we have to send another HEADER block without extra field, so we can see that the size of
// the dynamic table has decreased after we cleared it:
headers = getRequiredHeaders();
QHttp2Stream *clientStream2 = connection->createStream().unwrap();
clientStream2->sendHEADERS(headers, true);
QVERIFY(incomingRequestSpy.wait());
// Test that the size has been updated:
// encoder:
QCOMPARE_LT(connection->encoder.dynamicTableSize(), initialTableSize);
QCOMPARE(connection->encoder.maxDynamicTableCapacity(), 2048u);
QCOMPARE(connection->encoder.dynamicTableCapacity(), 2048u);
QCOMPARE(serverConnection->encoder.dynamicTableSize(), 0u);
QCOMPARE(serverConnection->encoder.maxDynamicTableCapacity(), 4096u);
QCOMPARE(serverConnection->encoder.dynamicTableCapacity(), 4096u);
// decoder:
QCOMPARE(connection->decoder.dynamicTableSize(), 0u);
QCOMPARE(connection->decoder.maxDynamicTableCapacity(), 4096u);
QCOMPARE(connection->decoder.dynamicTableCapacity(), 4096u);
QCOMPARE_LT(serverConnection->decoder.dynamicTableSize(), initialTableSize);
// If we add an API for this then this should also be updated at some stage:
QCOMPARE(serverConnection->decoder.maxDynamicTableCapacity(), 4096u);
QCOMPARE(serverConnection->decoder.dynamicTableCapacity(), 2048u);
quint32 newTableSize = connection->encoder.dynamicTableSize();
QCOMPARE(newTableSize, serverConnection->decoder.dynamicTableSize());
}
void tst_QHttp2Connection::testPING()
{
auto [client, server] = makeFakeConnectedSockets();