From 8b40ce14cb808ec0b5cdd6767ce8dac42df88e7c Mon Sep 17 00:00:00 2001 From: Bryan Turner Date: Sun, 28 Oct 2018 14:43:18 -0700 Subject: [PATCH 1/2] Restore max-http-header-size default value support Fix `TomcatWebServerFactoryCustomizer` to restore Spring Boot 2.0 behavior where a negative or zero `max-http-header-size` indicates that the server default should be used. See gh-14986 --- .../TomcatWebServerFactoryCustomizer.java | 2 +- .../TomcatWebServerFactoryCustomizerTests.java | 16 ++++++++++++++++ 2 files changed, 17 insertions(+), 1 deletion(-) diff --git a/spring-boot-project/spring-boot-autoconfigure/src/main/java/org/springframework/boot/autoconfigure/web/embedded/TomcatWebServerFactoryCustomizer.java b/spring-boot-project/spring-boot-autoconfigure/src/main/java/org/springframework/boot/autoconfigure/web/embedded/TomcatWebServerFactoryCustomizer.java index 18b756f6df5..bc5a438df15 100644 --- a/spring-boot-project/spring-boot-autoconfigure/src/main/java/org/springframework/boot/autoconfigure/web/embedded/TomcatWebServerFactoryCustomizer.java +++ b/spring-boot-project/spring-boot-autoconfigure/src/main/java/org/springframework/boot/autoconfigure/web/embedded/TomcatWebServerFactoryCustomizer.java @@ -86,7 +86,7 @@ public class TomcatWebServerFactoryCustomizer implements propertyMapper.from(tomcatProperties::getMinSpareThreads).when(this::isPositive) .to((minSpareThreads) -> customizeMinThreads(factory, minSpareThreads)); propertyMapper.from(this::determineMaxHttpHeaderSize).whenNonNull() - .asInt(DataSize::toBytes) + .asInt(DataSize::toBytes).when(this::isPositive) .to((maxHttpHeaderSize) -> customizeMaxHttpHeaderSize(factory, maxHttpHeaderSize)); propertyMapper.from(tomcatProperties::getMaxSwallowSize).whenNonNull() diff --git a/spring-boot-project/spring-boot-autoconfigure/src/test/java/org/springframework/boot/autoconfigure/web/embedded/TomcatWebServerFactoryCustomizerTests.java b/spring-boot-project/spring-boot-autoconfigure/src/test/java/org/springframework/boot/autoconfigure/web/embedded/TomcatWebServerFactoryCustomizerTests.java index 1ddbc689a0c..6f8083fa8c6 100644 --- a/spring-boot-project/spring-boot-autoconfigure/src/test/java/org/springframework/boot/autoconfigure/web/embedded/TomcatWebServerFactoryCustomizerTests.java +++ b/spring-boot-project/spring-boot-autoconfigure/src/test/java/org/springframework/boot/autoconfigure/web/embedded/TomcatWebServerFactoryCustomizerTests.java @@ -129,6 +129,22 @@ public class TomcatWebServerFactoryCustomizerTests { .isEqualTo(DataSize.ofKilobytes(1).toBytes())); } + @Test + public void customMaxHttpHeaderSizeIgnoredIfNegative() { + bind("server.max-http-header-size=-1"); + customizeAndRunServer((server) -> assertThat(((AbstractHttp11Protocol) server + .getTomcat().getConnector().getProtocolHandler()).getMaxHttpHeaderSize()) + .isEqualTo(DataSize.ofKilobytes(8).toBytes())); + } + + @Test + public void customMaxHttpHeaderSizeIgnoredIfZero() { + bind("server.max-http-header-size=0"); + customizeAndRunServer((server) -> assertThat(((AbstractHttp11Protocol) server + .getTomcat().getConnector().getProtocolHandler()).getMaxHttpHeaderSize()) + .isEqualTo(DataSize.ofKilobytes(8).toBytes())); + } + @Test @Deprecated public void customMaxHttpHeaderSizeWithDeprecatedProperty() { From 1451c0c0697274f0456ee3cbc41876628990eb6c Mon Sep 17 00:00:00 2001 From: Phillip Webb Date: Sun, 28 Oct 2018 18:24:46 -0700 Subject: [PATCH 2/2] Polish "Restore max-http-header-size default value support" Fix Jetty and Undertow customizers to restore Spring Boot 2.0 behavior where a negative or zero `max-http-header-size` indicates that the server default should be used. Closes gh-14986 --- .../JettyWebServerFactoryCustomizer.java | 2 +- .../UndertowWebServerFactoryCustomizer.java | 2 +- .../JettyWebServerFactoryCustomizerTests.java | 32 +++++++++- ...dertowWebServerFactoryCustomizerTests.java | 58 +++++++++++++++++++ 4 files changed, 90 insertions(+), 4 deletions(-) diff --git a/spring-boot-project/spring-boot-autoconfigure/src/main/java/org/springframework/boot/autoconfigure/web/embedded/JettyWebServerFactoryCustomizer.java b/spring-boot-project/spring-boot-autoconfigure/src/main/java/org/springframework/boot/autoconfigure/web/embedded/JettyWebServerFactoryCustomizer.java index 85a649ecb4e..6f96a46cdcb 100644 --- a/spring-boot-project/spring-boot-autoconfigure/src/main/java/org/springframework/boot/autoconfigure/web/embedded/JettyWebServerFactoryCustomizer.java +++ b/spring-boot-project/spring-boot-autoconfigure/src/main/java/org/springframework/boot/autoconfigure/web/embedded/JettyWebServerFactoryCustomizer.java @@ -75,7 +75,7 @@ public class JettyWebServerFactoryCustomizer implements propertyMapper.from(jettyProperties::getSelectors).whenNonNull() .to(factory::setSelectors); propertyMapper.from(properties::getMaxHttpHeaderSize).whenNonNull() - .asInt(DataSize::toBytes) + .asInt(DataSize::toBytes).when(this::isPositive) .to((maxHttpHeaderSize) -> customizeMaxHttpHeaderSize(factory, maxHttpHeaderSize)); propertyMapper.from(jettyProperties::getMaxHttpPostSize).asInt(DataSize::toBytes) diff --git a/spring-boot-project/spring-boot-autoconfigure/src/main/java/org/springframework/boot/autoconfigure/web/embedded/UndertowWebServerFactoryCustomizer.java b/spring-boot-project/spring-boot-autoconfigure/src/main/java/org/springframework/boot/autoconfigure/web/embedded/UndertowWebServerFactoryCustomizer.java index 2151cae4f6c..312bcfb82ee 100644 --- a/spring-boot-project/spring-boot-autoconfigure/src/main/java/org/springframework/boot/autoconfigure/web/embedded/UndertowWebServerFactoryCustomizer.java +++ b/spring-boot-project/spring-boot-autoconfigure/src/main/java/org/springframework/boot/autoconfigure/web/embedded/UndertowWebServerFactoryCustomizer.java @@ -86,7 +86,7 @@ public class UndertowWebServerFactoryCustomizer implements propertyMapper.from(this::getOrDeduceUseForwardHeaders) .to(factory::setUseForwardHeaders); propertyMapper.from(properties::getMaxHttpHeaderSize).whenNonNull() - .asInt(DataSize::toBytes) + .asInt(DataSize::toBytes).when(this::isPositive) .to((maxHttpHeaderSize) -> customizeMaxHttpHeaderSize(factory, maxHttpHeaderSize)); propertyMapper.from(undertowProperties::getMaxHttpPostSize) diff --git a/spring-boot-project/spring-boot-autoconfigure/src/test/java/org/springframework/boot/autoconfigure/web/embedded/JettyWebServerFactoryCustomizerTests.java b/spring-boot-project/spring-boot-autoconfigure/src/test/java/org/springframework/boot/autoconfigure/web/embedded/JettyWebServerFactoryCustomizerTests.java index a81312e5df8..4f6d1f052a0 100644 --- a/spring-boot-project/spring-boot-autoconfigure/src/test/java/org/springframework/boot/autoconfigure/web/embedded/JettyWebServerFactoryCustomizerTests.java +++ b/spring-boot-project/spring-boot-autoconfigure/src/test/java/org/springframework/boot/autoconfigure/web/embedded/JettyWebServerFactoryCustomizerTests.java @@ -18,6 +18,8 @@ package org.springframework.boot.autoconfigure.web.embedded; import java.io.File; import java.io.IOException; +import java.util.ArrayList; +import java.util.List; import java.util.Locale; import java.util.TimeZone; @@ -38,6 +40,7 @@ import org.springframework.boot.web.embedded.jetty.JettyServletWebServerFactory; import org.springframework.boot.web.embedded.jetty.JettyWebServer; import org.springframework.mock.env.MockEnvironment; import org.springframework.test.context.support.TestPropertySourceUtils; +import org.springframework.test.util.ReflectionTestUtils; import static org.assertj.core.api.Assertions.assertThat; import static org.mockito.Mockito.mock; @@ -147,15 +150,40 @@ public class JettyWebServerFactoryCustomizerTests { public void customizeMaxHttpHeaderSize() { bind("server.max-http-header-size=2048"); JettyWebServer server = customizeAndGetServer(); - for (Connector connector : server.getServer().getConnectors()) { + List requestHeaderSizes = getRequestHeaderSizes(server); + assertThat(requestHeaderSizes).containsOnly(2048); + } + + @Test + public void customMaxHttpHeaderSizeIgnoredIfNegative() { + bind("server.max-http-header-size=-1"); + JettyWebServer server = customizeAndGetServer(); + List requestHeaderSizes = getRequestHeaderSizes(server); + assertThat(requestHeaderSizes).containsOnly(8192); + } + + @Test + public void customMaxHttpHeaderSizeIgnoredIfZero() { + bind("server.max-http-header-size=0"); + JettyWebServer server = customizeAndGetServer(); + List requestHeaderSizes = getRequestHeaderSizes(server); + assertThat(requestHeaderSizes).containsOnly(8192); + } + + private List getRequestHeaderSizes(JettyWebServer server) { + List requestHeaderSizes = new ArrayList<>(); + Connector[] connectors = (Connector[]) ReflectionTestUtils.getField(server, + "connectors"); + for (Connector connector : connectors) { connector.getConnectionFactories().stream() .filter((factory) -> factory instanceof ConnectionFactory) .forEach((cf) -> { ConnectionFactory factory = (ConnectionFactory) cf; HttpConfiguration configuration = factory.getHttpConfiguration(); - assertThat(configuration.getRequestHeaderSize()).isEqualTo(2048); + requestHeaderSizes.add(configuration.getRequestHeaderSize()); }); } + return requestHeaderSizes; } private void bind(String... inlinedProperties) { diff --git a/spring-boot-project/spring-boot-autoconfigure/src/test/java/org/springframework/boot/autoconfigure/web/embedded/UndertowWebServerFactoryCustomizerTests.java b/spring-boot-project/spring-boot-autoconfigure/src/test/java/org/springframework/boot/autoconfigure/web/embedded/UndertowWebServerFactoryCustomizerTests.java index 33d0121f44f..401370dde0c 100644 --- a/spring-boot-project/spring-boot-autoconfigure/src/test/java/org/springframework/boot/autoconfigure/web/embedded/UndertowWebServerFactoryCustomizerTests.java +++ b/spring-boot-project/spring-boot-autoconfigure/src/test/java/org/springframework/boot/autoconfigure/web/embedded/UndertowWebServerFactoryCustomizerTests.java @@ -17,18 +17,28 @@ package org.springframework.boot.autoconfigure.web.embedded; import java.io.File; +import java.util.Arrays; +import io.undertow.Undertow; +import io.undertow.Undertow.Builder; +import io.undertow.UndertowOptions; import org.junit.Before; import org.junit.Test; +import org.xnio.OptionMap; import org.springframework.boot.autoconfigure.web.ServerProperties; import org.springframework.boot.context.properties.bind.Bindable; import org.springframework.boot.context.properties.bind.Binder; import org.springframework.boot.context.properties.source.ConfigurationPropertySources; import org.springframework.boot.web.embedded.undertow.ConfigurableUndertowWebServerFactory; +import org.springframework.boot.web.embedded.undertow.UndertowBuilderCustomizer; import org.springframework.mock.env.MockEnvironment; import org.springframework.test.context.support.TestPropertySourceUtils; +import org.springframework.test.util.ReflectionTestUtils; +import static org.assertj.core.api.Assertions.assertThat; +import static org.mockito.ArgumentMatchers.any; +import static org.mockito.BDDMockito.willAnswer; import static org.mockito.Mockito.mock; import static org.mockito.Mockito.verify; @@ -100,6 +110,54 @@ public class UndertowWebServerFactoryCustomizerTests { verify(factory).setUseForwardHeaders(true); } + @Test + public void customizeMaxHttpHeaderSize() { + bind("server.max-http-header-size=2048"); + Builder builder = Undertow.builder(); + ConfigurableUndertowWebServerFactory factory = mockFactory(builder); + this.customizer.customize(factory); + OptionMap map = ((OptionMap.Builder) ReflectionTestUtils.getField(builder, + "serverOptions")).getMap(); + assertThat(map.get(UndertowOptions.MAX_HEADER_SIZE).intValue()).isEqualTo(2048); + } + + @Test + public void customMaxHttpHeaderSizeIgnoredIfNegative() { + bind("server.max-http-header-size=-1"); + Builder builder = Undertow.builder(); + ConfigurableUndertowWebServerFactory factory = mockFactory(builder); + this.customizer.customize(factory); + OptionMap map = ((OptionMap.Builder) ReflectionTestUtils.getField(builder, + "serverOptions")).getMap(); + assertThat(map.contains(UndertowOptions.MAX_HEADER_SIZE)).isFalse(); + } + + @Test + public void customMaxHttpHeaderSizeIgnoredIfZero() { + bind("server.max-http-header-size=0"); + Builder builder = Undertow.builder(); + ConfigurableUndertowWebServerFactory factory = mockFactory(builder); + this.customizer.customize(factory); + OptionMap map = ((OptionMap.Builder) ReflectionTestUtils.getField(builder, + "serverOptions")).getMap(); + assertThat(map.contains(UndertowOptions.MAX_HEADER_SIZE)).isFalse(); + } + + private ConfigurableUndertowWebServerFactory mockFactory(Builder builder) { + ConfigurableUndertowWebServerFactory factory = mock( + ConfigurableUndertowWebServerFactory.class); + willAnswer((invocation) -> { + Object argument = invocation.getArgument(0); + Arrays.stream((argument instanceof UndertowBuilderCustomizer) + ? new UndertowBuilderCustomizer[] { + (UndertowBuilderCustomizer) argument } + : (UndertowBuilderCustomizer[]) argument) + .forEach((customizer) -> customizer.customize(builder)); + return null; + }).given(factory).addBuilderCustomizers(any()); + return factory; + } + private void bind(String... inlinedProperties) { TestPropertySourceUtils.addInlinedPropertiesToEnvironment(this.environment, inlinedProperties);