netty: include SETTINGS_MAX_HEADER_LIST_SIZE in SETTINGS

Closes #2350
This commit is contained in:
Łukasz Strzałkowski 2017-02-13 07:37:54 -08:00 committed by Carl Mastrangelo
parent 4029b3f0c2
commit 26913bb82e
5 changed files with 29 additions and 8 deletions

View File

@ -123,17 +123,19 @@ class NettyClientHandler extends AbstractNettyHandler {
Http2Connection connection = new DefaultHttp2Connection(false); Http2Connection connection = new DefaultHttp2Connection(false);
return newHandler(connection, frameReader, frameWriter, lifecycleManager, keepAliveManager, return newHandler(connection, frameReader, frameWriter, lifecycleManager, keepAliveManager,
flowControlWindow, ticker); flowControlWindow, maxHeaderListSize, ticker);
} }
@VisibleForTesting @VisibleForTesting
static NettyClientHandler newHandler(Http2Connection connection, Http2FrameReader frameReader, static NettyClientHandler newHandler(Http2Connection connection, Http2FrameReader frameReader,
Http2FrameWriter frameWriter, ClientTransportLifecycleManager lifecycleManager, Http2FrameWriter frameWriter, ClientTransportLifecycleManager lifecycleManager,
KeepAliveManager keepAliveManager, int flowControlWindow, Ticker ticker) { KeepAliveManager keepAliveManager, int flowControlWindow, int maxHeaderListSize,
Ticker ticker) {
Preconditions.checkNotNull(connection, "connection"); Preconditions.checkNotNull(connection, "connection");
Preconditions.checkNotNull(frameReader, "frameReader"); Preconditions.checkNotNull(frameReader, "frameReader");
Preconditions.checkNotNull(lifecycleManager, "lifecycleManager"); Preconditions.checkNotNull(lifecycleManager, "lifecycleManager");
Preconditions.checkArgument(flowControlWindow > 0, "flowControlWindow must be positive"); Preconditions.checkArgument(flowControlWindow > 0, "flowControlWindow must be positive");
Preconditions.checkArgument(maxHeaderListSize > 0, "maxHeaderListSize must be positive");
Preconditions.checkNotNull(ticker, "ticker"); Preconditions.checkNotNull(ticker, "ticker");
Http2FrameLogger frameLogger = new Http2FrameLogger(LogLevel.DEBUG, NettyClientHandler.class); Http2FrameLogger frameLogger = new Http2FrameLogger(LogLevel.DEBUG, NettyClientHandler.class);
@ -154,6 +156,7 @@ class NettyClientHandler extends AbstractNettyHandler {
settings.pushEnabled(false); settings.pushEnabled(false);
settings.initialWindowSize(flowControlWindow); settings.initialWindowSize(flowControlWindow);
settings.maxConcurrentStreams(0); settings.maxConcurrentStreams(0);
settings.maxHeaderListSize(maxHeaderListSize);
return new NettyClientHandler(decoder, encoder, settings, lifecycleManager, keepAliveManager, return new NettyClientHandler(decoder, encoder, settings, lifecycleManager, keepAliveManager,
ticker); ticker);

View File

@ -113,7 +113,7 @@ class NettyServerHandler extends AbstractNettyHandler {
Http2FrameWriter frameWriter = Http2FrameWriter frameWriter =
new Http2OutboundFrameLogger(new DefaultHttp2FrameWriter(), frameLogger); new Http2OutboundFrameLogger(new DefaultHttp2FrameWriter(), frameLogger);
return newHandler(frameReader, frameWriter, transportListener, maxStreams, flowControlWindow, return newHandler(frameReader, frameWriter, transportListener, maxStreams, flowControlWindow,
maxMessageSize); maxHeaderListSize, maxMessageSize);
} }
@VisibleForTesting @VisibleForTesting
@ -121,9 +121,11 @@ class NettyServerHandler extends AbstractNettyHandler {
ServerTransportListener transportListener, ServerTransportListener transportListener,
int maxStreams, int maxStreams,
int flowControlWindow, int flowControlWindow,
int maxHeaderListSize,
int maxMessageSize) { int maxMessageSize) {
Preconditions.checkArgument(maxStreams > 0, "maxStreams must be positive"); Preconditions.checkArgument(maxStreams > 0, "maxStreams must be positive");
Preconditions.checkArgument(flowControlWindow > 0, "flowControlWindow must be positive"); Preconditions.checkArgument(flowControlWindow > 0, "flowControlWindow must be positive");
Preconditions.checkArgument(maxHeaderListSize > 0, "maxHeaderListSize must be positive");
Preconditions.checkArgument(maxMessageSize > 0, "maxMessageSize must be positive"); Preconditions.checkArgument(maxMessageSize > 0, "maxMessageSize must be positive");
Http2Connection connection = new DefaultHttp2Connection(true); Http2Connection connection = new DefaultHttp2Connection(true);
@ -140,6 +142,7 @@ class NettyServerHandler extends AbstractNettyHandler {
Http2Settings settings = new Http2Settings(); Http2Settings settings = new Http2Settings();
settings.initialWindowSize(flowControlWindow); settings.initialWindowSize(flowControlWindow);
settings.maxConcurrentStreams(maxStreams); settings.maxConcurrentStreams(maxStreams);
settings.maxHeaderListSize(maxHeaderListSize);
return new NettyServerHandler(transportListener, decoder, encoder, settings, maxMessageSize); return new NettyServerHandler(transportListener, decoder, encoder, settings, maxMessageSize);
} }

View File

@ -109,6 +109,7 @@ public class NettyClientHandlerTest extends NettyHandlerTestBase<NettyClientHand
private Http2Headers grpcHeaders; private Http2Headers grpcHeaders;
private long nanoTime; // backs a ticker, for testing ping round-trip time measurement private long nanoTime; // backs a ticker, for testing ping round-trip time measurement
private int flowControlWindow = DEFAULT_WINDOW_SIZE; private int flowControlWindow = DEFAULT_WINDOW_SIZE;
private int maxHeaderListSize = Integer.MAX_VALUE;
private int streamId = 3; private int streamId = 3;
private ClientTransportLifecycleManager lifecycleManager; private ClientTransportLifecycleManager lifecycleManager;
private KeepAliveManager mockKeepAliveManager = null; private KeepAliveManager mockKeepAliveManager = null;
@ -574,7 +575,7 @@ public class NettyClientHandlerTest extends NettyHandlerTestBase<NettyClientHand
}; };
return NettyClientHandler.newHandler(connection, frameReader(), frameWriter(), return NettyClientHandler.newHandler(connection, frameReader(), frameWriter(),
lifecycleManager, mockKeepAliveManager, flowControlWindow, ticker); lifecycleManager, mockKeepAliveManager, flowControlWindow, maxHeaderListSize, ticker);
} }
@Override @Override

