From 191b1af880f5e88830c10eed1601a29d1ef398f3 Mon Sep 17 00:00:00 2001 From: Lachlan Roberts Date: Tue, 9 Mar 2021 16:58:26 +1100 Subject: [PATCH] Qpack refactoring, testing and bug fixes. Signed-off-by: Lachlan Roberts --- .../jetty/http3/qpack/EncodableEntry.java | 77 ++++++++-- .../jetty/http3/qpack/QpackContext.java | 13 ++ .../jetty/http3/qpack/QpackDecoder.java | 64 ++++++-- .../jetty/http3/qpack/QpackEncoder.java | 137 ++++++++++++------ .../eclipse/jetty/http3/qpack/StreamInfo.java | 29 +++- .../parser/DecoderInstructionParser.java | 40 ----- .../qpack/parser/EncodedFieldSection.java | 14 +- .../parser/EncoderInstructionParser.java | 34 ----- .../jetty/http3/qpack/table/DynamicTable.java | 130 ++++++++++++++--- .../jetty/http3/qpack/table/Entry.java | 13 +- .../jetty/http3/qpack/EncodeDecodeTest.java | 53 ++----- .../jetty/http3/qpack/EvictionTest.java | 103 +++++++++++++ .../jetty/http3/qpack/QpackTestUtil.java | 54 +++++++ .../jetty/http3/qpack/TestDecoderHandler.java | 10 +- .../jetty/http3/qpack/TestEncoderHandler.java | 10 +- 15 files changed, 566 insertions(+), 215 deletions(-) create mode 100644 jetty-http3/http3-qpack/src/test/java/org/eclipse/jetty/http3/qpack/EvictionTest.java create mode 100644 jetty-http3/http3-qpack/src/test/java/org/eclipse/jetty/http3/qpack/QpackTestUtil.java diff --git a/jetty-http3/http3-qpack/src/main/java/org/eclipse/jetty/http3/qpack/EncodableEntry.java b/jetty-http3/http3-qpack/src/main/java/org/eclipse/jetty/http3/qpack/EncodableEntry.java index 6d4d035383b..47e84f53e51 100644 --- a/jetty-http3/http3-qpack/src/main/java/org/eclipse/jetty/http3/qpack/EncodableEntry.java +++ b/jetty-http3/http3-qpack/src/main/java/org/eclipse/jetty/http3/qpack/EncodableEntry.java @@ -61,17 +61,52 @@ abstract class EncodableEntry @Override public void encode(ByteBuffer buffer, int base) { - byte staticBit = _entry.isStatic() ? (byte)0x40 : (byte)0x00; - buffer.put((byte)(0x80 | staticBit)); - int relativeIndex = _entry.getIndex() - base; - NBitInteger.encode(buffer, 6, relativeIndex); + boolean isStatic = _entry.isStatic(); + if (isStatic) + { + // Indexed Field Line with Static Reference. + buffer.put((byte)(0x80 | 0x40)); + int relativeIndex = _entry.getIndex(); + NBitInteger.encode(buffer, 6, relativeIndex); + } + else if (_entry.getIndex() < base) + { + // Indexed Field Line with Dynamic Reference. + buffer.put((byte)0x80); + int relativeIndex = base - (_entry.getIndex() + 1); + NBitInteger.encode(buffer, 6, relativeIndex); + } + else + { + // Indexed Field Line with Post-Base Index. + buffer.put((byte)0x10); + int relativeIndex = _entry.getIndex() - base; + NBitInteger.encode(buffer, 4, relativeIndex); + } } @Override public int getRequiredSize(int base) { - int relativeIndex = _entry.getIndex() - base; - return 1 + NBitInteger.octectsNeeded(6, relativeIndex); + boolean isStatic = _entry.isStatic(); + if (isStatic) + { + // Indexed Field Line with Static Reference. + int relativeIndex = _entry.getIndex(); + return 1 + NBitInteger.octectsNeeded(6, relativeIndex); + } + else if (_entry.getIndex() < base) + { + // Indexed Field Line with Dynamic Reference. + int relativeIndex = base - (_entry.getIndex() + 1); + return 1 + NBitInteger.octectsNeeded(6, relativeIndex); + } + else + { + // Indexed Field Line with Post-Base Index. + int relativeIndex = _entry.getIndex() - base; + return 1 + NBitInteger.octectsNeeded(4, relativeIndex); + } } @Override @@ -97,13 +132,33 @@ abstract class EncodableEntry @Override public void encode(ByteBuffer buffer, int base) { - byte allowIntermediary = 0x00; // TODO: this is 0x20 bit, when should this be set? - byte staticBit = _nameEntry.isStatic() ? (byte)0x10 : (byte)0x00; + // TODO: when should this be set? + // TODO: this is different for the Post-Base index case. + byte allowIntermediary = 0x00; // Encode the prefix. - buffer.put((byte)(0x40 | allowIntermediary | staticBit)); - int relativeIndex = _nameEntry.getIndex() - base; - NBitInteger.encode(buffer, 4, relativeIndex); + boolean isStatic = _nameEntry.isStatic(); + if (isStatic) + { + // Literal Field Line with Static Name Reference. + buffer.put((byte)(0x40 | allowIntermediary | 0x10)); + int relativeIndex = _nameEntry.getIndex(); + NBitInteger.encode(buffer, 4, relativeIndex); + } + else if (_nameEntry.getIndex() < base) + { + // Literal Field Line with Dynamic Name Reference. + buffer.put((byte)(0x40 | allowIntermediary)); + int relativeIndex = base - (_nameEntry.getIndex() + 1); + NBitInteger.encode(buffer, 4, relativeIndex); + } + else + { + // Literal Field Line with Post-Base Name Reference. + buffer.put(allowIntermediary); + int relativeIndex = _nameEntry.getIndex() - base; + NBitInteger.encode(buffer, 3, relativeIndex); + } // Encode the value. String value = getValue(); diff --git a/jetty-http3/http3-qpack/src/main/java/org/eclipse/jetty/http3/qpack/QpackContext.java b/jetty-http3/http3-qpack/src/main/java/org/eclipse/jetty/http3/qpack/QpackContext.java index ef04eb3a3c1..96897beec75 100644 --- a/jetty-http3/http3-qpack/src/main/java/org/eclipse/jetty/http3/qpack/QpackContext.java +++ b/jetty-http3/http3-qpack/src/main/java/org/eclipse/jetty/http3/qpack/QpackContext.java @@ -75,4 +75,17 @@ public class QpackContext return _dynamicTable.get(index); } + + /** + * Get the relative Index of an entry. + * @param entry the entry to get the index of. + * @return the relative index of the entry. + */ + public int indexOf(Entry entry) + { + if (entry.isStatic()) + return entry.getIndex(); + + return _dynamicTable.index(entry); + } } diff --git a/jetty-http3/http3-qpack/src/main/java/org/eclipse/jetty/http3/qpack/QpackDecoder.java b/jetty-http3/http3-qpack/src/main/java/org/eclipse/jetty/http3/qpack/QpackDecoder.java index 5624d0c53dc..5c1fb6ba494 100644 --- a/jetty-http3/http3-qpack/src/main/java/org/eclipse/jetty/http3/qpack/QpackDecoder.java +++ b/jetty-http3/http3-qpack/src/main/java/org/eclipse/jetty/http3/qpack/QpackDecoder.java @@ -13,6 +13,7 @@ package org.eclipse.jetty.http3.qpack; +import java.io.IOException; import java.nio.ByteBuffer; import java.util.ArrayList; import java.util.List; @@ -23,11 +24,13 @@ import org.eclipse.jetty.http.HttpHeader; import org.eclipse.jetty.http3.qpack.generator.InsertCountIncrementInstruction; import org.eclipse.jetty.http3.qpack.generator.Instruction; import org.eclipse.jetty.http3.qpack.generator.SectionAcknowledgmentInstruction; +import org.eclipse.jetty.http3.qpack.parser.DecoderInstructionParser; import org.eclipse.jetty.http3.qpack.parser.EncodedFieldSection; import org.eclipse.jetty.http3.qpack.parser.NBitIntegerParser; import org.eclipse.jetty.http3.qpack.table.DynamicTable; import org.eclipse.jetty.http3.qpack.table.Entry; import org.eclipse.jetty.http3.qpack.table.StaticTable; +import org.eclipse.jetty.util.component.Dumpable; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -35,7 +38,7 @@ import org.slf4j.LoggerFactory; * Qpack Decoder *

This is not thread safe and may only be called by 1 thread at a time.

*/ -public class QpackDecoder +public class QpackDecoder implements Dumpable { public static final Logger LOG = LoggerFactory.getLogger(QpackDecoder.class); public static final HttpField.LongValueHttpField CONTENT_LENGTH_0 = @@ -44,6 +47,7 @@ public class QpackDecoder private final Handler _handler; private final QpackContext _context; private final MetaDataBuilder _builder; + private final DecoderInstructionParser _parser; private final List _encodedFieldSections = new ArrayList<>(); private final NBitIntegerParser _integerDecoder = new NBitIntegerParser(); @@ -56,6 +60,7 @@ public class QpackDecoder _context = new QpackContext(); _builder = new MetaDataBuilder(maxHeaderSize); _handler = handler; + _parser = new DecoderInstructionParser(new DecoderAdapter()); } public QpackContext getQpackContext() @@ -67,7 +72,7 @@ public class QpackDecoder { void onHttpFields(int streamId, HttpFields httpFields); - void onInstruction(Instruction instruction); + void onInstruction(Instruction instruction) throws QpackException; } public void decode(int streamId, ByteBuffer buffer) throws QpackException @@ -103,7 +108,7 @@ public class QpackDecoder EncodedFieldSection encodedFieldSection = new EncodedFieldSection(streamId, requiredInsertCount, base); encodedFieldSection.parse(buffer); - if (encodedFieldSection.getRequiredInsertCount() <= insertCount) + if (requiredInsertCount <= insertCount) { _handler.onHttpFields(streamId, encodedFieldSection.decode(_context)); _handler.onInstruction(new SectionAcknowledgmentInstruction(streamId)); @@ -115,6 +120,11 @@ public class QpackDecoder } } + public void parseInstruction(ByteBuffer buffer) throws QpackException + { + _parser.parse(buffer); + } + private void checkEncodedFieldSections() throws QpackException { int insertCount = _context.getDynamicTable().getInsertCount(); @@ -128,7 +138,7 @@ public class QpackDecoder } } - public void setCapacity(int capacity) + void setCapacity(int capacity) { synchronized (this) { @@ -136,7 +146,7 @@ public class QpackDecoder } } - public void insert(int index) throws QpackException + void insert(int index) throws QpackException { synchronized (this) { @@ -150,11 +160,11 @@ public class QpackDecoder } } - public void insert(int nameIndex, boolean isDynamicTableIndex, String value) throws QpackException + void insert(int nameIndex, boolean isDynamicTableIndex, String value) throws QpackException { synchronized (this) { - StaticTable staticTable = _context.getStaticTable(); + StaticTable staticTable = QpackContext.getStaticTable(); DynamicTable dynamicTable = _context.getDynamicTable(); Entry referencedEntry = isDynamicTableIndex ? dynamicTable.get(nameIndex) : staticTable.get(nameIndex); @@ -166,14 +176,14 @@ public class QpackDecoder } } - public void insert(String name, String value) throws QpackException + void insert(String name, String value) throws QpackException { synchronized (this) { - DynamicTable dynamicTable = _context.getDynamicTable(); Entry entry = new Entry(new HttpField(name, value)); // Add the new Entry to the DynamicTable. + DynamicTable dynamicTable = _context.getDynamicTable(); dynamicTable.add(entry); _handler.onInstruction(new InsertCountIncrementInstruction(1)); checkEncodedFieldSections(); @@ -210,9 +220,45 @@ public class QpackDecoder return reqInsertCount; } + @Override + public void dump(Appendable out, String indent) throws IOException + { + synchronized (this) + { + Dumpable.dumpObjects(out, indent, _context.getDynamicTable()); + } + } + @Override public String toString() { return String.format("QpackDecoder@%x{%s}", hashCode(), _context); } + + class DecoderAdapter implements DecoderInstructionParser.Handler + { + @Override + public void onSetDynamicTableCapacity(int capacity) + { + setCapacity(capacity); + } + + @Override + public void onDuplicate(int index) throws QpackException + { + insert(index); + } + + @Override + public void onInsertNameWithReference(int nameIndex, boolean isDynamicTableIndex, String value) throws QpackException + { + insert(nameIndex, isDynamicTableIndex, value); + } + + @Override + public void onInsertWithLiteralName(String name, String value) throws QpackException + { + insert(name, value); + } + } } diff --git a/jetty-http3/http3-qpack/src/main/java/org/eclipse/jetty/http3/qpack/QpackEncoder.java b/jetty-http3/http3-qpack/src/main/java/org/eclipse/jetty/http3/qpack/QpackEncoder.java index e78b7da9ba2..0c35867b744 100644 --- a/jetty-http3/http3-qpack/src/main/java/org/eclipse/jetty/http3/qpack/QpackEncoder.java +++ b/jetty-http3/http3-qpack/src/main/java/org/eclipse/jetty/http3/qpack/QpackEncoder.java @@ -13,6 +13,7 @@ package org.eclipse.jetty.http3.qpack; +import java.io.IOException; import java.nio.ByteBuffer; import java.util.ArrayList; import java.util.EnumMap; @@ -32,15 +33,17 @@ import org.eclipse.jetty.http3.qpack.generator.IndexedNameEntryInstruction; import org.eclipse.jetty.http3.qpack.generator.Instruction; import org.eclipse.jetty.http3.qpack.generator.LiteralNameEntryInstruction; import org.eclipse.jetty.http3.qpack.generator.SetCapacityInstruction; +import org.eclipse.jetty.http3.qpack.parser.EncoderInstructionParser; import org.eclipse.jetty.http3.qpack.table.DynamicTable; import org.eclipse.jetty.http3.qpack.table.Entry; import org.eclipse.jetty.io.ByteBufferPool; import org.eclipse.jetty.io.NullByteBufferPool; import org.eclipse.jetty.util.BufferUtil; +import org.eclipse.jetty.util.component.Dumpable; import org.slf4j.Logger; import org.slf4j.LoggerFactory; -public class QpackEncoder +public class QpackEncoder implements Dumpable { private static final Logger LOG = LoggerFactory.getLogger(QpackEncoder.class); private static final HttpField[] STATUSES = new HttpField[599]; @@ -96,17 +99,17 @@ public class QpackEncoder public interface Handler { - void onInstruction(Instruction instruction); + void onInstruction(Instruction instruction) throws QpackException; } private final ByteBufferPool _bufferPool; private final Handler _handler; private final QpackContext _context; private final int _maxBlockedStreams; - private int _knownInsertCount; - - private int _blockedStreams = 0; private final Map _streamInfoMap = new HashMap<>(); + private final EncoderInstructionParser _parser; + private int _knownInsertCount; + private int _blockedStreams = 0; public QpackEncoder(Handler handler, int maxBlockedStreams) { @@ -120,6 +123,7 @@ public class QpackEncoder _context = new QpackContext(); _maxBlockedStreams = maxBlockedStreams; _knownInsertCount = 0; + _parser = new EncoderInstructionParser(new EncoderAdapter()); } public QpackContext getQpackContext() @@ -127,7 +131,12 @@ public class QpackEncoder return _context; } - public void setCapacity(int capacity) + /** + * Set the capacity of the DynamicTable and send a instruction to set the capacity on the remote Decoder. + * @param capacity the new capacity. + * @throws QpackException + */ + public void setCapacity(int capacity) throws QpackException { synchronized (this) { @@ -136,26 +145,23 @@ public class QpackEncoder } } - public void insertCountIncrement(int increment) throws QpackException + public void parseInstruction(ByteBuffer buffer) throws QpackException + { + _parser.parse(buffer); + } + + void insertCountIncrement(int increment) throws QpackException { synchronized (this) { int insertCount = _context.getDynamicTable().getInsertCount(); if (_knownInsertCount + increment > insertCount) throw new QpackException.StreamException("KnownInsertCount incremented over InsertCount"); - - // TODO: release any references to entries which used to insert new entries. - for (Entry entry : _context.getDynamicTable()) - { - if (entry.getIndex() > _knownInsertCount) - break; - } - _knownInsertCount += increment; } } - public void sectionAcknowledgement(int streamId) throws QpackException + void sectionAcknowledgement(int streamId) throws QpackException { synchronized (this) { @@ -165,6 +171,7 @@ public class QpackEncoder // The KnownInsertCount should be updated to the earliest sent RequiredInsertCount on that stream. StreamInfo.SectionInfo sectionInfo = streamInfo.acknowledge(); + sectionInfo.release(); _knownInsertCount = Math.max(_knownInsertCount, sectionInfo.getRequiredInsertCount()); // If we have no more outstanding section acknowledgments remove the StreamInfo. @@ -173,17 +180,23 @@ public class QpackEncoder } } - public void streamCancellation(int streamId) throws QpackException + void streamCancellation(int streamId) throws QpackException { synchronized (this) { StreamInfo streamInfo = _streamInfoMap.remove(streamId); if (streamInfo == null) throw new QpackException.StreamException("No StreamInfo for " + streamId); + + // Release all referenced entries outstanding on the stream that was cancelled. + for (StreamInfo.SectionInfo sectionInfo : streamInfo) + { + sectionInfo.release(); + } } } - private boolean referenceEntry(Entry entry) + private boolean referenceEntry(Entry entry, StreamInfo streamInfo) { if (entry == null) return false; @@ -195,32 +208,28 @@ public class QpackEncoder if (inEvictionZone) return false; + StreamInfo.SectionInfo sectionInfo = streamInfo.getCurrentSectionInfo(); + // If they have already acknowledged this entry we can reference it straight away. if (_knownInsertCount >= entry.getIndex() + 1) { - entry.reference(); + sectionInfo.reference(entry); return true; } - return false; - } - - private boolean referenceEntry(Entry entry, StreamInfo streamInfo) - { - if (referenceEntry(entry)) - return true; - // We may need to risk blocking the stream in order to reference it. if (streamInfo.isBlocked()) { - streamInfo.getCurrentSectionInfo().block(); + sectionInfo.block(); + sectionInfo.reference(entry); return true; } if (_blockedStreams < _maxBlockedStreams) { _blockedStreams++; - streamInfo.getCurrentSectionInfo().block(); + sectionInfo.block(); + sectionInfo.reference(entry); return true; } @@ -259,7 +268,6 @@ public class QpackEncoder synchronized (this) { DynamicTable dynamicTable = _context.getDynamicTable(); - dynamicTable.evict(); StreamInfo streamInfo = _streamInfoMap.get(streamId); if (streamInfo == null) @@ -285,6 +293,7 @@ public class QpackEncoder } } + sectionInfo.setRequiredInsertCount(requiredInsertCount); base = dynamicTable.getBase(); encodedInsertCount = encodeInsertCount(requiredInsertCount, dynamicTable.getCapacity()); signBit = base < requiredInsertCount; @@ -310,7 +319,7 @@ public class QpackEncoder return buffer; } - private EncodableEntry encode(StreamInfo streamInfo, HttpField field) + private EncodableEntry encode(StreamInfo streamInfo, HttpField field) throws QpackException { DynamicTable dynamicTable = _context.getDynamicTable(); @@ -322,21 +331,22 @@ public class QpackEncoder if (field instanceof PreEncodedHttpField) return EncodableEntry.getPreEncodedEntry((PreEncodedHttpField)field); - boolean canCreateEntry = shouldIndex(field) && (Entry.getSize(field) <= dynamicTable.getSpace()); + boolean canCreateEntry = shouldIndex(field) && dynamicTable.canInsert(field); Entry entry = _context.get(field); - if (entry != null && referenceEntry(entry, streamInfo)) + if (referenceEntry(entry, streamInfo)) { return EncodableEntry.getReferencedEntry(entry); } else { // Should we duplicate this entry. - if (entry != null && dynamicTable.canReference(entry) && canCreateEntry) + if (entry != null && canCreateEntry) { + int index = _context.indexOf(entry); Entry newEntry = new Entry(field); dynamicTable.add(newEntry); - _handler.onInstruction(new DuplicateInstruction(entry.getIndex())); + _handler.onInstruction(new DuplicateInstruction(index)); // Should we reference this entry and risk blocking. if (referenceEntry(newEntry, streamInfo)) @@ -346,14 +356,15 @@ public class QpackEncoder boolean huffman = shouldHuffmanEncode(field); Entry nameEntry = _context.get(field.getName()); - if (nameEntry != null && referenceEntry(nameEntry, streamInfo)) + if (referenceEntry(nameEntry, streamInfo)) { // Should we copy this entry if (canCreateEntry) { + int index = _context.indexOf(nameEntry); Entry newEntry = new Entry(field); dynamicTable.add(newEntry); - _handler.onInstruction(new IndexedNameEntryInstruction(!nameEntry.isStatic(), nameEntry.getIndex(), huffman, field.getValue())); + _handler.onInstruction(new IndexedNameEntryInstruction(!nameEntry.isStatic(), index, huffman, field.getValue())); // Should we reference this entry and risk blocking. if (referenceEntry(newEntry, streamInfo)) @@ -379,38 +390,40 @@ public class QpackEncoder } } - public boolean insert(HttpField field) + public boolean insert(HttpField field) throws QpackException { synchronized (this) { DynamicTable dynamicTable = _context.getDynamicTable(); - dynamicTable.evict(); if (field.getValue() == null) field = new HttpField(field.getHeader(), field.getName(), ""); - boolean canCreateEntry = shouldIndex(field) && (Entry.getSize(field) <= dynamicTable.getSpace()); + boolean canCreateEntry = shouldIndex(field) && dynamicTable.canInsert(field); if (!canCreateEntry) return false; - Entry newEntry = new Entry(field); - dynamicTable.add(newEntry); - + // We can always reference on insertion as it will always arrive before any eviction. Entry entry = _context.get(field); - if (referenceEntry(entry)) + if (entry != null) { - _handler.onInstruction(new DuplicateInstruction(entry.getIndex())); + int index = _context.indexOf(entry); + dynamicTable.add(new Entry(field)); + _handler.onInstruction(new DuplicateInstruction(index)); return true; } boolean huffman = shouldHuffmanEncode(field); Entry nameEntry = _context.get(field.getName()); - if (referenceEntry(nameEntry)) + if (nameEntry != null) { - _handler.onInstruction(new IndexedNameEntryInstruction(!nameEntry.isStatic(), nameEntry.getIndex(), huffman, field.getValue())); + int index = _context.indexOf(nameEntry); + dynamicTable.add(new Entry(field)); + _handler.onInstruction(new IndexedNameEntryInstruction(!nameEntry.isStatic(), index, huffman, field.getValue())); return true; } + dynamicTable.add(new Entry(field)); _handler.onInstruction(new LiteralNameEntryInstruction(huffman, field.getName(), huffman, field.getValue())); return true; } @@ -424,4 +437,34 @@ public class QpackEncoder int maxEntries = maxTableCapacity / 32; return (reqInsertCount % (2 * maxEntries)) + 1; } + + public class EncoderAdapter implements EncoderInstructionParser.Handler + { + @Override + public void onSectionAcknowledgement(int streamId) throws QpackException + { + sectionAcknowledgement(streamId); + } + + @Override + public void onStreamCancellation(int streamId) throws QpackException + { + streamCancellation(streamId); + } + + @Override + public void onInsertCountIncrement(int increment) throws QpackException + { + insertCountIncrement(increment); + } + } + + @Override + public void dump(Appendable out, String indent) throws IOException + { + synchronized (this) + { + Dumpable.dumpObjects(out, indent, _context.getDynamicTable()); + } + } } diff --git a/jetty-http3/http3-qpack/src/main/java/org/eclipse/jetty/http3/qpack/StreamInfo.java b/jetty-http3/http3-qpack/src/main/java/org/eclipse/jetty/http3/qpack/StreamInfo.java index c710ca99ccb..1bf08a56836 100644 --- a/jetty-http3/http3-qpack/src/main/java/org/eclipse/jetty/http3/qpack/StreamInfo.java +++ b/jetty-http3/http3-qpack/src/main/java/org/eclipse/jetty/http3/qpack/StreamInfo.java @@ -13,10 +13,15 @@ package org.eclipse.jetty.http3.qpack; +import java.util.ArrayList; +import java.util.Iterator; import java.util.LinkedList; +import java.util.List; import java.util.Queue; -class StreamInfo +import org.eclipse.jetty.http3.qpack.table.Entry; + +class StreamInfo implements Iterable { private final int _streamId; private final Queue _sectionInfos = new LinkedList<>(); @@ -62,8 +67,15 @@ class StreamInfo return false; } + @Override + public Iterator iterator() + { + return _sectionInfos.iterator(); + } + public static class SectionInfo { + private final List _entries = new ArrayList<>(); private int _requiredInsertCount; private boolean _block = false; @@ -77,6 +89,21 @@ class StreamInfo return _block; } + public void reference(Entry entry) + { + entry.reference(); + _entries.add(entry); + } + + public void release() + { + for (Entry entry : _entries) + { + entry.release(); + } + _entries.clear(); + } + public void setRequiredInsertCount(int requiredInsertCount) { _requiredInsertCount = requiredInsertCount; diff --git a/jetty-http3/http3-qpack/src/main/java/org/eclipse/jetty/http3/qpack/parser/DecoderInstructionParser.java b/jetty-http3/http3-qpack/src/main/java/org/eclipse/jetty/http3/qpack/parser/DecoderInstructionParser.java index b990cdb28ce..389e78b5af1 100644 --- a/jetty-http3/http3-qpack/src/main/java/org/eclipse/jetty/http3/qpack/parser/DecoderInstructionParser.java +++ b/jetty-http3/http3-qpack/src/main/java/org/eclipse/jetty/http3/qpack/parser/DecoderInstructionParser.java @@ -15,7 +15,6 @@ package org.eclipse.jetty.http3.qpack.parser; import java.nio.ByteBuffer; -import org.eclipse.jetty.http3.qpack.QpackDecoder; import org.eclipse.jetty.http3.qpack.QpackException; /** @@ -61,45 +60,6 @@ public class DecoderInstructionParser void onInsertWithLiteralName(String name, String value) throws QpackException; } - public static class DecoderAdapter implements Handler - { - private final QpackDecoder _decoder; - - public DecoderAdapter(QpackDecoder decoder) - { - _decoder = decoder; - } - - @Override - public void onSetDynamicTableCapacity(int capacity) - { - _decoder.setCapacity(capacity); - } - - @Override - public void onDuplicate(int index) throws QpackException - { - _decoder.insert(index); - } - - @Override - public void onInsertNameWithReference(int nameIndex, boolean isDynamicTableIndex, String value) throws QpackException - { - _decoder.insert(nameIndex, isDynamicTableIndex, value); - } - - @Override - public void onInsertWithLiteralName(String name, String value) throws QpackException - { - _decoder.insert(name, value); - } - } - - public DecoderInstructionParser(QpackDecoder decoder) - { - this(new DecoderAdapter(decoder)); - } - public DecoderInstructionParser(Handler handler) { _handler = handler; diff --git a/jetty-http3/http3-qpack/src/main/java/org/eclipse/jetty/http3/qpack/parser/EncodedFieldSection.java b/jetty-http3/http3-qpack/src/main/java/org/eclipse/jetty/http3/qpack/parser/EncodedFieldSection.java index bd06db3760b..c8c5021f415 100644 --- a/jetty-http3/http3-qpack/src/main/java/org/eclipse/jetty/http3/qpack/parser/EncodedFieldSection.java +++ b/jetty-http3/http3-qpack/src/main/java/org/eclipse/jetty/http3/qpack/parser/EncodedFieldSection.java @@ -118,7 +118,7 @@ public class EncodedFieldSection byte firstByte = buffer.get(buffer.position()); boolean allowEncoding = (firstByte & 0x10) != 0; - _stringParser.setPrefix(3); + _stringParser.setPrefix(4); String name = _stringParser.decode(buffer); if (name == null) throw new QpackException.CompressionException("Invalid Name"); @@ -136,7 +136,7 @@ public class EncodedFieldSection byte firstByte = buffer.get(buffer.position()); boolean allowEncoding = (firstByte & 0x08) != 0; - _integerParser.setPrefix(3); + _integerParser.setPrefix(4); int nameIndex = _integerParser.decode(buffer); if (nameIndex < 0) throw new QpackException.CompressionException("Invalid Index"); @@ -199,9 +199,9 @@ public class EncodedFieldSection public HttpField decode(QpackContext context) throws QpackException { if (_dynamicTable) - return context.getDynamicTable().getAbsolute(_base + _index).getHttpField(); + return context.getDynamicTable().getAbsolute(_base - (_index + 1)).getHttpField(); else - return context.getStaticTable().get(_index).getHttpField(); + return QpackContext.getStaticTable().get(_index).getHttpField(); } } @@ -217,7 +217,7 @@ public class EncodedFieldSection @Override public HttpField decode(QpackContext context) throws QpackException { - return context.getDynamicTable().getAbsolute(_base - _index).getHttpField(); + return context.getDynamicTable().getAbsolute(_base + _index).getHttpField(); } } @@ -242,7 +242,7 @@ public class EncodedFieldSection { HttpField field; if (_dynamicTable) - field = context.getDynamicTable().getAbsolute(_base + _nameIndex + 1).getHttpField(); + field = context.getDynamicTable().getAbsolute(_base - (_nameIndex + 1)).getHttpField(); else field = QpackContext.getStaticTable().get(_nameIndex).getHttpField(); @@ -266,7 +266,7 @@ public class EncodedFieldSection @Override public HttpField decode(QpackContext context) throws QpackException { - HttpField field = context.getDynamicTable().getAbsolute(_base - _nameIndex).getHttpField(); + HttpField field = context.getDynamicTable().getAbsolute(_base + _nameIndex).getHttpField(); return new HttpField(field.getHeader(), field.getName(), _value); } } diff --git a/jetty-http3/http3-qpack/src/main/java/org/eclipse/jetty/http3/qpack/parser/EncoderInstructionParser.java b/jetty-http3/http3-qpack/src/main/java/org/eclipse/jetty/http3/qpack/parser/EncoderInstructionParser.java index 657638a7d2e..65cf122ec02 100644 --- a/jetty-http3/http3-qpack/src/main/java/org/eclipse/jetty/http3/qpack/parser/EncoderInstructionParser.java +++ b/jetty-http3/http3-qpack/src/main/java/org/eclipse/jetty/http3/qpack/parser/EncoderInstructionParser.java @@ -15,7 +15,6 @@ package org.eclipse.jetty.http3.qpack.parser; import java.nio.ByteBuffer; -import org.eclipse.jetty.http3.qpack.QpackEncoder; import org.eclipse.jetty.http3.qpack.QpackException; /** @@ -48,39 +47,6 @@ public class EncoderInstructionParser void onInsertCountIncrement(int increment) throws QpackException; } - public static class EncoderAdapter implements Handler - { - private final QpackEncoder _encoder; - - public EncoderAdapter(QpackEncoder encoder) - { - _encoder = encoder; - } - - @Override - public void onSectionAcknowledgement(int streamId) throws QpackException - { - _encoder.sectionAcknowledgement(streamId); - } - - @Override - public void onStreamCancellation(int streamId) throws QpackException - { - _encoder.streamCancellation(streamId); - } - - @Override - public void onInsertCountIncrement(int increment) throws QpackException - { - _encoder.insertCountIncrement(increment); - } - } - - public EncoderInstructionParser(QpackEncoder encoder) - { - this(new EncoderAdapter(encoder)); - } - public EncoderInstructionParser(Handler handler) { _handler = handler; diff --git a/jetty-http3/http3-qpack/src/main/java/org/eclipse/jetty/http3/qpack/table/DynamicTable.java b/jetty-http3/http3-qpack/src/main/java/org/eclipse/jetty/http3/qpack/table/DynamicTable.java index bb7a28c23da..d5aa0c4c6bf 100644 --- a/jetty-http3/http3-qpack/src/main/java/org/eclipse/jetty/http3/qpack/table/DynamicTable.java +++ b/jetty-http3/http3-qpack/src/main/java/org/eclipse/jetty/http3/qpack/table/DynamicTable.java @@ -13,6 +13,7 @@ package org.eclipse.jetty.http3.qpack.table; +import java.io.IOException; import java.util.ArrayList; import java.util.HashMap; import java.util.Iterator; @@ -20,10 +21,10 @@ import java.util.List; import java.util.Map; import org.eclipse.jetty.http.HttpField; -import org.eclipse.jetty.http3.qpack.QpackContext; import org.eclipse.jetty.http3.qpack.QpackException; +import org.eclipse.jetty.util.component.Dumpable; -public class DynamicTable implements Iterable +public class DynamicTable implements Iterable, Dumpable { private final Map _fieldMap = new HashMap<>(); private final Map _nameMap = new HashMap<>(); @@ -34,9 +35,38 @@ public class DynamicTable implements Iterable private int _absoluteIndex; private int _drainingIndex; + /** + * Add an entry into the Dynamic Table. This will throw if it is not possible to insert this entry. + * Use {@link #canInsert(HttpField)} to check whether it is possible to insert this entry. + * @param entry the entry to insert. + */ public void add(Entry entry) { + // Evict entries until there is space for the new entry. + Iterator iterator = _entries.iterator(); int entrySize = entry.getSize(); + while (getSpace() < entrySize) + { + if (!iterator.hasNext()) + throw new IllegalStateException("not enough space in dynamic table to add entry"); + + Entry e = iterator.next(); + if (e.getReferenceCount() != 0) + throw new IllegalStateException("cannot evict entry that is still referenced"); + + // Evict the entry from the DynamicTable. + _size -= e.getSize(); + iterator.remove(); + + HttpField httpField = e.getHttpField(); + if (e == _fieldMap.get(httpField)) + _fieldMap.remove(httpField); + + String name = httpField.getLowerCaseName(); + if (e == _nameMap.get(name)) + _nameMap.remove(name); + } + if (entrySize + _size > _capacity) throw new IllegalStateException("No available space"); _size += entrySize; @@ -51,16 +81,73 @@ public class DynamicTable implements Iterable _drainingIndex = getDrainingIndex(); } - public int index(Entry entry) + /** + * Is there enough room to insert a new entry into the Dynamic Table, possibly evicting unreferenced entries. + * @param field the HttpField to insert into the table. + * @return if an entry with this HttpField can be inserted. + */ + public boolean canInsert(HttpField field) { - return _entries.indexOf(entry); + int availableSpace = getSpace(); + int requiredSpace = Entry.getSize(field); + + if (availableSpace >= requiredSpace) + return true; + + if (requiredSpace > _capacity) + return false; + + // Could we potentially evict enough space to insert this new field. + for (Entry entry : _entries) + { + if (entry.getReferenceCount() != 0) + return false; + + availableSpace += entry.getSize(); + if (availableSpace >= requiredSpace) + return true; + } + + return false; } + /** + * Get the relative index of an entry in the Dynamic Table. + * @param entry the entry to find the relative index of. + * @return the relative index of this entry. + */ + public int index(Entry entry) + { + if (_entries.isEmpty()) + throw new IllegalArgumentException("Invalid Index"); + + Entry firstEntry = _entries.get(0); + int index = entry.getIndex() - firstEntry.getIndex(); + if (index >= _entries.size()) + throw new IllegalArgumentException("Invalid Index"); + + return index; + } + + /** + * Get an entry from the Dynamic table given an absolute index. + * @param absoluteIndex the absolute index of the entry in the table. + * @return the entry with the absolute index. + */ public Entry getAbsolute(int absoluteIndex) throws QpackException { if (absoluteIndex < 0) throw new QpackException.CompressionException("Invalid Index"); - return _entries.stream().filter(e -> e.getIndex() == absoluteIndex).findFirst().orElse(null); + + if (_entries.isEmpty()) + throw new IllegalArgumentException("Invalid Index"); + + Entry firstEntry = _entries.get(0); + int index = absoluteIndex - firstEntry.getIndex(); + if (index >= _entries.size()) + throw new IllegalArgumentException("Invalid Index"); + + return _entries.get(index); } public Entry get(int index) @@ -112,31 +199,21 @@ public class DynamicTable implements Iterable public void setCapacity(int capacity) { - if (QpackContext.LOG.isDebugEnabled()) - QpackContext.LOG.debug(String.format("HdrTbl[%x] resized max=%d->%d", hashCode(), _capacity, capacity)); _capacity = capacity; - } - public boolean canReference(Entry entry) - { - return entry.getIndex() >= _drainingIndex; - } - - public void evict() - { - for (Entry entry : _entries) + Iterator iterator = _entries.iterator(); + while (_size > _capacity) { - if (entry.getIndex() >= _drainingIndex) - return; + if (!iterator.hasNext()) + throw new IllegalStateException(); - // We can only evict if there are no references outstanding to this entry. + Entry entry = iterator.next(); if (entry.getReferenceCount() != 0) return; // Evict the entry from the DynamicTable. _size -= entry.getSize(); - if (entry != _entries.remove(0)) - throw new IllegalStateException("Corruption in DynamicTable"); + iterator.remove(); HttpField httpField = entry.getHttpField(); if (entry == _fieldMap.get(httpField)) @@ -148,6 +225,11 @@ public class DynamicTable implements Iterable } } + public boolean canReference(Entry entry) + { + return entry.getIndex() >= _drainingIndex; + } + /** * Entries with indexes lower than the draining index should not be referenced. * This allows these entries to eventually be evicted as no more references will be made to them. @@ -183,6 +265,12 @@ public class DynamicTable implements Iterable return _entries.iterator(); } + @Override + public void dump(Appendable out, String indent) throws IOException + { + Dumpable.dumpObjects(out, indent, _entries); + } + @Override public String toString() { diff --git a/jetty-http3/http3-qpack/src/main/java/org/eclipse/jetty/http3/qpack/table/Entry.java b/jetty-http3/http3-qpack/src/main/java/org/eclipse/jetty/http3/qpack/table/Entry.java index 9c58a0308c1..7ff70b64441 100644 --- a/jetty-http3/http3-qpack/src/main/java/org/eclipse/jetty/http3/qpack/table/Entry.java +++ b/jetty-http3/http3-qpack/src/main/java/org/eclipse/jetty/http3/qpack/table/Entry.java @@ -65,6 +65,11 @@ public class Entry _referenceCount.incrementAndGet(); } + public void release() + { + _referenceCount.decrementAndGet(); + } + public int getReferenceCount() { return _referenceCount.get(); @@ -83,7 +88,13 @@ public class Entry @Override public String toString() { - return String.format("{%s,%d,%s,%x}", isStatic() ? "S" : "D", _absoluteIndex, _field, hashCode()); + return String.format("%s@%x{index=%d, refs=%d, field=\"%s\"}", getClass().getSimpleName(), hashCode(), + _absoluteIndex, _referenceCount.get(), _field); + } + + public static int getSize(Entry entry) + { + return getSize(entry.getHttpField()); } public static int getSize(HttpField field) diff --git a/jetty-http3/http3-qpack/src/test/java/org/eclipse/jetty/http3/qpack/EncodeDecodeTest.java b/jetty-http3/http3-qpack/src/test/java/org/eclipse/jetty/http3/qpack/EncodeDecodeTest.java index 732731bc45d..c17a27edc71 100644 --- a/jetty-http3/http3-qpack/src/test/java/org/eclipse/jetty/http3/qpack/EncodeDecodeTest.java +++ b/jetty-http3/http3-qpack/src/test/java/org/eclipse/jetty/http3/qpack/EncodeDecodeTest.java @@ -25,11 +25,7 @@ import org.eclipse.jetty.http3.qpack.generator.SectionAcknowledgmentInstruction; import org.eclipse.jetty.http3.qpack.generator.SetCapacityInstruction; import org.eclipse.jetty.http3.qpack.parser.DecoderInstructionParser; import org.eclipse.jetty.http3.qpack.parser.EncoderInstructionParser; -import org.eclipse.jetty.io.ByteBufferPool; -import org.eclipse.jetty.io.NullByteBufferPool; import org.eclipse.jetty.util.BufferUtil; -import org.eclipse.jetty.util.TypeUtil; -import org.hamcrest.Matcher; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; @@ -66,8 +62,6 @@ public class EncodeDecodeTest } }; _decoder = new QpackDecoder(_decoderHandler, MAX_HEADER_SIZE); - _encoderInstructionParser = new EncoderInstructionParser(_encoder); - _decoderInstructionParser = new DecoderInstructionParser(_decoder); } @Test @@ -79,7 +73,7 @@ public class EncodeDecodeTest ByteBuffer buffer = _encoder.encode(streamId, httpFields); assertNull(_encoderHandler.getInstruction()); - assertThat(BufferUtil.toHexString(buffer), equalsHex("0000 510b 2f69 6e64 6578 2e68 746d 6c")); + assertThat(BufferUtil.toHexString(buffer), QpackTestUtil.equalsHex("0000 510b 2f69 6e64 6578 2e68 746d 6c")); assertTrue(_encoderHandler.isEmpty()); _decoder.decode(streamId, buffer); @@ -88,7 +82,7 @@ public class EncodeDecodeTest assertThat(_decoderHandler.getInstruction(), instanceOf(SectionAcknowledgmentInstruction.class)); assertTrue(_decoderHandler.isEmpty()); - _encoderInstructionParser.parse(toBuffer(new SectionAcknowledgmentInstruction(streamId))); + _encoderInstructionParser.parse(QpackTestUtil.toBuffer(new SectionAcknowledgmentInstruction(streamId))); // B.2. Dynamic Table. @@ -97,9 +91,9 @@ public class EncodeDecodeTest Instruction instruction = _encoderHandler.getInstruction(); assertThat(instruction, instanceOf(SetCapacityInstruction.class)); assertThat(((SetCapacityInstruction)instruction).getCapacity(), is(220)); - assertThat(toString(instruction), equalsHex("3fbd01")); + assertThat(QpackTestUtil.toHexString(instruction), QpackTestUtil.equalsHex("3fbd01")); - _decoderInstructionParser.parse(toHex("3fbd01")); + _decoderInstructionParser.parse(QpackTestUtil.hexToBuffer("3fbd01")); assertThat(_decoder.getQpackContext().getDynamicTable().getCapacity(), is(220)); // Insert with named referenced to static table. Test we get two instructions generated to add to the dynamic table. @@ -113,24 +107,24 @@ public class EncodeDecodeTest assertThat(instruction, instanceOf(IndexedNameEntryInstruction.class)); assertThat(((IndexedNameEntryInstruction)instruction).getIndex(), is(0)); assertThat(((IndexedNameEntryInstruction)instruction).getValue(), is("www.example.com")); - assertThat(toString(instruction), equalsHex("c00f 7777 772e 6578 616d 706c 652e 636f 6d")); + assertThat(QpackTestUtil.toHexString(instruction), QpackTestUtil.equalsHex("c00f 7777 772e 6578 616d 706c 652e 636f 6d")); instruction = _encoderHandler.getInstruction(); assertThat(instruction, instanceOf(IndexedNameEntryInstruction.class)); assertThat(((IndexedNameEntryInstruction)instruction).getIndex(), is(1)); assertThat(((IndexedNameEntryInstruction)instruction).getValue(), is("/sample/path")); - assertThat(toString(instruction), equalsHex("c10c 2f73 616d 706c 652f 7061 7468")); + assertThat(QpackTestUtil.toHexString(instruction), QpackTestUtil.equalsHex("c10c 2f73 616d 706c 652f 7061 7468")); assertTrue(_encoderHandler.isEmpty()); // We cannot decode the buffer until we parse the two instructions generated above (we reach required insert count). _decoder.decode(streamId, buffer); assertNull(_decoderHandler.getHttpFields()); - _decoderInstructionParser.parse(toHex("c00f 7777 772e 6578 616d 706c 652e 636f 6d")); + _decoderInstructionParser.parse(QpackTestUtil.hexToBuffer("c00f 7777 772e 6578 616d 706c 652e 636f 6d")); assertNull(_decoderHandler.getHttpFields()); assertThat(_decoderHandler.getInstruction(), instanceOf(InsertCountIncrementInstruction.class)); - _decoderInstructionParser.parse(toHex("c10c 2f73 616d 706c 652f 7061 7468")); + _decoderInstructionParser.parse(QpackTestUtil.hexToBuffer("c10c 2f73 616d 706c 652f 7061 7468")); assertThat(_decoderHandler.getHttpFields(), is(httpFields)); assertThat(_decoderHandler.getInstruction(), instanceOf(InsertCountIncrementInstruction.class)); @@ -138,39 +132,14 @@ public class EncodeDecodeTest assertTrue(_decoderHandler.isEmpty()); // Parse the decoder instructions on the encoder. - _encoderInstructionParser.parse(toBuffer(new InsertCountIncrementInstruction(2))); - _encoderInstructionParser.parse(toBuffer(new SectionAcknowledgmentInstruction(streamId))); + _encoderInstructionParser.parse(QpackTestUtil.toBuffer(new InsertCountIncrementInstruction(2))); + _encoderInstructionParser.parse(QpackTestUtil.toBuffer(new SectionAcknowledgmentInstruction(streamId))); // B.3. Speculative Insert _encoder.insert(new HttpField("custom-key", "custom-value")); instruction = _encoderHandler.getInstruction(); assertThat(instruction, instanceOf(LiteralNameEntryInstruction.class)); - assertThat(toString(instruction), equalsHex("4a63 7573 746f 6d2d 6b65 790c 6375 7374 6f6d 2d76 616c 7565")); + assertThat(QpackTestUtil.toHexString(instruction), QpackTestUtil.equalsHex("4a63 7573 746f 6d2d 6b65 790c 6375 7374 6f6d 2d76 616c 7565")); _encoder.insertCountIncrement(1); } - - public static ByteBuffer toBuffer(Instruction instruction) - { - ByteBufferPool.Lease lease = new ByteBufferPool.Lease(new NullByteBufferPool()); - instruction.encode(lease); - assertThat(lease.getSize(), is(1)); - return lease.getByteBuffers().get(0); - } - - public static String toString(Instruction instruction) - { - return BufferUtil.toHexString(toBuffer(instruction)); - } - - public static ByteBuffer toHex(String hexString) - { - hexString = hexString.replaceAll("\\s+", ""); - return ByteBuffer.wrap(TypeUtil.fromHexString(hexString)); - } - - public static Matcher equalsHex(String expectedString) - { - expectedString = expectedString.replaceAll("\\s+", ""); - return org.hamcrest.text.IsEqualIgnoringCase.equalToIgnoringCase(expectedString); - } } diff --git a/jetty-http3/http3-qpack/src/test/java/org/eclipse/jetty/http3/qpack/EvictionTest.java b/jetty-http3/http3-qpack/src/test/java/org/eclipse/jetty/http3/qpack/EvictionTest.java new file mode 100644 index 00000000000..d8479c6519c --- /dev/null +++ b/jetty-http3/http3-qpack/src/test/java/org/eclipse/jetty/http3/qpack/EvictionTest.java @@ -0,0 +1,103 @@ +// +// ======================================================================== +// Copyright (c) 1995-2021 Mort Bay Consulting Pty Ltd and others. +// +// This program and the accompanying materials are made available under the +// terms of the Eclipse Public License v. 2.0 which is available at +// https://www.eclipse.org/legal/epl-2.0, or the Apache License, Version 2.0 +// which is available at https://www.apache.org/licenses/LICENSE-2.0. +// +// SPDX-License-Identifier: EPL-2.0 OR Apache-2.0 +// ======================================================================== +// + +package org.eclipse.jetty.http3.qpack; + +import java.nio.ByteBuffer; +import java.util.Random; + +import org.eclipse.jetty.http.HttpField; +import org.eclipse.jetty.http.HttpFields; +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Test; + +import static org.hamcrest.MatcherAssert.assertThat; +import static org.hamcrest.Matchers.is; + +public class EvictionTest +{ + private QpackEncoder _encoder; + private QpackDecoder _decoder; + private final TestDecoderHandler _decoderHandler = new TestDecoderHandler(); + private final TestEncoderHandler _encoderHandler = new TestEncoderHandler(); + private final Random random = new Random(); + + private static final int MAX_BLOCKED_STREAMS = 5; + private static final int MAX_HEADER_SIZE = 1024; + + @BeforeEach + public void before() + { + _decoder = new QpackDecoder(_decoderHandler, MAX_HEADER_SIZE); + _encoder = new QpackEncoder(_encoderHandler, MAX_BLOCKED_STREAMS) + { + @Override + protected boolean shouldHuffmanEncode(HttpField httpField) + { + return false; + } + }; + + // Set the instruction bytes to be passed on to the remote Encoder/Decoder through the handler directly. + _encoderHandler.setDecoder(_decoder); + _decoderHandler.setEncoder(_encoder); + } + + @Test + public void test() throws Exception + { + _encoder.setCapacity(1024); + + for (int i = 0; i < 10000; i++) + { + HttpFields httpFields = newRandomFields(5); + int streamId = getPositiveInt(10); + ByteBuffer encodedFields = _encoder.encode(streamId, httpFields); + _decoder.decode(streamId, encodedFields); + HttpFields result = _decoderHandler.getHttpFields(); + + System.err.println("encoder: "); + System.err.println(_encoder.dump()); + System.err.println(); + System.err.println("decoder: "); + System.err.println(_decoder.dump()); + System.err.println(); + System.err.println("===================="); + System.err.println(); + + assertThat(result, is(httpFields)); + } + } + + public HttpFields newRandomFields(int size) + { + HttpFields.Mutable fields = HttpFields.build(); + for (int i = 0; i < size; i++) + { + fields.add(newRandomField()); + } + return fields; + } + + public HttpField newRandomField() + { + String header = "header" + getPositiveInt(999); + String value = "value" + getPositiveInt(999); + return new HttpField(header, value); + } + + public int getPositiveInt(int max) + { + return Math.abs(random.nextInt(max)); + } +} diff --git a/jetty-http3/http3-qpack/src/test/java/org/eclipse/jetty/http3/qpack/QpackTestUtil.java b/jetty-http3/http3-qpack/src/test/java/org/eclipse/jetty/http3/qpack/QpackTestUtil.java new file mode 100644 index 00000000000..8e7db49e5a6 --- /dev/null +++ b/jetty-http3/http3-qpack/src/test/java/org/eclipse/jetty/http3/qpack/QpackTestUtil.java @@ -0,0 +1,54 @@ +// +// ======================================================================== +// Copyright (c) 1995-2021 Mort Bay Consulting Pty Ltd and others. +// +// This program and the accompanying materials are made available under the +// terms of the Eclipse Public License v. 2.0 which is available at +// https://www.eclipse.org/legal/epl-2.0, or the Apache License, Version 2.0 +// which is available at https://www.apache.org/licenses/LICENSE-2.0. +// +// SPDX-License-Identifier: EPL-2.0 OR Apache-2.0 +// ======================================================================== +// + +package org.eclipse.jetty.http3.qpack; + +import java.nio.ByteBuffer; + +import org.eclipse.jetty.http3.qpack.generator.Instruction; +import org.eclipse.jetty.io.ByteBufferPool; +import org.eclipse.jetty.io.NullByteBufferPool; +import org.eclipse.jetty.util.BufferUtil; +import org.eclipse.jetty.util.TypeUtil; +import org.hamcrest.Matcher; + +import static org.hamcrest.MatcherAssert.assertThat; +import static org.hamcrest.Matchers.is; + +public class QpackTestUtil +{ + public static Matcher equalsHex(String expectedString) + { + expectedString = expectedString.replaceAll("\\s+", ""); + return org.hamcrest.text.IsEqualIgnoringCase.equalToIgnoringCase(expectedString); + } + + public static ByteBuffer toBuffer(Instruction instruction) + { + ByteBufferPool.Lease lease = new ByteBufferPool.Lease(new NullByteBufferPool()); + instruction.encode(lease); + assertThat(lease.getSize(), is(1)); + return lease.getByteBuffers().get(0); + } + + public static ByteBuffer hexToBuffer(String hexString) + { + hexString = hexString.replaceAll("\\s+", ""); + return ByteBuffer.wrap(TypeUtil.fromHexString(hexString)); + } + + public static String toHexString(Instruction instruction) + { + return BufferUtil.toHexString(toBuffer(instruction)); + } +} diff --git a/jetty-http3/http3-qpack/src/test/java/org/eclipse/jetty/http3/qpack/TestDecoderHandler.java b/jetty-http3/http3-qpack/src/test/java/org/eclipse/jetty/http3/qpack/TestDecoderHandler.java index e7bee93da58..9d9213fef5f 100644 --- a/jetty-http3/http3-qpack/src/test/java/org/eclipse/jetty/http3/qpack/TestDecoderHandler.java +++ b/jetty-http3/http3-qpack/src/test/java/org/eclipse/jetty/http3/qpack/TestDecoderHandler.java @@ -23,6 +23,12 @@ public class TestDecoderHandler implements QpackDecoder.Handler { private final Queue _httpFieldsList = new LinkedList<>(); private final Queue _instructionList = new LinkedList<>(); + private QpackEncoder _encoder; + + public void setEncoder(QpackEncoder encoder) + { + _encoder = encoder; + } @Override public void onHttpFields(int streamId, HttpFields httpFields) @@ -31,9 +37,11 @@ public class TestDecoderHandler implements QpackDecoder.Handler } @Override - public void onInstruction(Instruction instruction) + public void onInstruction(Instruction instruction) throws QpackException { _instructionList.add(instruction); + if (_encoder != null) + _encoder.parseInstruction(QpackTestUtil.toBuffer(instruction)); } public HttpFields getHttpFields() diff --git a/jetty-http3/http3-qpack/src/test/java/org/eclipse/jetty/http3/qpack/TestEncoderHandler.java b/jetty-http3/http3-qpack/src/test/java/org/eclipse/jetty/http3/qpack/TestEncoderHandler.java index d5c9d1ffdde..bc1032b1a7b 100644 --- a/jetty-http3/http3-qpack/src/test/java/org/eclipse/jetty/http3/qpack/TestEncoderHandler.java +++ b/jetty-http3/http3-qpack/src/test/java/org/eclipse/jetty/http3/qpack/TestEncoderHandler.java @@ -21,11 +21,19 @@ import org.eclipse.jetty.http3.qpack.generator.Instruction; public class TestEncoderHandler implements QpackEncoder.Handler { private final Queue _instructionList = new LinkedList<>(); + private QpackDecoder _decoder; + + public void setDecoder(QpackDecoder decoder) + { + _decoder = decoder; + } @Override - public void onInstruction(Instruction instruction) + public void onInstruction(Instruction instruction) throws QpackException { _instructionList.add(instruction); + if (_decoder != null) + _decoder.parseInstruction(QpackTestUtil.toBuffer(instruction)); } public Instruction getInstruction()