Issue #9554 - additional cleanups and testing

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

jetty-http2/http2-hpack/src/main/java/org/eclipse/jetty/http2/hpack/MetaDataBuilder.java

@@ -290,10 +290,13 @@ public class MetaDataBuilder
      */
     public void checkSize(int length, boolean huffman) throws SessionException
     {
+        if (length < 0)
+            throw new IllegalArgumentException();
+
         // Apply a huffman fudge factor
         if (huffman)
-            length = (length * 4) / 3;
+            length = Math.multiplyExact(length, 4) / 3;
-        if ((_size + length) > _maxSize)
+        if (Math.addExact(_size, length) > _maxSize)
             throw new SessionException("Header too large %d > %d", _size + length, _maxSize);
     }
 }

jetty-http3/http3-qpack/src/main/java/org/eclipse/jetty/http3/qpack/internal/EncodableEntry.java

@@ -182,9 +182,26 @@ public abstract class EncodableEntry
         public int getRequiredSize(int base)
         {
             String value = getValue();
-            int relativeIndex =  _nameEntry.getIndex() - base;
             int valueLength = _huffman ? HuffmanEncoder.octetsNeeded(value) : value.length();
-            return 1 + NBitIntegerEncoder.octetsNeeded(4, relativeIndex) + 1 + NBitIntegerEncoder.octetsNeeded(7, valueLength) + valueLength;
+
+            int nameOctets;
+            if (_nameEntry.isStatic())
+            {
+                int relativeIndex =  _nameEntry.getIndex();
+                nameOctets = NBitIntegerEncoder.octetsNeeded(4, relativeIndex);
+            }
+            else if (_nameEntry.getIndex() < base)
+            {
+                int relativeIndex = base - (_nameEntry.getIndex() + 1);
+                nameOctets = NBitIntegerEncoder.octetsNeeded(4, relativeIndex);
+            }
+            else
+            {
+                int relativeIndex = _nameEntry.getIndex() - base;
+                nameOctets = NBitIntegerEncoder.octetsNeeded(3, relativeIndex);
+            }
+
+            return 1 + nameOctets + 1 + NBitIntegerEncoder.octetsNeeded(7, valueLength) + valueLength;
         }
 
         @Override

jetty-http3/http3-qpack/src/main/java/org/eclipse/jetty/http3/qpack/internal/instruction/LiteralNameEntryInstruction.java

@@ -82,7 +82,7 @@ public class LiteralNameEntryInstruction implements Instruction
         else
         {
             buffer.put((byte)(0x00));
-            NBitIntegerEncoder.encode(buffer, 5, _value.length());
+            NBitIntegerEncoder.encode(buffer, 7, _value.length());
             buffer.put(_value.getBytes(StandardCharsets.ISO_8859_1));
         }
 

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

@@ -14,18 +14,26 @@
 package org.eclipse.jetty.http3.qpack;
 
 import java.nio.ByteBuffer;
+import java.util.List;
 
+import org.eclipse.jetty.http3.qpack.internal.instruction.DuplicateInstruction;
+import org.eclipse.jetty.http3.qpack.internal.instruction.IndexedNameEntryInstruction;
+import org.eclipse.jetty.http3.qpack.internal.instruction.SetCapacityInstruction;
 import org.eclipse.jetty.http3.qpack.internal.parser.DecoderInstructionParser;
+import org.eclipse.jetty.io.ByteBufferPool;
+import org.eclipse.jetty.io.NullByteBufferPool;
 import org.junit.jupiter.api.BeforeEach;
 import org.junit.jupiter.api.Test;
 
 import static org.hamcrest.MatcherAssert.assertThat;
+import static org.hamcrest.Matchers.equalTo;
 import static org.hamcrest.Matchers.is;
 import static org.junit.jupiter.api.Assertions.assertNotNull;
 import static org.junit.jupiter.api.Assertions.assertTrue;
 
 public class DecoderInstructionParserTest
 {
+    private final NullByteBufferPool bufferPool = new NullByteBufferPool();
     private DecoderInstructionParser _instructionParser;
     private DecoderParserDebugHandler _handler;
 
@@ -41,6 +49,11 @@ public class DecoderInstructionParserTest
     {
         // Set Dynamic Table Capacity=220.
         ByteBuffer buffer = QpackTestUtil.hexToBuffer("3fbd 01");
+
+        // Assert that our generated value is equal to that of the spec example.
+        ByteBuffer encodedValue = getEncodedValue(new SetCapacityInstruction(220));
+        assertThat(buffer, equalTo(encodedValue));
+
         _instructionParser.parse(buffer);
         assertThat(_handler.setCapacities.poll(), is(220));
         assertTrue(_handler.isEmpty());
@@ -51,6 +64,11 @@ public class DecoderInstructionParserTest
     {
         // Duplicate (Relative Index = 2).
         ByteBuffer buffer = QpackTestUtil.hexToBuffer("02");
+
+        // Assert that our generated value is equal to that of the spec example.
+        ByteBuffer encodedValue = getEncodedValue(new DuplicateInstruction(2));
+        assertThat(buffer, equalTo(encodedValue));
+
         _instructionParser.parse(buffer);
         assertThat(_handler.duplicates.poll(), is(2));
         assertTrue(_handler.isEmpty());
@@ -61,6 +79,11 @@ public class DecoderInstructionParserTest
     {
         // Insert With Name Reference to Static Table, Index=0 (:authority=www.example.com).
         ByteBuffer buffer = QpackTestUtil.hexToBuffer("c00f 7777 772e 6578 616d 706c 652e 636f 6d");
+
+        // Assert that our generated value is equal to that of the spec example.
+        ByteBuffer encodedValue = getEncodedValue(new IndexedNameEntryInstruction(false, 0, false, "www.example.com"));
+        assertThat(buffer, equalTo(encodedValue));
+
         _instructionParser.parse(buffer);
         DecoderParserDebugHandler.ReferencedEntry entry = _handler.referencedNameEntries.poll();
         assertNotNull(entry);
@@ -94,4 +117,13 @@ public class DecoderInstructionParserTest
         // There are no other instructions received.
         assertTrue(_handler.isEmpty());
     }
+
+    private ByteBuffer getEncodedValue(Instruction instruction)
+    {
+        ByteBufferPool.Lease lease = new ByteBufferPool.Lease(bufferPool);
+        instruction.encode(lease);
+        List<ByteBuffer> byteBuffers = lease.getByteBuffers();
+        assertThat(byteBuffers.size(), equalTo(1));
+        return byteBuffers.get(0);
+    }
 }