From b98c3dccfd4b690d61cff12c5daa6cbd400fa908 Mon Sep 17 00:00:00 2001 From: Andy Wilkinson Date: Mon, 27 Jul 2020 10:47:14 +0100 Subject: [PATCH] Fix property-based configuration of Undertow socket options Previously, only UndertowOptions was used as the source of options for both server and socket options, but it only contains server options. As a result, attempting to configure any socket options defined by XNIO's Options class would fail. This commit updates the property-based configuration of options to use UndertowOptions as the source for server options and XNIO's Options as the source for socket options. Fixes gh-22502 --- .../UndertowWebServerFactoryCustomizer.java | 103 +++++++++++------- ...dertowWebServerFactoryCustomizerTests.java | 10 +- 2 files changed, 72 insertions(+), 41 deletions(-) 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 937c13b3dd2..8f644fb9879 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 @@ -26,7 +26,9 @@ import java.util.function.Consumer; import java.util.function.Function; import io.undertow.UndertowOptions; +import org.apache.commons.lang.ClassUtils; import org.xnio.Option; +import org.xnio.Options; import org.springframework.boot.autoconfigure.web.ServerProperties; import org.springframework.boot.autoconfigure.web.ServerProperties.Undertow; @@ -74,18 +76,18 @@ public class UndertowWebServerFactoryCustomizer @Override public void customize(ConfigurableUndertowWebServerFactory factory) { PropertyMapper map = PropertyMapper.get().alwaysApplyingWhenNonNull(); - FactoryOptions options = new FactoryOptions(factory); + ServerOptions options = new ServerOptions(factory); ServerProperties properties = this.serverProperties; map.from(properties::getMaxHttpHeaderSize).asInt(DataSize::toBytes).when(this::isPositive) - .to(options.server(UndertowOptions.MAX_HEADER_SIZE)); + .to(options.option(UndertowOptions.MAX_HEADER_SIZE)); map.from(properties::getConnectionTimeout).asInt(Duration::toMillis) - .to(options.server(UndertowOptions.NO_REQUEST_TIMEOUT)); + .to(options.option(UndertowOptions.NO_REQUEST_TIMEOUT)); mapUndertowProperties(factory, options); mapAccessLogProperties(factory); map.from(this::getOrDeduceUseForwardHeaders).to(factory::setUseForwardHeaders); } - private void mapUndertowProperties(ConfigurableUndertowWebServerFactory factory, FactoryOptions options) { + private void mapUndertowProperties(ConfigurableUndertowWebServerFactory factory, ServerOptions serverOptions) { PropertyMapper map = PropertyMapper.get().alwaysApplyingWhenNonNull(); Undertow properties = this.serverProperties.getUndertow(); map.from(properties::getBufferSize).whenNonNull().asInt(DataSize::toBytes).to(factory::setBufferSize); @@ -93,18 +95,19 @@ public class UndertowWebServerFactoryCustomizer map.from(properties::getWorkerThreads).to(factory::setWorkerThreads); map.from(properties::getDirectBuffers).to(factory::setUseDirectBuffers); map.from(properties::getMaxHttpPostSize).as(DataSize::toBytes).when(this::isPositive) - .to(options.server(UndertowOptions.MAX_ENTITY_SIZE)); - map.from(properties::getMaxParameters).to(options.server(UndertowOptions.MAX_PARAMETERS)); - map.from(properties::getMaxHeaders).to(options.server(UndertowOptions.MAX_HEADERS)); - map.from(properties::getMaxCookies).to(options.server(UndertowOptions.MAX_COOKIES)); - map.from(properties::isAllowEncodedSlash).to(options.server(UndertowOptions.ALLOW_ENCODED_SLASH)); - map.from(properties::isDecodeUrl).to(options.server(UndertowOptions.DECODE_URL)); - map.from(properties::getUrlCharset).as(Charset::name).to(options.server(UndertowOptions.URL_CHARSET)); - map.from(properties::isAlwaysSetKeepAlive).to(options.server(UndertowOptions.ALWAYS_SET_KEEP_ALIVE)); + .to(serverOptions.option(UndertowOptions.MAX_ENTITY_SIZE)); + map.from(properties::getMaxParameters).to(serverOptions.option(UndertowOptions.MAX_PARAMETERS)); + map.from(properties::getMaxHeaders).to(serverOptions.option(UndertowOptions.MAX_HEADERS)); + map.from(properties::getMaxCookies).to(serverOptions.option(UndertowOptions.MAX_COOKIES)); + map.from(properties::isAllowEncodedSlash).to(serverOptions.option(UndertowOptions.ALLOW_ENCODED_SLASH)); + map.from(properties::isDecodeUrl).to(serverOptions.option(UndertowOptions.DECODE_URL)); + map.from(properties::getUrlCharset).as(Charset::name).to(serverOptions.option(UndertowOptions.URL_CHARSET)); + map.from(properties::isAlwaysSetKeepAlive).to(serverOptions.option(UndertowOptions.ALWAYS_SET_KEEP_ALIVE)); map.from(properties::getNoRequestTimeout).asInt(Duration::toMillis) - .to(options.server(UndertowOptions.NO_REQUEST_TIMEOUT)); - map.from(properties.getOptions()::getServer).to(options.forEach(options::server)); - map.from(properties.getOptions()::getSocket).to(options.forEach(options::socket)); + .to(serverOptions.option(UndertowOptions.NO_REQUEST_TIMEOUT)); + map.from(properties.getOptions()::getServer).to(serverOptions.forEach(serverOptions::option)); + SocketOptions socketOptions = new SocketOptions(factory); + map.from(properties.getOptions()::getSocket).to(socketOptions.forEach(socketOptions::option)); } private boolean isPositive(Number value) { @@ -130,16 +133,17 @@ public class UndertowWebServerFactoryCustomizer return this.serverProperties.getForwardHeadersStrategy().equals(ServerProperties.ForwardHeadersStrategy.NATIVE); } - /** - * {@link ConfigurableUndertowWebServerFactory} wrapper that makes it easier to apply - * {@link UndertowOptions}. - */ - private static class FactoryOptions { + private abstract static class AbstractOptions { - private static final Map> NAME_LOOKUP; - static { + private final Class source; + + private final Map> nameLookup; + + private final ConfigurableUndertowWebServerFactory factory; + + AbstractOptions(Class source, ConfigurableUndertowWebServerFactory factory) { Map> lookup = new HashMap<>(); - ReflectionUtils.doWithLocalFields(UndertowOptions.class, (field) -> { + ReflectionUtils.doWithLocalFields(source, (field) -> { int modifiers = field.getModifiers(); if (Modifier.isPublic(modifiers) && Modifier.isStatic(modifiers) && Option.class.isAssignableFrom(field.getType())) { @@ -151,29 +155,22 @@ public class UndertowWebServerFactoryCustomizer } } }); - NAME_LOOKUP = Collections.unmodifiableMap(lookup); - } - - private final ConfigurableUndertowWebServerFactory factory; - - FactoryOptions(ConfigurableUndertowWebServerFactory factory) { + this.source = source; + this.nameLookup = Collections.unmodifiableMap(lookup); this.factory = factory; } - Consumer server(Option option) { - return (value) -> this.factory.addBuilderCustomizers((builder) -> builder.setServerOption(option, value)); - } - - Consumer socket(Option option) { - return (value) -> this.factory.addBuilderCustomizers((builder) -> builder.setSocketOption(option, value)); + protected ConfigurableUndertowWebServerFactory getFactory() { + return this.factory; } @SuppressWarnings("unchecked") Consumer> forEach(Function, Consumer> function) { return (map) -> { map.forEach((key, value) -> { - Option option = (Option) NAME_LOOKUP.get(getCanonicalName(key)); - Assert.state(option != null, "Unable to find '" + key + "' in UndertowOptions"); + Option option = (Option) this.nameLookup.get(getCanonicalName(key)); + Assert.state(option != null, + "Unable to find '" + key + "' in " + ClassUtils.getShortClassName(this.source)); T parsed = option.parseValue(value, getClass().getClassLoader()); function.apply(option).accept(parsed); }); @@ -189,4 +186,36 @@ public class UndertowWebServerFactoryCustomizer } + /** + * {@link ConfigurableUndertowWebServerFactory} wrapper that makes it easier to apply + * {@link UndertowOptions server options}. + */ + private static class ServerOptions extends AbstractOptions { + + ServerOptions(ConfigurableUndertowWebServerFactory factory) { + super(UndertowOptions.class, factory); + } + + Consumer option(Option option) { + return (value) -> getFactory().addBuilderCustomizers((builder) -> builder.setServerOption(option, value)); + } + + } + + /** + * {@link ConfigurableUndertowWebServerFactory} wrapper that makes it easier to apply + * {@link Options socket options}. + */ + private static class SocketOptions extends AbstractOptions { + + SocketOptions(ConfigurableUndertowWebServerFactory factory) { + super(Options.class, factory); + } + + Consumer option(Option option) { + return (value) -> getFactory().addBuilderCustomizers((builder) -> builder.setSocketOption(option, value)); + } + + } + } 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 2186844f781..5019a0e2f56 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 @@ -27,6 +27,7 @@ import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; import org.xnio.Option; import org.xnio.OptionMap; +import org.xnio.Options; import org.springframework.boot.autoconfigure.web.ServerProperties; import org.springframework.boot.context.properties.bind.Bindable; @@ -170,13 +171,14 @@ class UndertowWebServerFactoryCustomizerTests { @Test void customSocketOption() { - bind("server.undertow.options.socket.ALWAYS_SET_KEEP_ALIVE=false"); - assertThat(boundSocketOption(UndertowOptions.ALWAYS_SET_KEEP_ALIVE)).isFalse(); + bind("server.undertow.options.socket.CONNECTION_LOW_WATER=8"); + assertThat(boundSocketOption(Options.CONNECTION_LOW_WATER)).isEqualTo(8); } + @Test void customSocketOptionShouldBeRelaxed() { - bind("server.undertow.options.socket.always-set-keep-alive=false"); - assertThat(boundSocketOption(UndertowOptions.ALWAYS_SET_KEEP_ALIVE)).isFalse(); + bind("server.undertow.options.socket.connection-low-water=8"); + assertThat(boundSocketOption(Options.CONNECTION_LOW_WATER)).isEqualTo(8); } @Test