Issue #2679 - HTTP/2 Spec Compliance.

Fixed max frame length handling.

Signed-off-by: Simone Bordet <simone.bordet@gmail.com>

jetty-http2/http2-client/src/main/java/org/eclipse/jetty/http2/client/HTTP2Client.java

@@ -34,6 +34,7 @@ import org.eclipse.jetty.alpn.client.ALPNClientConnectionFactory;
 import org.eclipse.jetty.http2.BufferingFlowControlStrategy;
 import org.eclipse.jetty.http2.FlowControlStrategy;
 import org.eclipse.jetty.http2.api.Session;
+import org.eclipse.jetty.http2.frames.Frame;
 import org.eclipse.jetty.io.ByteBufferPool;
 import org.eclipse.jetty.io.ClientConnectionFactory;
 import org.eclipse.jetty.io.Connection;
@@ -129,6 +130,7 @@ public class HTTP2Client extends ContainerLifeCycle
     private List<String> protocols = Arrays.asList("h2", "h2-17", "h2-16", "h2-15", "h2-14");
     private int initialSessionRecvWindow = 16 * 1024 * 1024;
     private int initialStreamRecvWindow = 8 * 1024 * 1024;
+    private int maxFrameLength = Frame.DEFAULT_MAX_LENGTH;
     private FlowControlStrategy.Factory flowControlStrategyFactory = () -> new BufferingFlowControlStrategy(0.5F);
 
     @Override
@@ -334,6 +336,17 @@ public class HTTP2Client extends ContainerLifeCycle
         this.initialStreamRecvWindow = initialStreamRecvWindow;
     }
 
