Skip to content

Commit 692bda1

Browse files
philwebbphilwebb
committed
Polish "Fix connection timeout configuration for Netty"
See gh-16535
1 parent b0e4c71 commit 692bda1

File tree

2 files changed

+60
-26
lines changed

2 files changed

+60
-26
lines changed

‎spring-boot-project/spring-boot-autoconfigure/src/main/java/org/springframework/boot/autoconfigure/web/embedded/NettyWebServerFactoryCustomizer.java

Lines changed: 12 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -24,7 +24,6 @@
2424
import org.springframework.boot.cloud.CloudPlatform;
2525
import org.springframework.boot.context.properties.PropertyMapper;
2626
import org.springframework.boot.web.embedded.netty.NettyReactiveWebServerFactory;
27-
import org.springframework.boot.web.embedded.netty.NettyServerCustomizer;
2827
import org.springframework.boot.web.server.WebServerFactoryCustomizer;
2928
import org.springframework.core.Ordered;
3029
import org.springframework.core.env.Environment;
@@ -59,12 +58,10 @@ public int getOrder() {
5958
public void customize(NettyReactiveWebServerFactory factory) {
6059
factory.setUseForwardHeaders(getOrDeduceUseForwardHeaders(this.serverProperties, this.environment));
6160
PropertyMapper propertyMapper = PropertyMapper.get().alwaysApplyingWhenNonNull();
62-
propertyMapper.from(this.serverProperties::getMaxHttpHeaderSize).asInt(DataSize::toBytes)
61+
propertyMapper.from(this.serverProperties::getMaxHttpHeaderSize)
6362
.to((maxHttpRequestHeaderSize) -> customizeMaxHttpHeaderSize(factory, maxHttpRequestHeaderSize));
64-
propertyMapper.from(this.serverProperties::getConnectionTimeout).asInt(Duration::toMillis)
65-
.whenNot((connectionTimout) -> connectionTimout.equals(0))
66-
.as((connectionTimeout) -> connectionTimeout.equals(-1) ? 0 : connectionTimeout)
67-
.to((duration) -> factory.addServerCustomizers(getConnectionTimeOutCustomizer(duration)));
63+
propertyMapper.from(this.serverProperties::getConnectionTimeout)
64+
.to((connectionTimeout) -> customizeConnectionTimeout(factory, connectionTimeout));
6865
}
6966

7067
private boolean getOrDeduceUseForwardHeaders(ServerProperties serverProperties, Environment environment) {
@@ -75,14 +72,17 @@ private boolean getOrDeduceUseForwardHeaders(ServerProperties serverProperties,
7572
return platform != null && platform.isUsingForwardHeaders();
7673
}
7774

78-
private void customizeMaxHttpHeaderSize(NettyReactiveWebServerFactory factory, Integer maxHttpHeaderSize) {
79-
factory.addServerCustomizers((NettyServerCustomizer) (httpServer) -> httpServer.httpRequestDecoder(
80-
(httpRequestDecoderSpec) -> httpRequestDecoderSpec.maxHeaderSize(maxHttpHeaderSize)));
75+
private void customizeMaxHttpHeaderSize(NettyReactiveWebServerFactory factory, DataSize maxHttpHeaderSize) {
76+
factory.addServerCustomizers((httpServer) -> httpServer.httpRequestDecoder(
77+
(httpRequestDecoderSpec) -> httpRequestDecoderSpec.maxHeaderSize((int) maxHttpHeaderSize.toBytes())));
8178
}
8279

83-
private NettyServerCustomizer getConnectionTimeOutCustomizer(int duration) {
84-
return (httpServer) -> httpServer.tcpConfiguration(
85-
(tcpServer) -> tcpServer.selectorOption(ChannelOption.CONNECT_TIMEOUT_MILLIS, duration));
80+
private void customizeConnectionTimeout(NettyReactiveWebServerFactory factory, Duration connectionTimeout) {
81+
if (!connectionTimeout.isZero()) {
82+
long timeoutMillis = connectionTimeout.isNegative() ? 0 : connectionTimeout.toMillis();
83+
factory.addServerCustomizers((httpServer) -> httpServer.tcpConfiguration((tcpServer) -> tcpServer
84+
.selectorOption(ChannelOption.CONNECT_TIMEOUT_MILLIS, (int) timeoutMillis)));
85+
}
8686
}
8787

8888
}

‎spring-boot-project/spring-boot-autoconfigure/src/test/java/org/springframework/boot/autoconfigure/web/embedded/NettyWebServerFactoryCustomizerTests.java

Lines changed: 48 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -17,18 +17,29 @@
1717
package org.springframework.boot.autoconfigure.web.embedded;
1818

1919
import java.time.Duration;
20+
import java.util.Map;
2021

22+
import io.netty.bootstrap.ServerBootstrap;
23+
import io.netty.channel.ChannelOption;
2124
import org.junit.Before;
2225
import org.junit.Test;
26+
import org.mockito.ArgumentCaptor;
27+
import org.mockito.Captor;
28+
import org.mockito.MockitoAnnotations;
29+
import reactor.netty.http.server.HttpServer;
30+
import reactor.netty.tcp.TcpServer;
2331

2432
import org.springframework.boot.autoconfigure.web.ServerProperties;
2533
import org.springframework.boot.context.properties.source.ConfigurationPropertySources;
2634
import org.springframework.boot.web.embedded.netty.NettyReactiveWebServerFactory;
2735
import org.springframework.boot.web.embedded.netty.NettyServerCustomizer;
2836
import org.springframework.mock.env.MockEnvironment;
37+
import org.springframework.test.util.ReflectionTestUtils;
2938

30-
import static org.mockito.Mockito.any;
39+
import static org.assertj.core.api.Assertions.assertThat;
40+
import static org.mockito.ArgumentMatchers.any;
3141
import static org.mockito.Mockito.mock;
42+
import static org.mockito.Mockito.never;
3243
import static org.mockito.Mockito.times;
3344
import static org.mockito.Mockito.verify;
3445

@@ -46,20 +57,18 @@ public class NettyWebServerFactoryCustomizerTests {
4657

4758
private NettyWebServerFactoryCustomizer customizer;
4859

60+
@Captor
61+
private ArgumentCaptor<NettyServerCustomizer> customizerCaptor;
62+
4963
@Before
5064
public void setup() {
65+
MockitoAnnotations.initMocks(this);
5166
this.environment = new MockEnvironment();
5267
this.serverProperties = new ServerProperties();
5368
ConfigurationPropertySources.attach(this.environment);
5469
this.customizer = new NettyWebServerFactoryCustomizer(this.environment, this.serverProperties);
5570
}
5671

57-
private void clear() {
58-
this.serverProperties.setUseForwardHeaders(null);
59-
this.serverProperties.setMaxHttpHeaderSize(null);
60-
this.serverProperties.setConnectionTimeout(null);
61-
}
62-
6372
@Test
6473
public void deduceUseForwardHeaders() {
6574
this.environment.setProperty("DYNO", "-");
@@ -85,22 +94,47 @@ public void setUseForwardHeaders() {
8594

8695
@Test
8796
public void setConnectionTimeoutAsZero() {
88-
clear();
89-
this.serverProperties.setConnectionTimeout(Duration.ZERO);
90-
97+
setupConnectionTimeout(Duration.ZERO);
9198
NettyReactiveWebServerFactory factory = mock(NettyReactiveWebServerFactory.class);
9299
this.customizer.customize(factory);
93-
verify(factory, times(0)).addServerCustomizers(any(NettyServerCustomizer.class));
100+
verifyConnectionTimeout(factory, null);
94101
}
95102

96103
@Test
97104
public void setConnectionTimeoutAsMinusOne() {
98-
clear();
99-
this.serverProperties.setConnectionTimeout(Duration.ofNanos(-1));
105+
setupConnectionTimeout(Duration.ofNanos(-1));
106+
NettyReactiveWebServerFactory factory = mock(NettyReactiveWebServerFactory.class);
107+
this.customizer.customize(factory);
108+
verifyConnectionTimeout(factory, 0);
109+
}
100110

111+
@Test
112+
public void setConnectionTimeout() {
113+
setupConnectionTimeout(Duration.ofSeconds(1));
101114
NettyReactiveWebServerFactory factory = mock(NettyReactiveWebServerFactory.class);
102115
this.customizer.customize(factory);
103-
verify(factory, times(1)).addServerCustomizers(any(NettyServerCustomizer.class));
116+
verifyConnectionTimeout(factory, 1000);
117+
}
118+
119+
@SuppressWarnings("unchecked")
120+
private void verifyConnectionTimeout(NettyReactiveWebServerFactory factory, Integer expected) {
121+
if (expected == null) {
122+
verify(factory, never()).addServerCustomizers(any(NettyServerCustomizer.class));
123+
return;
124+
}
125+
verify(factory, times(1)).addServerCustomizers(this.customizerCaptor.capture());
126+
NettyServerCustomizer serverCustomizer = this.customizerCaptor.getValue();
127+
HttpServer httpServer = serverCustomizer.apply(HttpServer.create());
128+
TcpServer tcpConfiguration = ReflectionTestUtils.invokeMethod(httpServer, "tcpConfiguration");
129+
ServerBootstrap bootstrap = tcpConfiguration.configure();
130+
Map<Object, Object> options = (Map<Object, Object>) ReflectionTestUtils.getField(bootstrap, "options");
131+
assertThat(options).containsEntry(ChannelOption.CONNECT_TIMEOUT_MILLIS, expected);
132+
}
133+
134+
private void setupConnectionTimeout(Duration connectionTimeout) {
135+
this.serverProperties.setUseForwardHeaders(null);
136+
this.serverProperties.setMaxHttpHeaderSize(null);
137+
this.serverProperties.setConnectionTimeout(connectionTimeout);
104138
}
105139

106140
}

0 commit comments

Comments
 (0)