From 29a0af00a100742eba5c9c07d801b193070c0b7b Mon Sep 17 00:00:00 2001
From: Norman Maurer <norman_maurer@apple.com>
Date: Fri, 1 Dec 2023 10:07:57 +0100
Subject: [PATCH] QPACK: Allow stream cancellation even if the dynamic table
 was configured with max capacity of 0

Motivation:

It is fine to send stream cancellations even if the dynamic table had a capacity of 0 and so was disabled. We didn't follow the spec here and did case a NPE if we received a stream cancellation after the table was configured with capacity of 0.

Modifications:

- Allow stream cancellation if if the table capacity is 0
- Add unit test

Result:

Fixes https://github.com/netty/netty-incubator-codec-http3/issues/261 and https://github.com/netty/netty-incubator-codec-http3/issues/252
---
 .../incubator/codec/http3/QpackEncoder.java    |  7 ++++++-
 .../codec/http3/QpackDecoderHandlerTest.java   | 18 +++++++++++++++++-
 2 files changed, 23 insertions(+), 2 deletions(-)

diff --git a/src/main/java/io/netty/incubator/codec/http3/QpackEncoder.java b/src/main/java/io/netty/incubator/codec/http3/QpackEncoder.java
index 51b1771..f3cc179 100644
--- a/src/main/java/io/netty/incubator/codec/http3/QpackEncoder.java
+++ b/src/main/java/io/netty/incubator/codec/http3/QpackEncoder.java
@@ -163,7 +163,12 @@ void sectionAcknowledgment(long streamId) throws QpackException {
      * @param streamId which is cancelled.
      */
     void streamCancellation(long streamId) throws QpackException {
-        assert streamSectionTrackers != null;
+        // If a configureDynamicTable(...) was called with a maxTableCapacity of 0 we will have not instanced
+        // streamSectionTrackers. The remote peer might still send a stream cancellation for a stream, while it
+        // is optional. See https://www.rfc-editor.org/rfc/rfc9204.html#section-2.2.2.2
+        if (streamSectionTrackers == null) {
+            return;
+        }
         final Queue<Indices> tracker = streamSectionTrackers.remove(streamId);
         if (tracker != null) {
             for (;;) {
diff --git a/src/test/java/io/netty/incubator/codec/http3/QpackDecoderHandlerTest.java b/src/test/java/io/netty/incubator/codec/http3/QpackDecoderHandlerTest.java
index a14996c..5f10371 100644
--- a/src/test/java/io/netty/incubator/codec/http3/QpackDecoderHandlerTest.java
+++ b/src/test/java/io/netty/incubator/codec/http3/QpackDecoderHandlerTest.java
@@ -240,6 +240,18 @@ public void streamCancelUnknownStream() throws Exception {
         finishStreams();
     }
 
+    @Test
+    public void streamCancelDynamicTableWithMaxCapacity0() throws Exception {
+        setup(0);
+        encodeHeaders(headers -> headers.add(fooBar.name, fooBar.value));
+        verifyRequiredInsertCount(0);
+        verifyKnownReceivedCount(0);
+        // Send a stream cancellation for a dynamic table of capacity 0.
+        // See https://www.rfc-editor.org/rfc/rfc9204.html#section-2.2.2.2
+        sendStreamCancellation(decoderStream.streamId());
+        finishStreams(false);
+    }
+
     @Test
     public void invalidIncrement() throws Exception {
         setup(128);
@@ -325,8 +337,12 @@ private void setup(long maxTableCapacity) throws Exception {
     }
 
     private void finishStreams() {
+        finishStreams(true);
+    }
+
+    private void finishStreams(boolean encoderPendingMessage) {
         assertThat("Unexpected decoder stream message", decoderStream.finishAndReleaseAll(), is(false));
-        assertThat("Unexpected encoder stream message", encoderStream.finishAndReleaseAll(), is(true));
+        assertThat("Unexpected encoder stream message", encoderStream.finishAndReleaseAll(), is(encoderPendingMessage));
         assertThat("Unexpected parent stream message", parent.finishAndReleaseAll(), is(false));
     }