server.max-http-request-header-size doesn't affect Netty server with http2 enabled #36766
Conversation
…t on Netty server with http2 enabled.
|
@NersesAM Please sign the Contributor License Agreement! Click here to manually synchronize the status of this Pull Request. See the FAQ for frequently asked questions. |
|
@NersesAM Thank you for signing the Contributor License Agreement! |
spring-projects-issues
added
the
status: waiting-for-triage
|
CONTRIBUTING file is not stating against which branch should I raise PR so I have chosen the current latest release branch. Do I need to create PRs against main, 3.0, 2.7(?) as well? |
| @@ -83,25 +85,19 @@ private void customizeConnectionTimeout(NettyReactiveWebServerFactory factory, D | |||
| private void customizeRequestDecoder(NettyReactiveWebServerFactory factory, PropertyMapper propertyMapper) { | |||
| factory.addServerCustomizers((httpServer) -> httpServer.httpRequestDecoder((httpRequestDecoderSpec) -> { | |||
| propertyMapper.from(this.serverProperties.getMaxHttpRequestHeaderSize()) | |||
| .whenNonNull() | |||
There was a problem hiding this comment.
Removed these as well in this method because PropertyMapper is already initialised with .alwaysApplyingWhenNonNull() so these are redundant
There was a problem hiding this comment.
Thanks but please revert those as they are unrelated.
|
@NersesAM No need to create additional PRs. We can apply it to the appropriate branch once we've decided the earliest branch to apply the change to, then forward-merge to newer branches. |
| @@ -83,25 +85,19 @@ private void customizeConnectionTimeout(NettyReactiveWebServerFactory factory, D | |||
| private void customizeRequestDecoder(NettyReactiveWebServerFactory factory, PropertyMapper propertyMapper) { | |||
| factory.addServerCustomizers((httpServer) -> httpServer.httpRequestDecoder((httpRequestDecoderSpec) -> { | |||
| propertyMapper.from(this.serverProperties.getMaxHttpRequestHeaderSize()) | |||
| .whenNonNull() | |||
There was a problem hiding this comment.
Thanks but please revert those as they are unrelated.
| @@ -122,4 +117,9 @@ private void customizeMaxKeepAliveRequests(NettyReactiveWebServerFactory factory | |||
| factory.addServerCustomizers((httpServer) -> httpServer.maxKeepAliveRequests(maxKeepAliveRequests)); | |||
| } | |||
|
|
|||
| private void customizeHttp2MaxHeaderSize(NettyReactiveWebServerFactory factory, long maxHttpRequestHeaderSize) { | |||
| factory.addServerCustomizers(((httpServer) -> httpServer.http2Settings( | |||
There was a problem hiding this comment.
This looks odd to me. This is the first HTTP2-specific setting that I can see. And it being applied regardless of http2 being enabled doesn't look right.
I wonder why using HTTP2 requires to set this setting twice as we're already taking care of that in configuring HttpRequestDecoderSpec.
There was a problem hiding this comment.
http1.1 uses different netty codec. Compare this stacktrace with the one above and their settings are separate
io.netty.handler.codec.http.TooLongHttpHeaderException: HTTP header is larger than 8192 bytes.
at io.netty.handler.codec.http.HttpObjectDecoder$HeaderParser.newException(HttpObjectDecoder.java:1091)
at io.netty.handler.codec.http.HttpObjectDecoder$HeaderParser.parse(HttpObjectDecoder.java:1058)
at io.netty.handler.codec.http.HttpObjectDecoder.readHeaders(HttpObjectDecoder.java:656)
at io.netty.handler.codec.http.HttpObjectDecoder.decode(HttpObjectDecoder.java:285)
at io.netty.handler.codec.http.HttpServerCodec$HttpServerRequestDecoder.decode(HttpServerCodec.java:140)
at io.netty.handler.codec.ByteToMessageDecoder.decodeRemovalReentryProtection(ByteToMessageDecoder.java:529)
at io.netty.handler.codec.ByteToMessageDecoder.callDecode(ByteToMessageDecoder.java:468)
at io.netty.handler.codec.ByteToMessageDecoder.channelRead(ByteToMessageDecoder.java:290)
at io.netty.channel.CombinedChannelDuplexHandler.channelRead(CombinedChannelDuplexHandler.java:251)
at io.netty.channel.AbstractChannelHandlerContext.invokeChannelRead(AbstractChannelHandlerContext.java:442)
at io.netty.channel.AbstractChannelHandlerContext.invokeChannelRead(AbstractChannelHandlerContext.java:420)
at io.netty.channel.AbstractChannelHandlerContext.fireChannelRead(AbstractChannelHandlerContext.java:412)
at io.netty.channel.DefaultChannelPipeline$HeadContext.channelRead(DefaultChannelPipeline.java:1410)
at io.netty.channel.AbstractChannelHandlerContext.invokeChannelRead(AbstractChannelHandlerContext.java:440)
at io.netty.channel.AbstractChannelHandlerContext.invokeChannelRead(AbstractChannelHandlerContext.java:420)
at io.netty.channel.DefaultChannelPipeline.fireChannelRead(DefaultChannelPipeline.java:919)
at io.netty.channel.nio.AbstractNioByteChannel$NioByteUnsafe.read(AbstractNioByteChannel.java:166)
at io.netty.channel.nio.NioEventLoop.processSelectedKey(NioEventLoop.java:788)
at io.netty.channel.nio.NioEventLoop.processSelectedKeysOptimized(NioEventLoop.java:724)
at io.netty.channel.nio.NioEventLoop.processSelectedKeys(NioEventLoop.java:650)
at io.netty.channel.nio.NioEventLoop.run(NioEventLoop.java:562)
at io.netty.util.concurrent.SingleThreadEventExecutor$4.run(SingleThreadEventExecutor.java:997)
at io.netty.util.internal.ThreadExecutorMap$2.run(ThreadExecutorMap.java:74)
at io.netty.util.concurrent.FastThreadLocalRunnable.run(FastThreadLocalRunnable.java:30)
at java.base/java.lang.Thread.run(Thread.java:833)
here is where in reactor-netty-http sets the http2 settings into netty
https://github.com/reactor/reactor-netty/blob/99800186555bf874c4a4fdf6abf3e4b2da4fe0ee/reactor-netty-http/src/main/java/reactor/netty/http/server/HttpServerConfig.java#L430C7-L430C7
snicoll
added
the
status: waiting-for-feedback
snicoll
added
the
for: team-attention
philwebb
added
type: bug
mhalbritter
removed
status: on-hold
mhalbritter
changed the title
Fix an issue that server.max-http-request-header-size doesn't have an effect on Netty server with http2 enabled. See gh-36766
|
Thank you very much! I polished your changes in this commit: dc62e5f |
Setting
server.max-http-request-header-sizedoesn't work when http2 is enabled on Netty server.I have created a small repro project https://github.com/NersesAM/netty-http2-bug
Steps to reproduce:
Enabling Debug logging we can see following, at info level there are no logs printed
Same curl will work if the curl request is done with http 1.1.
This can be fixed with a custom NettyCustomizer uncomment
@Componentto validate it fixes the problem. But this is unexpected behaviour as the max header size setting should apply regardless which http protocol is used. So I suggest to fix it in Spring boot with this PR.