HTTP/3 QPACK - do not expect section ack for zero required insert count

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

@@ -166,7 +166,8 @@ public boolean decode(long streamId, ByteBuffer buffer, Handler handler) throws
                 if (LOG.isDebugEnabled())
                     LOG.debug("Decoded: streamId={}, metadata={}", streamId, metaData);
                 _metaDataNotifications.add(new MetaDataNotification(streamId, metaData, handler));
-                _instructions.add(new SectionAcknowledgmentInstruction(streamId));
+                if (requiredInsertCount > 0)
+                    _instructions.add(new SectionAcknowledgmentInstruction(streamId));
             }
             else
             {
@@ -236,15 +237,17 @@ private void checkEncodedFieldSections() throws QpackException
         int insertCount = _context.getDynamicTable().getInsertCount();
         for (EncodedFieldSection encodedFieldSection : _encodedFieldSections)
         {
-            if (encodedFieldSection.getRequiredInsertCount() <= insertCount)
+            int requiredInsertCount = encodedFieldSection.getRequiredInsertCount();
+            if (requiredInsertCount <= insertCount)
             {
                 long streamId = encodedFieldSection.getStreamId();
                 MetaData metaData = encodedFieldSection.decode(_context, _maxHeaderSize);
                 if (LOG.isDebugEnabled())
                     LOG.debug("Decoded: streamId={}, metadata={}", streamId, metaData);
 
                 _metaDataNotifications.add(new MetaDataNotification(streamId, metaData, encodedFieldSection.getHandler()));
-                _instructions.add(new SectionAcknowledgmentInstruction(streamId));
+                if (requiredInsertCount > 0)
+                    _instructions.add(new SectionAcknowledgmentInstruction(streamId));
             }
         }
     }

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

@@ -193,7 +193,15 @@ public void encode(ByteBuffer buffer, long streamId, MetaData metadata) throws Q
                         requiredInsertCount = entryRequiredInsertCount;
                 }
 
+                // We should not expect section acknowledgements for 0 required insert count.
                 sectionInfo.setRequiredInsertCount(requiredInsertCount);
+                if (requiredInsertCount == 0)
+                {
+                    streamInfo.remove(sectionInfo);
+                    if (streamInfo.isEmpty())
+                        _streamInfoMap.remove(streamId);
+                }
+
                 int base = dynamicTable.getBase();
                 int encodedInsertCount = encodeInsertCount(requiredInsertCount, dynamicTable.getCapacity());
                 boolean signBit = base < requiredInsertCount;

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

@@ -16,6 +16,10 @@
 import java.nio.ByteBuffer;
 import java.util.List;
 
+import org.eclipse.jetty.http.HttpField;
+import org.eclipse.jetty.http.HttpFields;
+import org.eclipse.jetty.http.HttpVersion;
+import org.eclipse.jetty.http.MetaData;
 import org.eclipse.jetty.io.ByteBufferPool;
 import org.eclipse.jetty.io.NullByteBufferPool;
 import org.eclipse.jetty.util.BufferUtil;