+    @ManagedAttribute("The max frame length in bytes")
+    public int getMaxFrameLength()
+    {
+        return maxFrameLength;
+    }
+
+    public void setMaxFrameLength(int maxFrameLength)
+    {
+        this.maxFrameLength = maxFrameLength;
+    }
+
     public void connect(InetSocketAddress address, Session.Listener listener, Promise<Session> promise)
     {
         connect(null, address, listener, promise);

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

@@ -67,6 +67,7 @@ public class HTTP2ClientConnectionFactory implements ClientConnectionFactory
         FlowControlStrategy flowControl = client.getFlowControlStrategyFactory().newFlowControlStrategy();
         HTTP2ClientSession session = new HTTP2ClientSession(scheduler, endPoint, generator, listener, flowControl);
         Parser parser = new Parser(byteBufferPool, session, 4096, 8192);
+        parser.setMaxFrameLength(client.getMaxFrameLength());
 
         HTTP2ClientConnection connection = new HTTP2ClientConnection(client, byteBufferPool, executor, endPoint,
                 parser, session, client.getInputBufferSize(), promise, listener);
@@ -110,6 +111,10 @@ public class HTTP2ClientConnectionFactory implements ClientConnectionFactory
                 settings = new HashMap<>();
             settings.computeIfAbsent(SettingsFrame.INITIAL_WINDOW_SIZE, k -> client.getInitialStreamRecvWindow());
 
+            Integer maxFrameLength = settings.get(SettingsFrame.MAX_FRAME_SIZE);
+            if (maxFrameLength != null)
+                getParser().setMaxFrameLength(maxFrameLength);
+
             PrefaceFrame prefaceFrame = new PrefaceFrame();
             SettingsFrame settingsFrame = new SettingsFrame(settings, false);
 

jetty-http2/http2-common/src/main/java/org/eclipse/jetty/http2/parser/HeaderParser.java

@@ -21,6 +21,7 @@ package org.eclipse.jetty.http2.parser;
 import java.nio.ByteBuffer;
 
 import org.eclipse.jetty.http2.frames.Frame;
+import org.eclipse.jetty.http2.frames.FrameType;
 
 /**
  * <p>The parser for the frame header of HTTP/2 frames.</p>
@@ -144,6 +145,12 @@ public class HeaderParser
         return streamId;
     }
 
+    @Override
+    public String toString()
+    {
+        return String.format("[%s|%d|%d|%d]", FrameType.from(getFrameType()), getLength(), flags, getStreamId());
+    }
+
     private enum State
     {
         LENGTH, TYPE, FLAGS, STREAM_ID, STREAM_ID_BYTES

jetty-http2/http2-common/src/main/java/org/eclipse/jetty/http2/parser/Parser.java

@@ -24,6 +24,7 @@ import java.util.function.UnaryOperator;
 import org.eclipse.jetty.http2.ErrorCode;
 import org.eclipse.jetty.http2.Flags;
 import org.eclipse.jetty.http2.frames.DataFrame;
+import org.eclipse.jetty.http2.frames.Frame;
 import org.eclipse.jetty.http2.frames.FrameType;
 import org.eclipse.jetty.http2.frames.GoAwayFrame;
 import org.eclipse.jetty.http2.frames.HeadersFrame;
@@ -53,6 +54,7 @@ public class Parser
     private final HeaderBlockParser headerBlockParser;
     private final BodyParser[] bodyParsers;
     private final UnknownBodyParser unknownBodyParser;
+    private int maxFrameLength;
     private boolean continuation;
     private State state = State.HEADER;
 
@@ -62,6 +64,7 @@ public class Parser
         this.headerParser = new HeaderParser();
         this.headerBlockParser = new HeaderBlockParser(byteBufferPool, new HpackDecoder(maxDynamicTableSize, maxHeaderSize));
         this.unknownBodyParser = new UnknownBodyParser(headerParser, listener);
+        this.maxFrameLength = Frame.DEFAULT_MAX_LENGTH;
         this.bodyParsers = new BodyParser[FrameType.values().length];
     }
 
@@ -139,10 +142,13 @@ public class Parser
         if (!headerParser.parse(buffer))
             return false;
 
-        FrameType frameType = FrameType.from(getFrameType());
         if (LOG.isDebugEnabled())
-            LOG.debug("Parsed {} frame header from {}", frameType, buffer);
+            LOG.debug("Parsed {} frame header from {}", headerParser, buffer);
 
+        if (headerParser.getLength() > getMaxFrameLength())
+            return handleFrameTooLarge(buffer);
+
+        FrameType frameType = FrameType.from(getFrameType());
         if (continuation)
         {
             if (frameType != FrameType.CONTINUATION)
@@ -169,6 +175,13 @@ public class Parser
         return true;
     }
 
+    private boolean handleFrameTooLarge(ByteBuffer buffer)
+    {
+        BufferUtil.clear(buffer);
+        notifyConnectionFailure(ErrorCode.FRAME_SIZE_ERROR.code, "invalid_frame_length");
+        return false;
+    }
+
     protected boolean parseBody(ByteBuffer buffer)
     {
         int type = getFrameType();
@@ -209,6 +222,16 @@ public class Parser
         return headerParser.hasFlag(bit);
     }
 
+    public int getMaxFrameLength()
+    {
+        return maxFrameLength;
+    }
+
+    public void setMaxFrameLength(int maxFrameLength)
+    {
+        this.maxFrameLength = maxFrameLength;
+    }
+
     protected void notifyConnectionFailure(int error, String reason)
     {
         try

jetty-http2/http2-common/src/test/java/org/eclipse/jetty/http2/frames/MaxFrameSizeParseTest.java

@@ -0,0 +1,66 @@
+//
+//  ========================================================================
+//  Copyright (c) 1995-2018 Mort Bay Consulting Pty. Ltd.
+//  ------------------------------------------------------------------------
+//  All rights reserved. This program and the accompanying materials
+//  are made available under the terms of the Eclipse Public License v1.0
+//  and Apache License v2.0 which accompanies this distribution.
+//
+//      The Eclipse Public License is available at
+//      http://www.eclipse.org/legal/epl-v10.html
+//
+//      The Apache License v2.0 is available at
+//      http://www.opensource.org/licenses/apache2.0.php
+//
+//  You may elect to redistribute this code under either of these licenses.
+//  ========================================================================
+//
+
+package org.eclipse.jetty.http2.frames;
+
+import java.nio.ByteBuffer;
+import java.util.concurrent.atomic.AtomicInteger;
+import java.util.function.UnaryOperator;
+
+import org.eclipse.jetty.http2.ErrorCode;
+import org.eclipse.jetty.http2.parser.Parser;
+import org.eclipse.jetty.io.ByteBufferPool;
+import org.eclipse.jetty.io.MappedByteBufferPool;
+import org.junit.Assert;
+import org.junit.Test;
+
+public class MaxFrameSizeParseTest
+{
+    private final ByteBufferPool byteBufferPool = new MappedByteBufferPool();
+
+    @Test
+    public void testMaxFrameSize()
+    {
+        int maxFrameLength = Frame.DEFAULT_MAX_LENGTH + 16;
+
+        AtomicInteger failure = new AtomicInteger();
+        Parser parser = new Parser(byteBufferPool, new Parser.Listener.Adapter()
+        {
+            @Override
+            public void onConnectionFailure(int error, String reason)
+            {
+                failure.set(error);
+            }
+        }, 4096, 8192);
+        parser.setMaxFrameLength(maxFrameLength);
+        parser.init(UnaryOperator.identity());
+
+        // Iterate a few times to be sure the parser is properly reset.
+        for (int i = 0; i < 2; ++i)
+        {
+            byte[] bytes = new byte[]{0, 0, 0, 0, 0, 0, 0, 0, 0, 1, 0};
+            ByteBuffer buffer = ByteBuffer.wrap(bytes);
+            buffer.putInt(0, maxFrameLength + 1);
+            buffer.position(1);
+            while (buffer.hasRemaining())
+                parser.parse(buffer);
+        }
+
+        Assert.assertEquals(ErrorCode.FRAME_SIZE_ERROR.code, failure.get());
+    }
+}

jetty-http2/http2-server/src/main/java/org/eclipse/jetty/http2/server/AbstractHTTP2ServerConnectionFactory.java

@@ -50,6 +50,7 @@ public abstract class AbstractHTTP2ServerConnectionFactory extends AbstractConne
     private int initialStreamRecvWindow = 512 * 1024;
     private int maxConcurrentStreams = 128;
     private int maxHeaderBlockFragment = 0;
+    private int maxFrameLength = Frame.DEFAULT_MAX_LENGTH;
     private FlowControlStrategy.Factory flowControlStrategyFactory = () -> new BufferingFlowControlStrategy(0.5F);
     private long streamIdleTimeout;
 
@@ -145,6 +146,17 @@ public abstract class AbstractHTTP2ServerConnectionFactory extends AbstractConne
         this.streamIdleTimeout = streamIdleTimeout;
     }
 
+    @ManagedAttribute("The max frame length in bytes")
+    public int getMaxFrameLength()
+    {
+        return maxFrameLength;
+    }
+
+    public void setMaxFrameLength(int maxFrameLength)
+    {
+        this.maxFrameLength = maxFrameLength;
+    }
+
     /**
      * @return -1
      * @deprecated feature removed, no replacement
@@ -205,6 +217,7 @@ public abstract class AbstractHTTP2ServerConnectionFactory extends AbstractConne
         session.setWriteThreshold(getHttpConfiguration().getOutputBufferSize());
 
         ServerParser parser = newServerParser(connector, session);
+        parser.setMaxFrameLength(getMaxFrameLength());
         HTTP2Connection connection = new HTTP2ServerConnection(connector.getByteBufferPool(), connector.getExecutor(),
                         endPoint, httpConfiguration, parser, session, getInputBufferSize(), listener);
         connection.addListener(connectionListener);