From 71d4076ab2d7cf23f986a9666e1acea451f51016 Mon Sep 17 00:00:00 2001 From: Nerses Aznauryan <1935500+NersesAM@users.noreply.github.com> Date: Sat, 5 Aug 2023 11:35:53 +0400 Subject: [PATCH 1/2] Fix issue when server.max-http-request-header-size doesn't have effect on Netty server with http2 enabled. --- .../NettyWebServerFactoryCustomizer.java | 16 +++++----- .../NettyWebServerFactoryCustomizerTests.java | 30 +++++++++++++++---- 2 files changed, 33 insertions(+), 13 deletions(-) diff --git a/spring-boot-project/spring-boot-autoconfigure/src/main/java/org/springframework/boot/autoconfigure/web/embedded/NettyWebServerFactoryCustomizer.java b/spring-boot-project/spring-boot-autoconfigure/src/main/java/org/springframework/boot/autoconfigure/web/embedded/NettyWebServerFactoryCustomizer.java index c1d6ba684b8d..f4d90931dc5f 100644 --- a/spring-boot-project/spring-boot-autoconfigure/src/main/java/org/springframework/boot/autoconfigure/web/embedded/NettyWebServerFactoryCustomizer.java +++ b/spring-boot-project/spring-boot-autoconfigure/src/main/java/org/springframework/boot/autoconfigure/web/embedded/NettyWebServerFactoryCustomizer.java @@ -64,6 +64,8 @@ public void customize(NettyReactiveWebServerFactory factory) { map.from(nettyProperties::getIdleTimeout).to((idleTimeout) -> customizeIdleTimeout(factory, idleTimeout)); map.from(nettyProperties::getMaxKeepAliveRequests) .to((maxKeepAliveRequests) -> customizeMaxKeepAliveRequests(factory, maxKeepAliveRequests)); + map.from(this.serverProperties.getMaxHttpRequestHeaderSize()) + .to((maxHttpRequestHeaderSize) -> customizeHttp2MaxHeaderSize(factory, maxHttpRequestHeaderSize.toBytes())); customizeRequestDecoder(factory, map); } @@ -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() .to((maxHttpRequestHeader) -> httpRequestDecoderSpec .maxHeaderSize((int) maxHttpRequestHeader.toBytes())); ServerProperties.Netty nettyProperties = this.serverProperties.getNetty(); maxChunkSize(propertyMapper, httpRequestDecoderSpec, nettyProperties); propertyMapper.from(nettyProperties.getMaxInitialLineLength()) - .whenNonNull() .to((maxInitialLineLength) -> httpRequestDecoderSpec .maxInitialLineLength((int) maxInitialLineLength.toBytes())); propertyMapper.from(nettyProperties.getH2cMaxContentLength()) - .whenNonNull() .to((h2cMaxContentLength) -> httpRequestDecoderSpec .h2cMaxContentLength((int) h2cMaxContentLength.toBytes())); propertyMapper.from(nettyProperties.getInitialBufferSize()) - .whenNonNull() .to((initialBufferSize) -> httpRequestDecoderSpec.initialBufferSize((int) initialBufferSize.toBytes())); - propertyMapper.from(nettyProperties.isValidateHeaders()) - .whenNonNull() - .to(httpRequestDecoderSpec::validateHeaders); + propertyMapper.from(nettyProperties.isValidateHeaders()).to(httpRequestDecoderSpec::validateHeaders); return httpRequestDecoderSpec; })); } @@ -110,7 +106,6 @@ private void customizeRequestDecoder(NettyReactiveWebServerFactory factory, Prop private void maxChunkSize(PropertyMapper propertyMapper, HttpRequestDecoderSpec httpRequestDecoderSpec, ServerProperties.Netty nettyProperties) { propertyMapper.from(nettyProperties.getMaxChunkSize()) - .whenNonNull() .to((maxChunkSize) -> httpRequestDecoderSpec.maxChunkSize((int) maxChunkSize.toBytes())); } @@ -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( + (http2SettingsSpecBuilder) -> http2SettingsSpecBuilder.maxHeaderListSize(maxHttpRequestHeaderSize)))); + } + } diff --git a/spring-boot-project/spring-boot-autoconfigure/src/test/java/org/springframework/boot/autoconfigure/web/embedded/NettyWebServerFactoryCustomizerTests.java b/spring-boot-project/spring-boot-autoconfigure/src/test/java/org/springframework/boot/autoconfigure/web/embedded/NettyWebServerFactoryCustomizerTests.java index e2267a005a74..c83521c646d4 100644 --- a/spring-boot-project/spring-boot-autoconfigure/src/test/java/org/springframework/boot/autoconfigure/web/embedded/NettyWebServerFactoryCustomizerTests.java +++ b/spring-boot-project/spring-boot-autoconfigure/src/test/java/org/springframework/boot/autoconfigure/web/embedded/NettyWebServerFactoryCustomizerTests.java @@ -26,6 +26,7 @@ import org.mockito.ArgumentCaptor; import org.mockito.Captor; import org.mockito.junit.jupiter.MockitoExtension; +import reactor.netty.http.Http2SettingsSpec; import reactor.netty.http.server.HttpRequestDecoderSpec; import reactor.netty.http.server.HttpServer; @@ -126,9 +127,19 @@ void setMaxKeepAliveRequests() { verifyMaxKeepAliveRequests(factory, 100); } + @Test + void setHttp2MaxRequestHeaderSize() { + DataSize headerSize = DataSize.ofKilobytes(24); + this.serverProperties.setMaxHttpRequestHeaderSize(headerSize); + NettyReactiveWebServerFactory factory = mock(NettyReactiveWebServerFactory.class); + this.customizer.customize(factory); + verifyHttp2MaxHeaderSize(factory, headerSize.toBytes()); + } + @Test void configureHttpRequestDecoder() { ServerProperties.Netty nettyProperties = this.serverProperties.getNetty(); + this.serverProperties.setMaxHttpRequestHeaderSize(DataSize.ofKilobytes(24)); nettyProperties.setValidateHeaders(false); nettyProperties.setInitialBufferSize(DataSize.ofBytes(512)); nettyProperties.setH2cMaxContentLength(DataSize.ofKilobytes(1)); @@ -136,11 +147,12 @@ void configureHttpRequestDecoder() { nettyProperties.setMaxInitialLineLength(DataSize.ofKilobytes(32)); NettyReactiveWebServerFactory factory = mock(NettyReactiveWebServerFactory.class); this.customizer.customize(factory); - then(factory).should().addServerCustomizers(this.customizerCaptor.capture()); - NettyServerCustomizer serverCustomizer = this.customizerCaptor.getValue(); + then(factory).should(times(2)).addServerCustomizers(this.customizerCaptor.capture()); + NettyServerCustomizer serverCustomizer = this.customizerCaptor.getAllValues().get(1); HttpServer httpServer = serverCustomizer.apply(HttpServer.create()); HttpRequestDecoderSpec decoder = httpServer.configuration().decoder(); assertThat(decoder.validateHeaders()).isFalse(); + assertThat(decoder.maxHeaderSize()).isEqualTo(this.serverProperties.getMaxHttpRequestHeaderSize().toBytes()); assertThat(decoder.initialBufferSize()).isEqualTo(nettyProperties.getInitialBufferSize().toBytes()); assertThat(decoder.h2cMaxContentLength()).isEqualTo(nettyProperties.getH2cMaxContentLength().toBytes()); assertMaxChunkSize(nettyProperties, decoder); @@ -162,7 +174,7 @@ private void verifyConnectionTimeout(NettyReactiveWebServerFactory factory, Inte then(factory).should(never()).addServerCustomizers(any(NettyServerCustomizer.class)); return; } - then(factory).should(times(2)).addServerCustomizers(this.customizerCaptor.capture()); + then(factory).should(times(3)).addServerCustomizers(this.customizerCaptor.capture()); NettyServerCustomizer serverCustomizer = this.customizerCaptor.getAllValues().get(0); HttpServer httpServer = serverCustomizer.apply(HttpServer.create()); Map, ?> options = httpServer.configuration().options(); @@ -174,7 +186,7 @@ private void verifyIdleTimeout(NettyReactiveWebServerFactory factory, Duration e then(factory).should(never()).addServerCustomizers(any(NettyServerCustomizer.class)); return; } - then(factory).should(times(2)).addServerCustomizers(this.customizerCaptor.capture()); + then(factory).should(times(3)).addServerCustomizers(this.customizerCaptor.capture()); NettyServerCustomizer serverCustomizer = this.customizerCaptor.getAllValues().get(0); HttpServer httpServer = serverCustomizer.apply(HttpServer.create()); Duration idleTimeout = httpServer.configuration().idleTimeout(); @@ -182,11 +194,19 @@ private void verifyIdleTimeout(NettyReactiveWebServerFactory factory, Duration e } private void verifyMaxKeepAliveRequests(NettyReactiveWebServerFactory factory, int expected) { - then(factory).should(times(2)).addServerCustomizers(this.customizerCaptor.capture()); + then(factory).should(times(3)).addServerCustomizers(this.customizerCaptor.capture()); NettyServerCustomizer serverCustomizer = this.customizerCaptor.getAllValues().get(0); HttpServer httpServer = serverCustomizer.apply(HttpServer.create()); int maxKeepAliveRequests = httpServer.configuration().maxKeepAliveRequests(); assertThat(maxKeepAliveRequests).isEqualTo(expected); } + private void verifyHttp2MaxHeaderSize(NettyReactiveWebServerFactory factory, long expected) { + then(factory).should(times(2)).addServerCustomizers(this.customizerCaptor.capture()); + NettyServerCustomizer serverCustomizer = this.customizerCaptor.getAllValues().get(0); + HttpServer httpServer = serverCustomizer.apply(HttpServer.create()); + Http2SettingsSpec decoder = httpServer.configuration().http2SettingsSpec(); + assertThat(decoder.maxHeaderListSize()).isEqualTo(expected); + } + } From e3a9cda157850d2b802ee33bfb083156335c9dc3 Mon Sep 17 00:00:00 2001 From: Nerses Aznauryan <1935500+NersesAM@users.noreply.github.com> Date: Tue, 8 Aug 2023 22:25:05 +0400 Subject: [PATCH 2/2] Revert unrelated .whenNotNull() call removals. --- .../web/embedded/NettyWebServerFactoryCustomizer.java | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/spring-boot-project/spring-boot-autoconfigure/src/main/java/org/springframework/boot/autoconfigure/web/embedded/NettyWebServerFactoryCustomizer.java b/spring-boot-project/spring-boot-autoconfigure/src/main/java/org/springframework/boot/autoconfigure/web/embedded/NettyWebServerFactoryCustomizer.java index f4d90931dc5f..41c42ff01389 100644 --- a/spring-boot-project/spring-boot-autoconfigure/src/main/java/org/springframework/boot/autoconfigure/web/embedded/NettyWebServerFactoryCustomizer.java +++ b/spring-boot-project/spring-boot-autoconfigure/src/main/java/org/springframework/boot/autoconfigure/web/embedded/NettyWebServerFactoryCustomizer.java @@ -85,19 +85,25 @@ 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() .to((maxHttpRequestHeader) -> httpRequestDecoderSpec .maxHeaderSize((int) maxHttpRequestHeader.toBytes())); ServerProperties.Netty nettyProperties = this.serverProperties.getNetty(); maxChunkSize(propertyMapper, httpRequestDecoderSpec, nettyProperties); propertyMapper.from(nettyProperties.getMaxInitialLineLength()) + .whenNonNull() .to((maxInitialLineLength) -> httpRequestDecoderSpec .maxInitialLineLength((int) maxInitialLineLength.toBytes())); propertyMapper.from(nettyProperties.getH2cMaxContentLength()) + .whenNonNull() .to((h2cMaxContentLength) -> httpRequestDecoderSpec .h2cMaxContentLength((int) h2cMaxContentLength.toBytes())); propertyMapper.from(nettyProperties.getInitialBufferSize()) + .whenNonNull() .to((initialBufferSize) -> httpRequestDecoderSpec.initialBufferSize((int) initialBufferSize.toBytes())); - propertyMapper.from(nettyProperties.isValidateHeaders()).to(httpRequestDecoderSpec::validateHeaders); + propertyMapper.from(nettyProperties.isValidateHeaders()) + .whenNonNull() + .to(httpRequestDecoderSpec::validateHeaders); return httpRequestDecoderSpec; })); } @@ -106,6 +112,7 @@ private void customizeRequestDecoder(NettyReactiveWebServerFactory factory, Prop private void maxChunkSize(PropertyMapper propertyMapper, HttpRequestDecoderSpec httpRequestDecoderSpec, ServerProperties.Netty nettyProperties) { propertyMapper.from(nettyProperties.getMaxChunkSize()) + .whenNonNull() .to((maxChunkSize) -> httpRequestDecoderSpec.maxChunkSize((int) maxChunkSize.toBytes())); }