Fixes #2722 - Improve configurability for SETTINGS frames. (#2723)

* Fixes #2722 - Improve configurability for SETTINGS frames.

Signed-off-by: Simone Bordet <simone.bordet@gmail.com>
This commit is contained in:
Simone Bordet 2018-07-18 10:07:14 +02:00 committed by GitHub
parent 3d5b769706
commit 9eca404da2
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
7 changed files with 145 additions and 11 deletions

View File

@ -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.SettingsFrame;
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 maxSettingsKeys = SettingsFrame.DEFAULT_MAX_KEYS;
private FlowControlStrategy.Factory flowControlStrategyFactory = () -> new BufferingFlowControlStrategy(0.5F);
@Override
@ -334,6 +336,17 @@ public class HTTP2Client extends ContainerLifeCycle
this.initialStreamRecvWindow = initialStreamRecvWindow;
}
@ManagedAttribute("The max number of keys in all SETTINGS frames")
public int getMaxSettingsKeys()
{
return maxSettingsKeys;
}
public void setMaxSettingsKeys(int maxSettingsKeys)
{
this.maxSettingsKeys = maxSettingsKeys;
}
public void connect(InetSocketAddress address, Session.Listener listener, Promise<Session> promise)
{
connect(null, address, listener, promise);

View File

@ -68,6 +68,7 @@ public class HTTP2ClientConnectionFactory implements ClientConnectionFactory
HTTP2ClientSession session = new HTTP2ClientSession(scheduler, endPoint, generator, listener, flowControl);
Parser parser = new Parser(byteBufferPool, session, 4096, 8192);
parser.setMaxSettingsKeys(client.getMaxSettingsKeys());
HTTP2ClientConnection connection = new HTTP2ClientConnection(client, byteBufferPool, executor, endPoint,
parser, session, client.getInputBufferSize(), promise, listener);

View File

@ -22,6 +22,8 @@ import java.util.Map;
public class SettingsFrame extends Frame
{
public static final int DEFAULT_MAX_KEYS = 64;
public static final int HEADER_TABLE_SIZE = 1;
public static final int ENABLE_PUSH = 2;
public static final int MAX_CONCURRENT_STREAMS = 3;

View File

@ -52,6 +52,7 @@ public class Parser
private final HeaderParser headerParser;
private final HeaderBlockParser headerBlockParser;
private final BodyParser[] bodyParsers;
private int maxSettingsKeys = SettingsFrame.DEFAULT_MAX_KEYS;
private boolean continuation;
private State state = State.HEADER;
@ -71,7 +72,7 @@ public class Parser
bodyParsers[FrameType.HEADERS.getType()] = new HeadersBodyParser(headerParser, listener, headerBlockParser, headerBlockFragments);
bodyParsers[FrameType.PRIORITY.getType()] = new PriorityBodyParser(headerParser, listener);
bodyParsers[FrameType.RST_STREAM.getType()] = new ResetBodyParser(headerParser, listener);
bodyParsers[FrameType.SETTINGS.getType()] = new SettingsBodyParser(headerParser, listener);
bodyParsers[FrameType.SETTINGS.getType()] = new SettingsBodyParser(headerParser, listener, getMaxSettingsKeys());
bodyParsers[FrameType.PUSH_PROMISE.getType()] = new PushPromiseBodyParser(headerParser, listener, headerBlockParser);
bodyParsers[FrameType.PING.getType()] = new PingBodyParser(headerParser, listener);
bodyParsers[FrameType.GO_AWAY.getType()] = new GoAwayBodyParser(headerParser, listener);
@ -203,6 +204,16 @@ public class Parser
return headerParser.hasFlag(bit);
}
public int getMaxSettingsKeys()
{
return maxSettingsKeys;
}
public void setMaxSettingsKeys(int maxSettingsKeys)
{
this.maxSettingsKeys = maxSettingsKeys;
}
protected void notifyConnectionFailure(int error, String reason)
{
try

View File

@ -32,16 +32,25 @@ import org.eclipse.jetty.util.log.Logger;
public class SettingsBodyParser extends BodyParser
{
private static final Logger LOG = Log.getLogger(SettingsBodyParser.class);
private final int maxKeys;
private State state = State.PREPARE;
private int cursor;
private int length;
private int settingId;
private int settingValue;
private int keys;
private Map<Integer, Integer> settings;
public SettingsBodyParser(HeaderParser headerParser, Parser.Listener listener)
{
this(headerParser, listener, SettingsFrame.DEFAULT_MAX_KEYS);
}
public SettingsBodyParser(HeaderParser headerParser, Parser.Listener listener, int maxKeys)
{
super(headerParser, listener);
this.maxKeys = maxKeys;
}
protected void reset()
@ -54,6 +63,11 @@ public class SettingsBodyParser extends BodyParser
settings = null;
}
public int getMaxKeys()
{
return maxKeys;
}
@Override
protected void emptyBody(ByteBuffer buffer)
{
@ -116,7 +130,8 @@ public class SettingsBodyParser extends BodyParser
settingValue = buffer.getInt();
if (LOG.isDebugEnabled())
LOG.debug(String.format("setting %d=%d",settingId, settingValue));
settings.put(settingId, settingValue);
if (!onSetting(buffer, settings, settingId, settingValue))
return false;
state = State.SETTING_ID;
length -= 4;
if (length == 0)
@ -142,7 +157,8 @@ public class SettingsBodyParser extends BodyParser
{
if (LOG.isDebugEnabled())
LOG.debug(String.format("setting %d=%d",settingId, settingValue));
settings.put(settingId, settingValue);
if (!onSetting(buffer, settings, settingId, settingValue))
return false;
state = State.SETTING_ID;
if (length == 0)
return onSettings(settings);
@ -158,6 +174,15 @@ public class SettingsBodyParser extends BodyParser
return false;
}
protected boolean onSetting(ByteBuffer buffer, Map<Integer, Integer> settings, int key, int value)
{
++keys;
if (keys > getMaxKeys())
return connectionFailure(buffer, ErrorCode.ENHANCE_YOUR_CALM_ERROR.code, "invalid_settings_frame");
settings.put(key, value);
return true;
}
protected boolean onSettings(Map<Integer, Integer> settings)
{
SettingsFrame frame = new SettingsFrame(settings, hasFlag(Flags.ACK));

View File

@ -41,9 +41,9 @@ public class SettingsGenerateParseTest
private final ByteBufferPool byteBufferPool = new MappedByteBufferPool();
@Test
public void testGenerateParseNoSettings() throws Exception
public void testGenerateParseNoSettings()
{
List<SettingsFrame> frames = testGenerateParse(Collections.<Integer, Integer>emptyMap());
List<SettingsFrame> frames = testGenerateParse(Collections.emptyMap());
Assert.assertEquals(1, frames.size());
SettingsFrame frame = frames.get(0);
Assert.assertEquals(0, frame.getSettings().size());
@ -51,7 +51,7 @@ public class SettingsGenerateParseTest
}
@Test
public void testGenerateParseSettings() throws Exception
public void testGenerateParseSettings()
{
Map<Integer, Integer> settings1 = new HashMap<>();
int key1 = 13;
@ -73,7 +73,7 @@ public class SettingsGenerateParseTest
{
SettingsGenerator generator = new SettingsGenerator(new HeaderGenerator());
final List<SettingsFrame> frames = new ArrayList<>();
List<SettingsFrame> frames = new ArrayList<>();
Parser parser = new Parser(byteBufferPool, new Parser.Listener.Adapter()
{
@Override
@ -104,11 +104,11 @@ public class SettingsGenerateParseTest
}
@Test
public void testGenerateParseInvalidSettings() throws Exception
public void testGenerateParseInvalidSettings()
{
SettingsGenerator generator = new SettingsGenerator(new HeaderGenerator());
final AtomicInteger errorRef = new AtomicInteger();
AtomicInteger errorRef = new AtomicInteger();
Parser parser = new Parser(byteBufferPool, new Parser.Listener.Adapter()
{
@Override
@ -139,11 +139,11 @@ public class SettingsGenerateParseTest
}
@Test
public void testGenerateParseOneByteAtATime() throws Exception
public void testGenerateParseOneByteAtATime()
{
SettingsGenerator generator = new SettingsGenerator(new HeaderGenerator());
final List<SettingsFrame> frames = new ArrayList<>();
List<SettingsFrame> frames = new ArrayList<>();
Parser parser = new Parser(byteBufferPool, new Parser.Listener.Adapter()
{
@Override
@ -182,4 +182,72 @@ public class SettingsGenerateParseTest
Assert.assertTrue(frame.isReply());
}
}
@Test
public void testGenerateParseTooManySettingsInOneFrame()
{
SettingsGenerator generator = new SettingsGenerator(new HeaderGenerator());
AtomicInteger errorRef = new AtomicInteger();
Parser parser = new Parser(byteBufferPool, new Parser.Listener.Adapter()
{
@Override
public void onConnectionFailure(int error, String reason)
{
errorRef.set(error);
}
}, 4096, 8192);
int maxSettingsKeys = 32;
parser.setMaxSettingsKeys(maxSettingsKeys);
parser.init(UnaryOperator.identity());
Map<Integer, Integer> settings = new HashMap<>();
for (int i = 0; i < maxSettingsKeys + 1; ++i)
settings.put(i + 10, i);
ByteBufferPool.Lease lease = new ByteBufferPool.Lease(byteBufferPool);
generator.generateSettings(lease, settings, false);
for (ByteBuffer buffer : lease.getByteBuffers())
{
while (buffer.hasRemaining())
parser.parse(buffer);
}
Assert.assertEquals(ErrorCode.ENHANCE_YOUR_CALM_ERROR.code, errorRef.get());
}
@Test
public void testGenerateParseTooManySettingsInMultipleFrames()
{
SettingsGenerator generator = new SettingsGenerator(new HeaderGenerator());
AtomicInteger errorRef = new AtomicInteger();
Parser parser = new Parser(byteBufferPool, new Parser.Listener.Adapter()
{
@Override
public void onConnectionFailure(int error, String reason)
{
errorRef.set(error);
}
}, 4096, 8192);
int maxSettingsKeys = 32;
parser.setMaxSettingsKeys(maxSettingsKeys);
parser.init(UnaryOperator.identity());
Map<Integer, Integer> settings = new HashMap<>();
settings.put(13, 17);
ByteBufferPool.Lease lease = new ByteBufferPool.Lease(byteBufferPool);
for (int i = 0; i < maxSettingsKeys + 1; ++i)
generator.generateSettings(lease, settings, false);
for (ByteBuffer buffer : lease.getByteBuffers())
{
while (buffer.hasRemaining())
parser.parse(buffer);
}
Assert.assertEquals(ErrorCode.ENHANCE_YOUR_CALM_ERROR.code, errorRef.get());
}
}

View File

@ -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 maxSettingsKeys = SettingsFrame.DEFAULT_MAX_KEYS;
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 number of keys in all SETTINGS frames")
public int getMaxSettingsKeys()
{
return maxSettingsKeys;
}
public void setMaxSettingsKeys(int maxSettingsKeys)
{
this.maxSettingsKeys = maxSettingsKeys;
}
/**
* @return -1
* @deprecated feature removed, no replacement
@ -205,6 +217,8 @@ public abstract class AbstractHTTP2ServerConnectionFactory extends AbstractConne
session.setWriteThreshold(getHttpConfiguration().getOutputBufferSize());
ServerParser parser = newServerParser(connector, session);
parser.setMaxSettingsKeys(getMaxSettingsKeys());
HTTP2Connection connection = new HTTP2ServerConnection(connector.getByteBufferPool(), connector.getExecutor(),
endPoint, httpConfiguration, parser, session, getInputBufferSize(), listener);
connection.addListener(connectionListener);