Rewrite QpackEncoder.encode for HttpField, implement Duplicates.

Signed-off-by: Lachlan Roberts <lachlan@webtide.com>

jetty-http3/http3-qpack/src/main/java/org/eclipse/jetty/http3/qpack/QpackEncoder.java

@@ -28,6 +28,7 @@ import org.eclipse.jetty.http.HttpHeader;
 import org.eclipse.jetty.http.HttpMethod;
 import org.eclipse.jetty.http.HttpStatus;
 import org.eclipse.jetty.http.PreEncodedHttpField;
+import org.eclipse.jetty.http3.qpack.generator.DuplicateInstruction;
 import org.eclipse.jetty.http3.qpack.generator.IndexedNameEntryInstruction;
 import org.eclipse.jetty.http3.qpack.generator.Instruction;
 import org.eclipse.jetty.http3.qpack.generator.LiteralNameEntryInstruction;
@@ -105,6 +106,7 @@ public class QpackEncoder
     private final int _maxBlockedStreams;
     private final Map<Integer, AtomicInteger> _blockedStreams = new HashMap<>();
     private boolean _validateEncoding = true;
+    private int _knownInsertCount;
 
     public QpackEncoder(Handler handler, int maxBlockedStreams)
     {
@@ -117,6 +119,7 @@ public class QpackEncoder
         _bufferPool = bufferPool;
         _context = new QpackContext();
         _maxBlockedStreams = maxBlockedStreams;
+        _knownInsertCount = 0;
     }
 
     private boolean acquireBlockedStream(int streamId)
@@ -151,6 +154,21 @@ public class QpackEncoder
         _handler.onInstruction(new SetCapacityInstruction(capacity));
     }
 
+    public void insertCountIncrement(int increment)
+    {
+        _knownInsertCount += increment;
+    }
+
+    public void sectionAcknowledgement(int streamId)
+    {
+        // TODO: Implement.
+    }
+
+    public void streamCancellation(int streamId)
+    {
+        // TODO: Implement.
+    }
+
     public QpackContext getQpackContext()
     {
         return _context;
@@ -166,6 +184,31 @@ public class QpackEncoder
         _validateEncoding = validateEncoding;
     }
 
+    public boolean referenceEntry(Entry entry, int streamId)
+    {
+        if (entry == null)
+            return false;
+
+        if (entry.isStatic())
+            return true;
+
+        boolean inEvictionZone = !_context.getDynamicTable().canReference(entry);
+        if (inEvictionZone)
+            return false;
+
+        if (_knownInsertCount >= entry.getIndex() + 1)
+        {
+            entry.reference();
+            return true;
+        }
+
+        // We might be able to risk blocking the decoder stream and reference this immediately.
+        boolean riskBlockedStream = acquireBlockedStream(streamId);
+        if (riskBlockedStream)
+            entry.reference();
+        return riskBlockedStream;
+    }
+
     public static boolean shouldIndex(HttpField httpField)
     {
         return !DO_NOT_INDEX.contains(httpField.getHeader());
@@ -241,45 +284,62 @@ public class QpackEncoder
         // TODO:
         //  1. The field.getHeader() could be null.
         //  3. Handle pre-encoded HttpFields.
-        //  4. Someone still needs to generate the HTTP/3 pseudo headers. (this should be independent of HTTP/3 though?)
+
+        boolean canCreateEntry = shouldIndex(field) && (Entry.getSize(field) <= dynamicTable.getSpace());
 
         Entry entry = _context.get(field);
-        if (entry != null && _context.canReference(entry))
+        if (entry != null && referenceEntry(entry, streamId))
         {
-            // TODO: we may want to duplicate the entry if it is in the eviction zone?
-            //  then we would also need to reference this entry, is that okay?
-            entry.reference();
             return new EncodableEntry(entry);
         }
-
+        else
-        Entry nameEntry = _context.get(field.getName());
-        boolean canReferenceName = nameEntry != null && _context.canReference(nameEntry);
-
-        Entry newEntry = new Entry(field);
-        if (shouldIndex(field) && (newEntry.getSize() <= dynamicTable.getSpace()))
         {
-            dynamicTable.add(newEntry);
+            // Should we duplicate this entry.
-
+            if (entry != null && dynamicTable.canReference(entry) && canCreateEntry)
-            boolean huffman = shouldHuffmanEncode(field);
-            if (canReferenceName)
             {
-                boolean isDynamic = !nameEntry.isStatic();
+                Entry newEntry = new Entry(field);
-                int nameIndex = _context.index(nameEntry);
+                dynamicTable.add(newEntry);
-                _handler.onInstruction(new IndexedNameEntryInstruction(isDynamic, nameIndex, huffman, field.getValue()));
+                _handler.onInstruction(new DuplicateInstruction(entry.getIndex()));
-            }
-            else
-            {
-                _handler.onInstruction(new LiteralNameEntryInstruction(huffman, field.getName(), huffman, field.getValue()));
-            }
 
-            // We might be able to risk blocking the decoder stream and reference this immediately.
+                // Should we reference this entry and risk blocking.
-            if (acquireBlockedStream(streamId))
+                if (referenceEntry(newEntry, streamId))
-                return new EncodableEntry(newEntry);
+                    return new EncodableEntry(newEntry);
+            }
         }
 
