From e4fa9ce8c6751f4ad696ff75b7783b7f7af516f9 Mon Sep 17 00:00:00 2001 From: Brian Clozel Date: Wed, 16 Oct 2019 11:36:21 +0200 Subject: [PATCH] Deprecate server.connection-timeout property Prior to this commit, all supported servers would share the same configuration property `server.connection-timeout`. Unfortunately, in many cases the behavior of this timeout changes depending on the server. From actual connection setup timeout, to detecting and closing idle connections, this property cannot be properly translated from one server implementation to another. This commit deprecates this configuration property and introduces server specific properties: * `server.jetty.connection-idle-timeout` (Time that the connection can be idle before it is closed.) * `server.netty.connection-timeout` (Connection timeout of the Netty channel.) * `server.tomcat.connection-timeout` (Amount of time the connector will wait, after accepting a connection, for the request URI line to be presented.) * `server.undertow.no-request-timeout` (Amount of time a connection can sit idle without processing a request, before it is closed by the server.) `server.connection-timeout` is now deprecated and will be removed in a future release. Fixes gh-18473 --- .../autoconfigure/web/ServerProperties.java | 71 +++++++++++++++++++ .../JettyWebServerFactoryCustomizer.java | 6 +- .../NettyWebServerFactoryCustomizer.java | 10 ++- .../TomcatWebServerFactoryCustomizer.java | 2 + .../UndertowWebServerFactoryCustomizer.java | 6 +- .../JettyWebServerFactoryCustomizerTests.java | 20 ++++++ .../NettyWebServerFactoryCustomizerTests.java | 24 +++++-- ...TomcatWebServerFactoryCustomizerTests.java | 8 +++ ...dertowWebServerFactoryCustomizerTests.java | 4 +- .../appendix-application-properties.adoc | 4 ++ 10 files changed, 143 insertions(+), 12 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 6f446254814..6e3144ffe31 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 @@ -111,6 +111,8 @@ public class ServerProperties { private final Jetty jetty = new Jetty(); + private final Netty netty = new Netty(); + private final Undertow undertow = new Undertow(); public Integer getPort() { @@ -153,10 +155,14 @@ public class ServerProperties { this.maxHttpHeaderSize = maxHttpHeaderSize; } + @Deprecated + @DeprecatedConfigurationProperty( + reason = "Each server behaves differently. Use server specific properties instead.") public Duration getConnectionTimeout() { return this.connectionTimeout; } + @Deprecated public void setConnectionTimeout(Duration connectionTimeout) { this.connectionTimeout = connectionTimeout; } @@ -193,6 +199,10 @@ public class ServerProperties { return this.jetty; } + public Netty getNetty() { + return this.netty; + } + public Undertow getUndertow() { return this.undertow; } @@ -376,6 +386,12 @@ public class ServerProperties { */ private List additionalTldSkipPatterns = new ArrayList<>(); + /** + * Amount of time the connector will wait, after accepting a connection, for the + * request URI line to be presented. + */ + private Duration connectionTimeout; + /** * Static resource configuration. */ @@ -532,6 +548,14 @@ public class ServerProperties { this.additionalTldSkipPatterns = additionalTldSkipPatterns; } + public Duration getConnectionTimeout() { + return this.connectionTimeout; + } + + public void setConnectionTimeout(Duration connectionTimeout) { + this.connectionTimeout = connectionTimeout; + } + public Resource getResource() { return this.resource; } @@ -738,6 +762,11 @@ public class ServerProperties { */ private Integer selectors = -1; + /** + * Time that the connection can be idle before it is closed. + */ + private Duration connectionIdleTimeout; + public Accesslog getAccesslog() { return this.accesslog; } @@ -766,6 +795,14 @@ public class ServerProperties { this.selectors = selectors; } + public Duration getConnectionIdleTimeout() { + return this.connectionIdleTimeout; + } + + public void setConnectionIdleTimeout(Duration connectionIdleTimeout) { + this.connectionIdleTimeout = connectionIdleTimeout; + } + /** * Jetty access log properties. */ @@ -931,6 +968,26 @@ public class ServerProperties { } + /** + * Netty properties. + */ + public static class Netty { + + /** + * Connection timeout of the Netty channel. + */ + private Duration connectionTimeout; + + public Duration getConnectionTimeout() { + return this.connectionTimeout; + } + + public void setConnectionTimeout(Duration connectionTimeout) { + this.connectionTimeout = connectionTimeout; + } + + } + /** * Undertow properties. */ @@ -970,6 +1027,12 @@ public class ServerProperties { */ private boolean eagerFilterInit = true; + /** + * Amount of time a connection can sit idle without processing a request, before + * it is closed by the server. + */ + private Duration noRequestTimeout; + private final Accesslog accesslog = new Accesslog(); public DataSize getMaxHttpPostSize() { @@ -1020,6 +1083,14 @@ public class ServerProperties { this.eagerFilterInit = eagerFilterInit; } + public Duration getNoRequestTimeout() { + return this.noRequestTimeout; + } + + public void setNoRequestTimeout(Duration noRequestTimeout) { + this.noRequestTimeout = noRequestTimeout; + } + public Accesslog getAccesslog() { return this.accesslog; } 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 6dbcb27419c..3b230f210ed 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 @@ -77,7 +77,9 @@ public class JettyWebServerFactoryCustomizer propertyMapper.from(jettyProperties::getMaxHttpPostSize).asInt(DataSize::toBytes).when(this::isPositive) .to((maxHttpPostSize) -> customizeMaxHttpPostSize(factory, maxHttpPostSize)); propertyMapper.from(properties::getConnectionTimeout).whenNonNull() - .to((connectionTimeout) -> customizeConnectionTimeout(factory, connectionTimeout)); + .to((connectionTimeout) -> customizeIdleTimeout(factory, connectionTimeout)); + propertyMapper.from(jettyProperties::getConnectionIdleTimeout).whenNonNull() + .to((idleTimeout) -> customizeIdleTimeout(factory, idleTimeout)); propertyMapper.from(jettyProperties::getAccesslog).when(ServerProperties.Jetty.Accesslog::isEnabled) .to((accesslog) -> customizeAccessLog(factory, accesslog)); } @@ -94,7 +96,7 @@ public class JettyWebServerFactoryCustomizer return platform != null && platform.isUsingForwardHeaders(); } - private void customizeConnectionTimeout(ConfigurableJettyWebServerFactory factory, Duration connectionTimeout) { + private void customizeIdleTimeout(ConfigurableJettyWebServerFactory factory, Duration connectionTimeout) { factory.addServerCustomizers((server) -> { for (org.eclipse.jetty.server.Connector connector : server.getConnectors()) { if (connector instanceof AbstractConnector) { 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 2344b9755a1..54c4d4576ac 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 @@ -61,6 +61,9 @@ public class NettyWebServerFactoryCustomizer propertyMapper.from(this.serverProperties::getMaxHttpHeaderSize) .to((maxHttpRequestHeaderSize) -> customizeMaxHttpHeaderSize(factory, maxHttpRequestHeaderSize)); propertyMapper.from(this.serverProperties::getConnectionTimeout) + .to((connectionTimeout) -> customizeGenericConnectionTimeout(factory, connectionTimeout)); + ServerProperties.Netty nettyProperties = this.serverProperties.getNetty(); + propertyMapper.from(nettyProperties::getConnectionTimeout).whenNonNull() .to((connectionTimeout) -> customizeConnectionTimeout(factory, connectionTimeout)); } @@ -77,7 +80,7 @@ public class NettyWebServerFactoryCustomizer (httpRequestDecoderSpec) -> httpRequestDecoderSpec.maxHeaderSize((int) maxHttpHeaderSize.toBytes()))); } - private void customizeConnectionTimeout(NettyReactiveWebServerFactory factory, Duration connectionTimeout) { + private void customizeGenericConnectionTimeout(NettyReactiveWebServerFactory factory, Duration connectionTimeout) { if (!connectionTimeout.isZero()) { long timeoutMillis = connectionTimeout.isNegative() ? 0 : connectionTimeout.toMillis(); factory.addServerCustomizers((httpServer) -> httpServer.tcpConfiguration((tcpServer) -> tcpServer @@ -85,4 +88,9 @@ public class NettyWebServerFactoryCustomizer } } + private void customizeConnectionTimeout(NettyReactiveWebServerFactory factory, Duration connectionTimeout) { + factory.addServerCustomizers((httpServer) -> httpServer.tcpConfiguration((tcpServer) -> tcpServer + .selectorOption(ChannelOption.CONNECT_TIMEOUT_MILLIS, (int) connectionTimeout.toMillis()))); + } + } 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 e876f0e1778..2218bc2c8ca 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 @@ -94,6 +94,8 @@ public class TomcatWebServerFactoryCustomizer propertyMapper.from(tomcatProperties::getUriEncoding).whenNonNull().to(factory::setUriEncoding); propertyMapper.from(properties::getConnectionTimeout).whenNonNull() .to((connectionTimeout) -> customizeConnectionTimeout(factory, connectionTimeout)); + propertyMapper.from(tomcatProperties::getConnectionTimeout).whenNonNull() + .to((connectionTimeout) -> customizeConnectionTimeout(factory, connectionTimeout)); propertyMapper.from(tomcatProperties::getMaxConnections).when(this::isPositive) .to((maxConnections) -> customizeMaxConnections(factory, maxConnections)); propertyMapper.from(tomcatProperties::getAcceptCount).when(this::isPositive) 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 5cdc23269fe..5f187321bd2 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 @@ -81,14 +81,16 @@ public class UndertowWebServerFactoryCustomizer propertyMapper.from(undertowProperties::getMaxHttpPostSize).asInt(DataSize::toBytes).when(this::isPositive) .to((maxHttpPostSize) -> customizeMaxHttpPostSize(factory, maxHttpPostSize)); propertyMapper.from(properties::getConnectionTimeout) - .to((connectionTimeout) -> customizeConnectionTimeout(factory, connectionTimeout)); + .to((connectionTimeout) -> customizeNoRequestTimeout(factory, connectionTimeout)); + propertyMapper.from(undertowProperties::getNoRequestTimeout).whenNonNull() + .to((connectionTimeout) -> customizeNoRequestTimeout(factory, connectionTimeout)); } private boolean isPositive(Number value) { return value.longValue() > 0; } - private void customizeConnectionTimeout(ConfigurableUndertowWebServerFactory factory, Duration connectionTimeout) { + private void customizeNoRequestTimeout(ConfigurableUndertowWebServerFactory factory, Duration connectionTimeout) { factory.addBuilderCustomizers((builder) -> builder.setServerOption(UndertowOptions.NO_REQUEST_TIMEOUT, (int) connectionTimeout.toMillis())); } 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 c45e788c891..eadae6daa1b 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 @@ -19,10 +19,13 @@ package org.springframework.boot.autoconfigure.web.embedded; import java.io.File; import java.io.IOException; import java.util.ArrayList; +import java.util.Arrays; import java.util.List; import java.util.Locale; import java.util.TimeZone; +import java.util.stream.Collectors; +import org.eclipse.jetty.server.AbstractConnector; import org.eclipse.jetty.server.Connector; import org.eclipse.jetty.server.HttpConfiguration; import org.eclipse.jetty.server.HttpConfiguration.ConnectionFactory; @@ -159,6 +162,23 @@ public class JettyWebServerFactoryCustomizerTests { assertThat(requestHeaderSizes).containsOnly(8192); } + @Test + public void customIdleTimeout() { + bind("server.jetty.idle-timeout=60s"); + JettyWebServer server = customizeAndGetServer(); + List timeouts = connectorsIdleTimeouts(server); + assertThat(timeouts).containsOnly(60000L); + } + + private List connectorsIdleTimeouts(JettyWebServer server) { + // Start (and directly stop) server to have connectors available + server.start(); + server.stop(); + return Arrays.stream(server.getServer().getConnectors()) + .filter((connector) -> connector instanceof AbstractConnector).map(Connector::getIdleTimeout) + .collect(Collectors.toList()); + } + private List getRequestHeaderSizes(JettyWebServer server) { List requestHeaderSizes = new ArrayList<>(); // Start (and directly stop) server to have connectors available 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 7a1e434246d..75111a81552 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 @@ -93,21 +93,29 @@ public class NettyWebServerFactoryCustomizerTests { } @Test - public void setConnectionTimeoutAsZero() { - setupConnectionTimeout(Duration.ZERO); + public void setServerConnectionTimeoutAsZero() { + setupServerConnectionTimeout(Duration.ZERO); NettyReactiveWebServerFactory factory = mock(NettyReactiveWebServerFactory.class); this.customizer.customize(factory); verifyConnectionTimeout(factory, null); } @Test - public void setConnectionTimeoutAsMinusOne() { - setupConnectionTimeout(Duration.ofNanos(-1)); + public void setServerConnectionTimeoutAsMinusOne() { + setupServerConnectionTimeout(Duration.ofNanos(-1)); NettyReactiveWebServerFactory factory = mock(NettyReactiveWebServerFactory.class); this.customizer.customize(factory); verifyConnectionTimeout(factory, 0); } + @Test + public void setServerConnectionTimeout() { + setupServerConnectionTimeout(Duration.ofSeconds(1)); + NettyReactiveWebServerFactory factory = mock(NettyReactiveWebServerFactory.class); + this.customizer.customize(factory); + verifyConnectionTimeout(factory, 1000); + } + @Test public void setConnectionTimeout() { setupConnectionTimeout(Duration.ofSeconds(1)); @@ -131,10 +139,16 @@ public class NettyWebServerFactoryCustomizerTests { assertThat(options).containsEntry(ChannelOption.CONNECT_TIMEOUT_MILLIS, expected); } - private void setupConnectionTimeout(Duration connectionTimeout) { + private void setupServerConnectionTimeout(Duration connectionTimeout) { this.serverProperties.setUseForwardHeaders(null); this.serverProperties.setMaxHttpHeaderSize(null); this.serverProperties.setConnectionTimeout(connectionTimeout); } + private void setupConnectionTimeout(Duration connectionTimeout) { + this.serverProperties.setUseForwardHeaders(null); + this.serverProperties.setMaxHttpHeaderSize(null); + this.serverProperties.getNetty().setConnectionTimeout(connectionTimeout); + } + } 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 7280ef4867b..20aef9f8d63 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 @@ -151,6 +151,14 @@ public class TomcatWebServerFactoryCustomizerTests { .getMaxSwallowSize()).isEqualTo(DataSize.ofMegabytes(10).toBytes())); } + @Test + public void customConnectionTimeout() { + bind("server.tomcat.connection-timeout=30s"); + customizeAndRunServer((server) -> assertThat( + ((AbstractProtocol) server.getTomcat().getConnector().getProtocolHandler()).getConnectionTimeout()) + .isEqualTo(30000)); + } + @Test public void customRemoteIpValve() { bind("server.tomcat.remote-ip-header=x-my-remote-ip-header", 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 e49c33c24f9..7c488346e42 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 @@ -135,13 +135,13 @@ public class UndertowWebServerFactoryCustomizerTests { @Test public void customConnectionTimeout() { - bind("server.connection-timeout=100"); + bind("server.undertow.no-request-timeout=1m"); 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); + assertThat(map.get(UndertowOptions.NO_REQUEST_TIMEOUT)).isEqualTo(60000); } private ConfigurableUndertowWebServerFactory mockFactory(Builder builder) { diff --git a/spring-boot-project/spring-boot-docs/src/main/asciidoc/appendix-application-properties.adoc b/spring-boot-project/spring-boot-docs/src/main/asciidoc/appendix-application-properties.adoc index 01c652d20c6..4488ba74c15 100644 --- a/spring-boot-project/spring-boot-docs/src/main/asciidoc/appendix-application-properties.adoc +++ b/spring-boot-project/spring-boot-docs/src/main/asciidoc/appendix-application-properties.adoc @@ -207,8 +207,10 @@ Rather, pick only the properties that you need. server.jetty.accesslog.log-server=false # Enable logging of the request hostname. server.jetty.accesslog.retention-period=31 # Number of days before rotated log files are deleted. server.jetty.accesslog.time-zone=GMT # Timezone of the request log. + server.jetty.connection-idle-timeout= # Time that the connection can be idle before it is closed. server.jetty.max-http-post-size=200000B # Maximum size of the HTTP post or put content. server.jetty.selectors=-1 # Number of selector threads to use. When the value is -1, the default, the number of selectors is derived from the operating environment. + server.netty.connection-timeout= # Connection timeout of the Netty channel. server.max-http-header-size=8KB # Maximum size of the HTTP message header. server.port=8080 # Server HTTP port. server.server-header= # Value to use for the Server response header (if empty, no header is sent). @@ -259,6 +261,7 @@ Rather, pick only the properties that you need. server.tomcat.additional-tld-skip-patterns= # Comma-separated list of additional patterns that match jars to ignore for TLD scanning. server.tomcat.background-processor-delay=10s # Delay between the invocation of backgroundProcess methods. If a duration suffix is not specified, seconds will be used. server.tomcat.basedir= # Tomcat base directory. If not specified, a temporary directory is used. + server.tomcat.connection-timeout= # Amount of time the connector will wait, after accepting a connection, for the request URI line to be presented. server.tomcat.internal-proxies=10\\.\\d{1,3}\\.\\d{1,3}\\.\\d{1,3}|\\ 192\\.168\\.\\d{1,3}\\.\\d{1,3}|\\ 169\\.254\\.\\d{1,3}\\.\\d{1,3}|\\ @@ -293,6 +296,7 @@ Rather, pick only the properties that you need. server.undertow.eager-filter-init=true # Whether servlet filters should be initialized on startup. server.undertow.io-threads= # Number of I/O threads to create for the worker. The default is derived from the number of available processors. server.undertow.max-http-post-size=-1B # Maximum size of the HTTP post content. When the value is -1, the default, the size is unlimited. + server.undertow.no-request-timeout= # Amount of time a connection can sit idle without processing a request, before it is closed by the server. server.undertow.worker-threads= # Number of worker threads. The default is 8 times the number of I/O threads. # FREEMARKER ({spring-boot-autoconfigure-module-code}/freemarker/FreeMarkerProperties.java[FreeMarkerProperties])