HPack: test and fix Dynamic Table Size Update

Expose some new API to make it easier to test too.
Will be used in follow-up patches for a http2 bugfix.

The setMaxDynamicTableSize() method no longer sets capacity or evicts
entries. This can then be used to set the new maximum size which we will
accept later when we receive a Dynamic Table Size Update.

This flow makes sense with both encoder and decoder. The decoder would
usually be set by us when we send our SETTINGS frame, and then the
Dynamic Table Size Update would be sent by the peer.
For the encoder we would set the maximum size when we receive the
SETTINGS frame, and then we must defer the actual resizing until we
can send the Dynamic Table Size Update ourselves, usually along with the
next HEADERS frame.

Pick-to: 6.9 6.8 6.5
Task-number: QTBUG-132277
Change-Id: I959f5006eb09427d130735efe136da8c04453fa2
Reviewed-by: Timur Pocheptsov <timur.pocheptsov@qt.io>
This commit is contained in:
Mårten Nordheim 2024-12-18 12:38:41 +01:00
parent 8778a49360
commit be477819ac
5 changed files with 71 additions and 3 deletions

View File

@ -111,6 +111,16 @@ quint32 Encoder::dynamicTableSize() const
return lookupTable.dynamicDataSize();
}
quint32 Encoder::dynamicTableCapacity() const
{
return lookupTable.dynamicDataCapacity();
}
quint32 Encoder::maxDynamicTableCapacity() const
{
return lookupTable.maxDynamicDataCapacity();
}
bool Encoder::encodeRequest(BitOStream &outputStream, const HttpHeader &header)
{
if (!header.size()) {
@ -408,6 +418,16 @@ quint32 Decoder::dynamicTableSize() const
return lookupTable.dynamicDataSize();
}
quint32 Decoder::dynamicTableCapacity() const
{
return lookupTable.dynamicDataCapacity();
}
quint32 Decoder::maxDynamicTableCapacity() const
{
return lookupTable.maxDynamicDataCapacity();
}
void Decoder::setMaxDynamicTableSize(quint32 size)
{
// Up to a caller (HTTP2 protocol handler)
@ -491,6 +511,11 @@ bool Decoder::processDecodedField(BitPattern fieldType,
return false;
}
if (lookupTable.maxDynamicDataCapacity() < lookupTable.dynamicDataCapacity()) {
qDebug("about to add a new field, but expected a Dynamic Table Size Update");
return false; // We expected a dynamic table size update.
}
header.push_back(HeaderField(name, value));
return true;
}

View File

@ -38,6 +38,8 @@ public:
Encoder(quint32 maxTableSize, bool compressStrings);
quint32 dynamicTableSize() const;
quint32 dynamicTableCapacity() const;
quint32 maxDynamicTableCapacity() const;
bool encodeRequest(class BitOStream &outputStream,
const HttpHeader &header);
@ -93,6 +95,8 @@ public:
}
quint32 dynamicTableSize() const;
quint32 dynamicTableCapacity() const;
quint32 maxDynamicTableCapacity() const;
void setMaxDynamicTableSize(quint32 size);

View File

@ -208,6 +208,16 @@ quint32 FieldLookupTable::dynamicDataSize() const
return dataSize;
}
quint32 FieldLookupTable::dynamicDataCapacity() const
{
return tableCapacity;
}
quint32 FieldLookupTable::maxDynamicDataCapacity() const
{
return maxTableSize;
}
void FieldLookupTable::clearDynamicTable()
{
searchIndex.clear();
@ -386,6 +396,7 @@ bool FieldLookupTable::updateDynamicTableSize(quint32 size)
{
if (!size) {
clearDynamicTable();
tableCapacity = 0;
return true;
}
@ -404,10 +415,10 @@ void FieldLookupTable::setMaxDynamicTableSize(quint32 size)
// This is for an external user, for example, HTTP2 protocol
// layer that can receive SETTINGS frame from its peer.
// No validity checks here, up to this external user.
// We update max size and capacity (this can also result in
// items evicted or even dynamic table completely cleared).
// We update max size only, the capacity will be updated
// later through the Dynamic Table Size Update mechanism
// in HPack.
maxTableSize = size;
updateDynamicTableSize(size);
}
// This data is from the HPACK's specs and it's quite conveniently sorted,

View File

@ -124,6 +124,8 @@ public:
quint32 numberOfStaticEntries() const;
quint32 numberOfDynamicEntries() const;
quint32 dynamicDataSize() const;
quint32 dynamicDataCapacity() const;
quint32 maxDynamicDataCapacity() const;
void clearDynamicTable();
bool indexIsValid(quint32 index) const;

View File

@ -36,6 +36,8 @@ private Q_SLOTS:
void lookupTableStatic();
void lookupTableDynamic();
void dynamicTableSizeUpdate();
void hpackEncodeRequest_data();
void hpackEncodeRequest();
void hpackDecodeRequest_data();
@ -469,6 +471,30 @@ void tst_Hpack::lookupTableDynamic()
QVERIFY(table.indexOf("name1") == 0);
}
void tst_Hpack::dynamicTableSizeUpdate()
{
std::vector<uchar> buffer;
buffer.reserve(4096);
BitOStream out(buffer);
HPack::Encoder encoder(4096u, true);
QVERIFY(encoder.encodeSizeUpdate(out, 2048u));
QVERIFY(encoder.encodeRequest(out, {{":method", "GET"}, {":path", "/"}, {":scheme", "https"}}));
BitIStream in(buffer.data(), buffer.data() + buffer.size());
HPack::Decoder decoder(4096u);
QCOMPARE(decoder.dynamicTableCapacity(), 4096u);
QCOMPARE(decoder.maxDynamicTableCapacity(), 4096u);
QVERIFY(decoder.decodeHeaderFields(in));
QCOMPARE(decoder.dynamicTableCapacity(), 2048u);
// The max is adjusted out-of-band, so it's not changed here:
QCOMPARE(decoder.maxDynamicTableCapacity(), 4096u);
}
void tst_Hpack::hpackEncodeRequest_data()
{
QTest::addColumn<bool>("compression");