-        if (canReferenceName)
+        boolean huffman = shouldHuffmanEncode(field);
+        Entry nameEntry = _context.get(field.getName());
+        if (nameEntry != null && referenceEntry(nameEntry, streamId))
+        {
+            // Should we copy this entry
+            if (canCreateEntry)
+            {
+                Entry newEntry = new Entry(field);
+                dynamicTable.add(newEntry);
+                _handler.onInstruction(new IndexedNameEntryInstruction(!nameEntry.isStatic(), nameEntry.getIndex(), huffman, field.getValue()));
+
+                // Should we reference this entry and risk blocking.
+                if (referenceEntry(newEntry, streamId))
+                    return new EncodableEntry(newEntry);
+            }
+
             return new EncodableEntry(nameEntry, field);
-        return new EncodableEntry(field);
+        }
+        else
+        {
+            if (canCreateEntry)
+            {
+                Entry newEntry = new Entry(field);
+                dynamicTable.add(newEntry);
+                _handler.onInstruction(new LiteralNameEntryInstruction(huffman, field.getName(), huffman, field.getValue()));
+
+                // Should we reference this entry and risk blocking.
+                if (referenceEntry(newEntry, streamId))
+                    return new EncodableEntry(newEntry);
+            }
+
+            return new EncodableEntry(field);
+        }
     }
 
     public static int encodeInsertCount(int reqInsertCount, int maxTableCapacity)

jetty-http3/http3-qpack/src/main/java/org/eclipse/jetty/http3/qpack/parser/DecoderInstructionParser.java

@@ -15,6 +15,8 @@ package org.eclipse.jetty.http3.qpack.parser;
 
 import java.nio.ByteBuffer;
 
+import org.eclipse.jetty.http3.qpack.QpackEncoder;
+
 /**
  * Receives instructions coming from the remote Encoder as a sequence of unframed instructions.
  */
@@ -45,6 +47,39 @@ public class DecoderInstructionParser
         void onInsertCountIncrement(int increment);
     }
 
