Fixes #11822 - HTTP/2 responses exceeding SETTINGS_MAX_HEADER_LIST_SIZE do not result in RST_STREAM or GOAWAY. (#12165)

Now HpackException.SessionException is treated specially in HTTP2Flusher.
It is caught, it fails all the entries, and then tries to send a GOAWAY, which will be the only frame allowed into the HTTP2FLusher at that point.

Signed-off-by: Simone Bordet <[email protected]>

Author: sbordet

jetty-core/jetty-http2/jetty-http2-client/src/main/java/org/eclipse/jetty/http2/client/HTTP2ClientConnectionFactory.java

@@ -95,8 +95,7 @@ private HTTP2ClientConnection(HTTP2Client client, EndPoint endpoint, HTTP2Client
         public void onOpen()
         {
             Map<Integer, Integer> settings = listener.onPreface(getSession());
-            if (settings == null)
-                settings = new HashMap<>();
+            settings = settings == null ? new HashMap<>() : new HashMap<>(settings);
 
             // Below we want to populate any settings to send to the server
             // that have a different default than what prescribed by the RFC.

jetty-core/jetty-http2/jetty-http2-common/src/main/java/org/eclipse/jetty/http2/HTTP2Session.java

@@ -1248,6 +1248,11 @@ protected Entry(Frame frame, HTTP2Stream stream, Callback callback)
             this.stream = stream;
         }
 
+        public Frame frame()
+        {
+            return frame;
+        }
+
         public abstract int getFrameBytesGenerated();
 
         public int getDataBytesRemaining()

jetty-core/jetty-http2/jetty-http2-common/src/main/java/org/eclipse/jetty/http2/internal/HTTP2Flusher.java

@@ -15,6 +15,7 @@
 
 import java.io.IOException;
 import java.nio.ByteBuffer;
+import java.nio.channels.ClosedChannelException;
 import java.util.ArrayDeque;
 import java.util.ArrayList;
 import java.util.Collection;
@@ -25,9 +26,11 @@
 import java.util.Queue;
 import java.util.Set;
 
+import org.eclipse.jetty.http2.ErrorCode;
 import org.eclipse.jetty.http2.FlowControlStrategy;
 import org.eclipse.jetty.http2.HTTP2Session;
 import org.eclipse.jetty.http2.HTTP2Stream;
+import org.eclipse.jetty.http2.frames.FrameType;
 import org.eclipse.jetty.http2.frames.WindowUpdateFrame;
 import org.eclipse.jetty.http2.hpack.HpackException;
 import org.eclipse.jetty.io.ByteBufferPool;
@@ -92,10 +95,9 @@ public boolean prepend(HTTP2Session.Entry entry)
                 entries.offerFirst(entry);
                 if (LOG.isDebugEnabled())
                     LOG.debug("Prepended {}, entries={}", entry, entries.size());
+                return true;
             }
         }
-        if (closed == null)
-            return true;
         closed(entry, closed);
         return false;
     }
@@ -106,15 +108,17 @@ public boolean append(HTTP2Session.Entry entry)
         try (AutoLock ignored = lock.lock())
         {
             closed = terminated;
+            // If it was not possible to HPACK encode, then allow to send a GOAWAY.
+            if (closed instanceof HpackException.SessionException && entry.frame().getType() == FrameType.GO_AWAY)
+                closed = null;
             if (closed == null)
             {
                 entries.offer(entry);
                 if (LOG.isDebugEnabled())
                     LOG.debug("Appended {}, entries={}, {}", entry, entries.size(), this);
+                return true;
             }
         }
-        if (closed == null)
-            return true;
         closed(entry, closed);
         return false;
     }
@@ -130,10 +134,9 @@ public boolean append(List<HTTP2Session.Entry> list)
                 list.forEach(entries::offer);
                 if (LOG.isDebugEnabled())
                     LOG.debug("Appended {}, entries={} {}", list, entries.size(), this);
+                return true;
             }
         }
-        if (closed == null)
-            return true;
         list.forEach(entry -> closed(entry, closed));
         return false;
     }
@@ -163,7 +166,21 @@ protected Action process() throws Throwable
         try (AutoLock ignored = lock.lock())
         {
             if (terminated != null)
-                throw terminated;
+            {
+                boolean rethrow = true;
+                if (terminated instanceof HpackException.SessionException)
+                 {
+                     HTTP2Session.Entry entry = entries.peek();
+                     if (entry != null && entry.frame().getType() == FrameType.GO_AWAY)
+                     {
+                         // Allow a SessionException to be processed once to send a GOAWAY.
+                         terminated = new ClosedChannelException().initCause(terminated);
+                         rethrow = false;
+                     }
+                 }
+                 if (rethrow)
+                     throw terminated;
+            }
 
             WindowEntry windowEntry;
             while ((windowEntry = windows.poll()) != null)
@@ -248,6 +265,15 @@ protected Action process() throws Throwable
                     entry.failed(failure);
                     pending.remove();
                 }
+                catch (HpackException.SessionException failure)
+                {
+                    if (LOG.isDebugEnabled())
+                        LOG.debug("Failure generating {}", entry, failure);
+                    onSessionFailure(failure);
+                    // The method above will try to send
+                    // a GOAWAY, so we will iterate again.
+                    return Action.IDLE;
+                }
                 catch (Throwable failure)
                 {
                     // Failure to generate the entry is catastrophic.
@@ -339,7 +365,23 @@ protected void onCompleteSuccess()
     protected void onCompleteFailure(Throwable x)
     {
         accumulator.release();
+        Throwable closed = fail(x);
+        // If the failure came from within the
+        // flusher, we need to close the connection.
+        if (closed == null)
+            session.onWriteFailure(x);
+    }
+
+    private void onSessionFailure(Throwable x)
+    {
+        accumulator.release();
+        Throwable closed = fail(x);
+        if (closed == null)
+            session.close(ErrorCode.COMPRESSION_ERROR.code, null, NOOP);
+    }
 
+    private Throwable fail(Throwable x)
+    {
         Throwable closed;
         Set<HTTP2Session.Entry> allEntries;
         try (AutoLock ignored = lock.lock())
@@ -361,11 +403,7 @@ protected void onCompleteFailure(Throwable x)
         allEntries.addAll(pendingEntries);
         pendingEntries.clear();
         allEntries.forEach(entry -> entry.failed(x));
-
-        // If the failure came from within the
-        // flusher, we need to close the connection.
-        if (closed == null)
-            session.onWriteFailure(x);
+        return closed;
     }
 
     public void terminate(Throwable cause)

jetty-core/jetty-http2/jetty-http2-tests/src/test/java/org/eclipse/jetty/http2/tests/HTTP2ServerTest.java

@@ -428,11 +428,20 @@ public boolean handle(Request request, Response response, Callback callback)
                 }
                 output.flush();
 
+                AtomicBoolean goAway = new AtomicBoolean();
                 Parser parser = new Parser(bufferPool, 8192);
-                parser.init(new Parser.Listener() {});
+                parser.init(new Parser.Listener()
+                {
+                    @Override
+                    public void onGoAway(GoAwayFrame frame)
+                    {
+                        goAway.set(true);
+                    }
+                });
                 boolean closed = parseResponse(client, parser);
 
-                assertTrue(closed);
+                assertFalse(closed);
+                assertTrue(goAway.get());
             }
         }
     }