View File

@ -70,7 +70,6 @@ import io.netty.channel.nio.NioEventLoopGroup;
import io.netty.channel.socket.SocketChannelConfig; import io.netty.channel.socket.SocketChannelConfig;
import io.netty.channel.socket.nio.NioServerSocketChannel; import io.netty.channel.socket.nio.NioServerSocketChannel;
import io.netty.channel.socket.nio.NioSocketChannel; import io.netty.channel.socket.nio.NioSocketChannel;
import io.netty.handler.codec.http2.Http2Exception;
import io.netty.handler.codec.http2.StreamBufferingEncoder; import io.netty.handler.codec.http2.StreamBufferingEncoder;
import io.netty.handler.ssl.SslContext; import io.netty.handler.ssl.SslContext;
import io.netty.handler.ssl.SupportedCipherSuiteFilter; import io.netty.handler.ssl.SupportedCipherSuiteFilter;
@ -290,8 +289,10 @@ public class NettyClientTransportTest {
+ " size limit!"); + " size limit!");
} catch (Exception e) { } catch (Exception e) {
Throwable rootCause = getRootCause(e); Throwable rootCause = getRootCause(e);
assertTrue(rootCause instanceof Http2Exception); Status status = ((StatusException) rootCause).getStatus();
assertEquals("Header size exceeded max allowed size (1)", rootCause.getMessage()); assertEquals(Status.Code.INTERNAL, status.getCode());
assertEquals("HTTP/2 error code: PROTOCOL_ERROR\nReceived Rst Stream",
status.getDescription());
} }
} }

View File

@ -106,6 +106,7 @@ public class NettyServerHandlerTest extends NettyHandlerTestBase<NettyServerHand
private int flowControlWindow = DEFAULT_WINDOW_SIZE; private int flowControlWindow = DEFAULT_WINDOW_SIZE;
private int maxConcurrentStreams = Integer.MAX_VALUE; private int maxConcurrentStreams = Integer.MAX_VALUE;
private int maxHeaderListSize = Integer.MAX_VALUE;
private class ServerTransportListenerImpl implements ServerTransportListener { private class ServerTransportListenerImpl implements ServerTransportListener {
@ -275,6 +276,18 @@ public class NettyServerHandlerTest extends NettyHandlerTestBase<NettyServerHand
assertEquals(maxConcurrentStreams, captor.getValue().maxConcurrentStreams().intValue()); assertEquals(maxConcurrentStreams, captor.getValue().maxConcurrentStreams().intValue());
} }
@Test
public void shouldAdvertiseMaxHeaderListSize() throws Exception {
maxHeaderListSize = 123;
setUp();
ArgumentCaptor<Http2Settings> captor = ArgumentCaptor.forClass(Http2Settings.class);
verifyWrite().writeSettings(
any(ChannelHandlerContext.class), captor.capture(), any(ChannelPromise.class));
assertEquals(maxHeaderListSize, captor.getValue().maxHeaderListSize().intValue());
}
@Test @Test
@Ignore("Re-enable once https://github.com/grpc/grpc-java/issues/1175 is fixed") @Ignore("Re-enable once https://github.com/grpc/grpc-java/issues/1175 is fixed")
public void connectionWindowShouldBeOverridden() throws Exception { public void connectionWindowShouldBeOverridden() throws Exception {
@ -357,7 +370,7 @@ public class NettyServerHandlerTest extends NettyHandlerTestBase<NettyServerHand
@Override @Override
protected NettyServerHandler newHandler() { protected NettyServerHandler newHandler() {
return NettyServerHandler.newHandler(frameReader(), frameWriter(), transportListener, return NettyServerHandler.newHandler(frameReader(), frameWriter(), transportListener,
maxConcurrentStreams, flowControlWindow, DEFAULT_MAX_MESSAGE_SIZE); maxConcurrentStreams, flowControlWindow, maxHeaderListSize, DEFAULT_MAX_MESSAGE_SIZE);
} }
@Override @Override