@@ -27,6 +31,23 @@
 
 public class QpackTestUtil
 {
+    public static ByteBuffer toBuffer(Instruction... instructions)
+    {
+        ByteBufferPool.Lease lease = new ByteBufferPool.Lease(new NullByteBufferPool());
+        for (Instruction instruction : instructions)
+        {
+            instruction.encode(lease);
+        }
+        ByteBuffer combinedBuffer = BufferUtil.allocate(Math.toIntExact(lease.getTotalLength()));
+        BufferUtil.clearToFill(combinedBuffer);
+        for (ByteBuffer buffer : lease.getByteBuffers())
+        {
+            combinedBuffer.put(buffer);
+        }
+        BufferUtil.flipToFlush(combinedBuffer, 0);
+        return combinedBuffer;
+    }
+
     public static Matcher<String> equalsHex(String expectedString)
     {
         expectedString = expectedString.replaceAll("\\s+", "");
@@ -56,4 +77,43 @@ public static String toHexString(Instruction instruction)
     {
         return BufferUtil.toHexString(toBuffer(List.of(instruction)));
     }
+
+    public static ByteBuffer encode(QpackEncoder encoder, long streamId, MetaData metaData) throws QpackException
+    {
+        ByteBuffer buffer = BufferUtil.allocate(1024);
+        BufferUtil.clearToFill(buffer);
+        encoder.encode(buffer, streamId, metaData);
+        BufferUtil.flipToFlush(buffer, 0);
+        return buffer;
+    }
+
+    public static HttpFields.Mutable toHttpFields(HttpField field)
+    {
+        return HttpFields.build().add(field);
+    }
+
+    public static MetaData toMetaData(String name, String value)
+    {
+        return toMetaData(toHttpFields(new HttpField(name, value)));
+    }
+
+    public static MetaData toMetaData(String method, String path, String scheme)
+    {
+        return toMetaData(method, path, scheme, null);
+    }
+
+    public static MetaData toMetaData(String method, String path, String scheme, HttpFields.Mutable fields)
+    {
+        if (fields == null)
+            fields = HttpFields.build();
+        fields.put(":scheme", scheme);
+        fields.put(":method", method);
+        fields.put(":path", path);
+        return new MetaData(HttpVersion.HTTP_3, fields);
+    }
+
+    public static MetaData toMetaData(HttpFields fields)
+    {
+        return new MetaData(HttpVersion.HTTP_3, fields);
+    }
 }

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

@@ -0,0 +1,79 @@
+//
+// ========================================================================
+// Copyright (c) 1995-2022 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.internal.instruction.SectionAcknowledgmentInstruction;
+import org.eclipse.jetty.util.BufferUtil;
+import org.junit.jupiter.api.BeforeEach;
+import org.junit.jupiter.api.Test;
+
+import static org.eclipse.jetty.http3.qpack.QpackTestUtil.encode;
+import static org.eclipse.jetty.http3.qpack.QpackTestUtil.toBuffer;
+import static org.eclipse.jetty.http3.qpack.QpackTestUtil.toMetaData;
+import static org.hamcrest.MatcherAssert.assertThat;
+import static org.hamcrest.Matchers.containsString;
+import static org.junit.jupiter.api.Assertions.assertNull;
+import static org.junit.jupiter.api.Assertions.assertThrows;
+
+public class SectionAcknowledgmentTest
+{
+    private static final int MAX_BLOCKED_STREAMS = 5;
+    private static final int MAX_HEADER_SIZE = 1024;
+
+    private QpackEncoder _encoder;
+    private QpackDecoder _decoder;
+    private TestDecoderHandler _decoderHandler;
+    private TestEncoderHandler _encoderHandler;
+
+    @BeforeEach
+    public void before()
+    {
+        _encoderHandler = new TestEncoderHandler();
+        _decoderHandler = new TestDecoderHandler();
+        _encoder = new QpackEncoder(_encoderHandler, MAX_BLOCKED_STREAMS);
+        _decoder = new QpackDecoder(_decoderHandler, MAX_HEADER_SIZE);
+    }
+
+    @Test
+    public void testSectionAcknowledgmentForZeroRequiredInsertCountOnDecoder() throws Exception
+    {
+        // Encode a header with only a value contained in the static table.
+        ByteBuffer buffer = encode(_encoder, 0, toMetaData("GET", "/", "http"));
+
+        // No instruction since no addition to table.
+        Instruction instruction = _encoderHandler.getInstruction();
+        assertNull(instruction);
+
+        // Decoding should generate no instruction.
+        _decoder.decode(0, buffer, _decoderHandler);
+        instruction = _decoderHandler.getInstruction();
+        assertNull(instruction);
+    }
+
+    @Test
+    public void testSectionAcknowledgmentForZeroRequiredInsertCountOnEncoder() throws Exception
+    {
+        // Encode a header with only a value contained in the static table.
+        ByteBuffer buffer = encode(_encoder, 0, toMetaData("GET", "/", "http"));
+        System.err.println(BufferUtil.toDetailString(buffer));
+
+        // Parsing a section ack instruction on the encoder when we are not expecting it should result in QPACK_DECODER_STREAM_ERROR.
+        SectionAcknowledgmentInstruction instruction = new SectionAcknowledgmentInstruction(0);
+        ByteBuffer instructionBuffer = toBuffer(instruction);
+        QpackException error = assertThrows(QpackException.class, () -> _encoder.parseInstructions(instructionBuffer));
+        assertThat(error.getMessage(), containsString("No StreamInfo for 0"));
+    }
+}