+    public static class EncoderHandler implements Handler
+    {
+        private final QpackEncoder _encoder;
+
+        public EncoderHandler(QpackEncoder encoder)
+        {
+            _encoder = encoder;
+        }
+
+        @Override
+        public void onSectionAcknowledgement(int streamId)
+        {
+            _encoder.sectionAcknowledgement(streamId);
+        }
+
+        @Override
+        public void onStreamCancellation(int streamId)
+        {
+            _encoder.streamCancellation(streamId);
+        }
+
+        @Override
+        public void onInsertCountIncrement(int increment)
+        {
+            _encoder.insertCountIncrement(increment);
+        }
+    }
+
+    public DecoderInstructionParser(QpackEncoder encoder)
+    {
+        this(new EncoderHandler(encoder));
+    }
+
     public DecoderInstructionParser(Handler handler)
     {
         _handler = handler;

jetty-http3/http3-qpack/src/main/java/org/eclipse/jetty/http3/qpack/parser/EncoderInstructionParser.java

@@ -100,7 +100,6 @@ public class EncoderInstructionParser
         this(new DecoderHandler(decoder));
     }
 
-
     public EncoderInstructionParser(Handler handler)
     {
         _handler = handler;

jetty-http3/http3-qpack/src/main/java/org/eclipse/jetty/http3/qpack/table/Entry.java

@@ -42,7 +42,7 @@ public class Entry
 
     public int getSize()
     {
-        return 32 + StringUtil.getLength(_field.getName()) + StringUtil.getLength(_field.getValue());
+        return getSize(_field);
     }
 
     public void setIndex(int index)
@@ -85,4 +85,9 @@ public class Entry
     {
         return String.format("{%s,%d,%s,%x}", isStatic() ? "S" : "D", _absoluteIndex, _field, hashCode());
     }
+
+    public static int getSize(HttpField field)
+    {
+        return 32 + StringUtil.getLength(field.getName()) + StringUtil.getLength(field.getValue());
+    }
 }

jetty-http3/http3-qpack/src/test/java/org/eclipse/jetty/http3/qpack/DecoderInstructionParserTest.java

@@ -20,6 +20,7 @@ import java.util.Queue;
 import org.eclipse.jetty.http3.qpack.parser.DecoderInstructionParser;
 import org.eclipse.jetty.util.BufferUtil;
 import org.eclipse.jetty.util.TypeUtil;
+import org.junit.jupiter.api.Disabled;
 import org.junit.jupiter.api.Test;
 
 import static org.hamcrest.MatcherAssert.assertThat;
@@ -114,6 +115,7 @@ public class DecoderInstructionParserTest
         }
     }
 
+    @Disabled
     @Test
     public void testStreamCancellation() throws Exception
     {
@@ -121,6 +123,7 @@ public class DecoderInstructionParserTest
         throw new RuntimeException("TODO: testStreamCancellation");
     }
 
+    @Disabled
     @Test
     public void testInsertCountIncrement() throws Exception
     {

jetty-http3/http3-qpack/src/test/java/org/eclipse/jetty/http3/qpack/EncodeDecodeTest.java

@@ -47,8 +47,8 @@ public class EncodeDecodeTest
     private DecoderInstructionParser _decoderInstructionParser;
     private EncoderInstructionParser _encoderInstructionParser;
 
-    private final int MAX_BLOCKED_STREAMS = 5;
+    private static final int MAX_BLOCKED_STREAMS = 5;
-    private final int MAX_HEADER_SIZE = 1024;
+    private static final int MAX_HEADER_SIZE = 1024;
 
     @BeforeEach
     public void before()
@@ -58,6 +58,7 @@ public class EncodeDecodeTest
         _encoder = new QpackEncoder(_encoderHandler, MAX_BLOCKED_STREAMS);
         _decoder = new QpackDecoder(_decoderHandler, MAX_HEADER_SIZE);
         _encoderInstructionParser = new EncoderInstructionParser(_decoder);
+        _decoderInstructionParser = new DecoderInstructionParser(_encoder);
     }
 
     @Test

jetty-http3/http3-qpack/src/test/java/org/eclipse/jetty/http3/qpack/NBitIntegerParserTest.java

@@ -41,7 +41,6 @@ public class NBitIntegerParserTest
         int value = parser.decode(buffer1);
         assertThat(value, is(-1));
 
-        parser.setPrefix(7);
         value = parser.decode(buffer2);
         assertThat(value, is(1337));
     }