From ca1a666f58ac3587a6ad67d0a27c5ab6c93be910 Mon Sep 17 00:00:00 2001 From: Andy Wilkinson Date: Thu, 18 Apr 2019 12:18:29 +0100 Subject: [PATCH] Polish "Add support for configuring remaining Undertow server options" See gh-16278 --- .../autoconfigure/web/ServerProperties.java | 66 ++++----- .../UndertowWebServerFactoryCustomizer.java | 34 ++--- ...dertowWebServerFactoryCustomizerTests.java | 136 +++++++++--------- 3 files changed, 121 insertions(+), 115 deletions(-) diff --git a/spring-boot-project/spring-boot-autoconfigure/src/main/java/org/springframework/boot/autoconfigure/web/ServerProperties.java b/spring-boot-project/spring-boot-autoconfigure/src/main/java/org/springframework/boot/autoconfigure/web/ServerProperties.java index b8ee50e26db..9603392e1a8 100644 --- a/spring-boot-project/spring-boot-autoconfigure/src/main/java/org/springframework/boot/autoconfigure/web/ServerProperties.java +++ b/spring-boot-project/spring-boot-autoconfigure/src/main/java/org/springframework/boot/autoconfigure/web/ServerProperties.java @@ -29,6 +29,8 @@ import java.util.Locale; import java.util.Map; import java.util.TimeZone; +import io.undertow.UndertowOptions; + import org.springframework.boot.context.properties.ConfigurationProperties; import org.springframework.boot.context.properties.DeprecatedConfigurationProperty; import org.springframework.boot.context.properties.NestedConfigurationProperty; @@ -1117,47 +1119,47 @@ public class ServerProperties { private boolean eagerFilterInit = true; /** - * The maximum number of query or path parameters that are allowed. This limit - * exists to prevent hash collision based DOS attacks. + * Maximum number of query or path parameters that are allowed. This limit exists + * to prevent hash collision based DOS attacks. */ - private Integer maxParameters; + private int maxParameters = UndertowOptions.DEFAULT_MAX_PARAMETERS; /** - * The maximum number of headers that are allowed. This limit exists to prevent - * hash collision based DOS attacks. + * Maximum number of headers that are allowed. This limit exists to prevent hash + * collision based DOS attacks. */ - private Integer maxHeaders; + private int maxHeaders = UndertowOptions.DEFAULT_MAX_HEADERS; /** - * The maximum number of cookies that are allowed. This limit exists to prevent - * hash collision based DOS attacks. + * Maximum number of cookies that are allowed. This limit exists to prevent hash + * collision based DOS attacks. */ - private Integer maxCookies; + private int maxCookies = 200; /** - * Set this to true if you want the server to decode percent encoded slash - * characters. This is probably a bad idea, as it can have security implications, - * due to different servers interpreting the slash differently. Only enable this - * if you have a legacy application that requires it. + * Whether the server should decode percent encoded slash characters. Enabling + * encoded slashes can have security implications due to different servers + * interpreting the slash differently. Only enable this if you have a legacy + * application that requires it. */ - private Boolean allowEncodedSlash; + private boolean allowEncodedSlash = false; /** - * If the URL should be decoded. If this is not set to true then percent encoded - * characters in the URL will be left as is. + * Whether the URL should be decoded. When disabled, percent-encoded characters in + * the URL will be left as-is. */ - private Boolean decodeUrl; + private boolean decodeUrl = true; /** - * The charset to decode the URL to. + * Charset used to decode URLs. */ - private String urlCharset; + private Charset urlCharset = StandardCharsets.UTF_8; /** - * If the 'Connection: keep-alive' header should be added to all responses, even - * if not required by spec. + * Whether the 'Connection: keep-alive' header should be added to all responses, + * even if not required by the HTTP specification. */ - private Boolean alwaysSetKeepAlive; + private boolean alwaysSetKeepAlive = true; private final Accesslog accesslog = new Accesslog(); @@ -1209,7 +1211,7 @@ public class ServerProperties { this.eagerFilterInit = eagerFilterInit; } - public Integer getMaxParameters() { + public int getMaxParameters() { return this.maxParameters; } @@ -1217,11 +1219,11 @@ public class ServerProperties { this.maxParameters = maxParameters; } - public Integer getMaxHeaders() { + public int getMaxHeaders() { return this.maxHeaders; } - public void setMaxHeaders(Integer maxHeaders) { + public void setMaxHeaders(int maxHeaders) { this.maxHeaders = maxHeaders; } @@ -1233,15 +1235,15 @@ public class ServerProperties { this.maxCookies = maxCookies; } - public Boolean isAllowEncodedSlash() { + public boolean isAllowEncodedSlash() { return this.allowEncodedSlash; } - public void setAllowEncodedSlash(Boolean allowEncodedSlash) { + public void setAllowEncodedSlash(boolean allowEncodedSlash) { this.allowEncodedSlash = allowEncodedSlash; } - public Boolean isDecodeUrl() { + public boolean isDecodeUrl() { return this.decodeUrl; } @@ -1249,19 +1251,19 @@ public class ServerProperties { this.decodeUrl = decodeUrl; } - public String getUrlCharset() { + public Charset getUrlCharset() { return this.urlCharset; } - public void setUrlCharset(String urlCharset) { + public void setUrlCharset(Charset urlCharset) { this.urlCharset = urlCharset; } - public Boolean isAlwaysSetKeepAlive() { + public boolean isAlwaysSetKeepAlive() { return this.alwaysSetKeepAlive; } - public void setAlwaysSetKeepAlive(Boolean alwaysSetKeepAlive) { + public void setAlwaysSetKeepAlive(boolean alwaysSetKeepAlive) { this.alwaysSetKeepAlive = alwaysSetKeepAlive; } 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 0bd6f897b61..5cadb2fb058 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 @@ -89,45 +89,45 @@ public class UndertowWebServerFactoryCustomizer implements propertyMapper.from(properties::getMaxHttpHeaderSize).whenNonNull() .asInt(DataSize::toBytes).when(this::isPositive) - .to((maxHttpHeaderSize) -> customizeProperties(factory, + .to((maxHttpHeaderSize) -> customizeServerOption(factory, UndertowOptions.MAX_HEADER_SIZE, maxHttpHeaderSize)); - propertyMapper.from(undertowProperties::getMaxHttpPostSize) - .asInt(DataSize::toBytes).when(this::isPositive) - .to((maxHttpPostSize) -> customizeProperties(factory, - UndertowOptions.MAX_HEADER_SIZE, maxHttpPostSize)); + propertyMapper.from(undertowProperties::getMaxHttpPostSize).as(DataSize::toBytes) + .when(this::isPositive) + .to((maxHttpPostSize) -> customizeServerOption(factory, + UndertowOptions.MAX_ENTITY_SIZE, maxHttpPostSize)); propertyMapper.from(properties::getConnectionTimeout) - .to((connectionTimeout) -> customizeProperties(factory, + .to((connectionTimeout) -> customizeServerOption(factory, UndertowOptions.NO_REQUEST_TIMEOUT, (int) connectionTimeout.toMillis())); propertyMapper.from(undertowProperties::getMaxParameters) - .to((maxParameters) -> customizeProperties(factory, + .to((maxParameters) -> customizeServerOption(factory, UndertowOptions.MAX_PARAMETERS, maxParameters)); propertyMapper.from(undertowProperties::getMaxHeaders) - .to((maxHeaders) -> customizeProperties(factory, + .to((maxHeaders) -> customizeServerOption(factory, UndertowOptions.MAX_HEADERS, maxHeaders)); propertyMapper.from(undertowProperties::getMaxCookies) - .to((maxCookies) -> customizeProperties(factory, + .to((maxCookies) -> customizeServerOption(factory, UndertowOptions.MAX_COOKIES, maxCookies)); propertyMapper.from(undertowProperties::isAllowEncodedSlash) - .to((allowEncodedSlash) -> customizeProperties(factory, + .to((allowEncodedSlash) -> customizeServerOption(factory, UndertowOptions.ALLOW_ENCODED_SLASH, allowEncodedSlash)); propertyMapper.from(undertowProperties::isDecodeUrl) - .to((isDecodeUrl) -> customizeProperties(factory, + .to((isDecodeUrl) -> customizeServerOption(factory, UndertowOptions.DECODE_URL, isDecodeUrl)); propertyMapper.from(undertowProperties::getUrlCharset) - .to((urlCharset) -> customizeProperties(factory, - UndertowOptions.URL_CHARSET, urlCharset)); + .to((urlCharset) -> customizeServerOption(factory, + UndertowOptions.URL_CHARSET, urlCharset.name())); propertyMapper.from(undertowProperties::isAlwaysSetKeepAlive) - .to((alwaysSetKeepAlive) -> customizeProperties(factory, + .to((alwaysSetKeepAlive) -> customizeServerOption(factory, UndertowOptions.ALWAYS_SET_KEEP_ALIVE, alwaysSetKeepAlive)); factory.addDeploymentInfoCustomizers((deploymentInfo) -> deploymentInfo @@ -138,10 +138,10 @@ public class UndertowWebServerFactoryCustomizer implements return value.longValue() > 0; } - private void customizeProperties(ConfigurableUndertowWebServerFactory factory, - Option propType, T prop) { + private void customizeServerOption(ConfigurableUndertowWebServerFactory factory, + Option option, T value) { factory.addBuilderCustomizers( - (builder) -> builder.setServerOption(propType, prop)); + (builder) -> builder.setServerOption(option, value)); } private boolean getOrDeduceUseForwardHeaders() { 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 fa0d62db76b..64c65543f4c 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,6 +17,7 @@ package org.springframework.boot.autoconfigure.web.embedded; import java.io.File; +import java.nio.charset.StandardCharsets; import java.util.Arrays; import io.undertow.Undertow; @@ -24,6 +25,7 @@ import io.undertow.Undertow.Builder; import io.undertow.UndertowOptions; import org.junit.Before; import org.junit.Test; +import org.xnio.Option; import org.xnio.OptionMap; import org.springframework.boot.autoconfigure.web.ServerProperties; @@ -88,37 +90,75 @@ public class UndertowWebServerFactoryCustomizerTests { } @Test - public void customizeUndertowConnectionCommonSettings() { - bind("server.undertow.maxParameters=50", "server.undertow.maxHeaders=60", - "server.undertow.maxCookies=70", "server.undertow.allowEncodedSlash=true", - "server.undertow.decodeUrl=true", "server.undertow.urlCharset=UTF-8", - "server.undertow.alwaysSetKeepAlive=true"); - 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_PARAMETERS)).isEqualTo(50); - assertThat(map.get(UndertowOptions.MAX_HEADERS)).isEqualTo(60); - assertThat(map.get(UndertowOptions.MAX_COOKIES)).isEqualTo(70); - assertThat(map.get(UndertowOptions.ALLOW_ENCODED_SLASH)).isTrue(); - assertThat(map.get(UndertowOptions.DECODE_URL)).isTrue(); - assertThat(map.get(UndertowOptions.URL_CHARSET)).isEqualTo("UTF-8"); - assertThat(map.get(UndertowOptions.ALWAYS_SET_KEEP_ALIVE)).isTrue(); + public void customMaxHttpHeaderSize() { + bind("server.max-http-header-size=2048"); + assertThat(boundServerOption(UndertowOptions.MAX_HEADER_SIZE)).isEqualTo(2048); } @Test - public void customizeUndertowCommonConnectionCommonBoolSettings() { - bind("server.undertow.allowEncodedSlash=false", "server.undertow.decodeUrl=false", - "server.undertow.alwaysSetKeepAlive=false"); - 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.ALLOW_ENCODED_SLASH)).isFalse(); - assertThat(map.get(UndertowOptions.DECODE_URL)).isFalse(); - assertThat(map.get(UndertowOptions.ALWAYS_SET_KEEP_ALIVE)).isFalse(); + public void customMaxHttpHeaderSizeIgnoredIfNegative() { + bind("server.max-http-header-size=-1"); + assertThat(boundServerOption(UndertowOptions.MAX_HEADER_SIZE)).isNull(); + } + + public void customMaxHttpHeaderSizeIgnoredIfZero() { + bind("server.max-http-header-size=0"); + assertThat(boundServerOption(UndertowOptions.MAX_HEADER_SIZE)).isNull(); + } + + @Test + public void customMaxHttpPostSize() { + bind("server.undertow.max-http-post-size=256"); + assertThat(boundServerOption(UndertowOptions.MAX_ENTITY_SIZE)).isEqualTo(256); + } + + @Test + public void customConnectionTimeout() { + bind("server.connectionTimeout=100"); + assertThat(boundServerOption(UndertowOptions.NO_REQUEST_TIMEOUT)).isEqualTo(100); + } + + @Test + public void customMaxParameters() { + bind("server.undertow.max-parameters=4"); + assertThat(boundServerOption(UndertowOptions.MAX_PARAMETERS)).isEqualTo(4); + } + + @Test + public void customMaxHeaders() { + bind("server.undertow.max-headers=4"); + assertThat(boundServerOption(UndertowOptions.MAX_HEADERS)).isEqualTo(4); + } + + @Test + public void customMaxCookies() { + bind("server.undertow.max-cookies=4"); + assertThat(boundServerOption(UndertowOptions.MAX_COOKIES)).isEqualTo(4); + } + + @Test + public void allowEncodedSlashes() { + bind("server.undertow.allow-encoded-slash=true"); + assertThat(boundServerOption(UndertowOptions.ALLOW_ENCODED_SLASH)).isTrue(); + } + + @Test + public void disableUrlDecoding() { + bind("server.undertow.decode-url=false"); + assertThat(boundServerOption(UndertowOptions.DECODE_URL)).isFalse(); + } + + @Test + public void customUrlCharset() { + bind("server.undertow.url-charset=UTF-16"); + assertThat(boundServerOption(UndertowOptions.URL_CHARSET)) + .isEqualTo(StandardCharsets.UTF_16.name()); + } + + @Test + public void disableAlwaysSetKeepAlive() { + bind("server.undertow.always-set-keep-alive=false"); + assertThat(boundServerOption(UndertowOptions.ALWAYS_SET_KEEP_ALIVE)).isFalse(); } @Test @@ -147,49 +187,13 @@ public class UndertowWebServerFactoryCustomizerTests { verify(factory).setUseForwardHeaders(true); } - @Test - public void customizeMaxHttpHeaderSize() { - bind("server.max-http-header-size=2048"); + private T boundServerOption(Option option) { 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(); - } - - @Test - public void customConnectionTimeout() { - bind("server.connection-timeout=100"); - 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.NO_REQUEST_TIMEOUT)).isTrue(); - assertThat(map.get(UndertowOptions.NO_REQUEST_TIMEOUT)).isEqualTo(100); + return map.get(option); } private ConfigurableUndertowWebServerFactory mockFactory(Builder builder) {