From e5a539d80e35e1b577d4c1212c1a26fe73102a14 Mon Sep 17 00:00:00 2001 From: David Byron Date: Thu, 29 Apr 2021 11:22:37 -0700 Subject: [PATCH 1/2] Add configuration property for Tomcat's rejectIllegalHeader See gh-26311 --- .../boot/autoconfigure/web/ServerProperties.java | 13 +++++++++++++ .../embedded/TomcatWebServerFactoryCustomizer.java | 12 ++++++++++++ .../autoconfigure/web/ServerPropertiesTests.java | 7 +++++++ .../TomcatWebServerFactoryCustomizerTests.java | 8 ++++++++ 4 files changed, 40 insertions(+) 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 dccdf5073a4..508fac0bbef 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 @@ -424,6 +424,11 @@ public class ServerProperties { */ private final Remoteip remoteip = new Remoteip(); + /** + * reject illegal header setting. + */ + private Boolean rejectIllegalHeader; + public DataSize getMaxHttpFormPostSize() { return this.maxHttpFormPostSize; } @@ -572,6 +577,14 @@ public class ServerProperties { return this.remoteip; } + public Boolean getRejectIllegalHeader() { + return this.rejectIllegalHeader; + } + + public void setRejectIllegalHeader(Boolean rejectIllegalHeader) { + this.rejectIllegalHeader = rejectIllegalHeader; + } + /** * Tomcat access log properties. */ 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 9e44dd4bfa5..1545a3c879e 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 @@ -117,6 +117,8 @@ public class TomcatWebServerFactoryCustomizer .to((relaxedChars) -> customizeRelaxedPathChars(factory, relaxedChars)); propertyMapper.from(tomcatProperties::getRelaxedQueryChars).as(this::joinCharacters).whenHasText() .to((relaxedChars) -> customizeRelaxedQueryChars(factory, relaxedChars)); + propertyMapper.from(tomcatProperties::getRejectIllegalHeader).whenNonNull() + .to((rejectIllegalHeader) -> customizeRejectIllegalHeader(factory, rejectIllegalHeader)); customizeStaticResources(factory); customizeErrorReportValve(properties.getError(), factory); } @@ -192,6 +194,16 @@ public class TomcatWebServerFactoryCustomizer factory.addConnectorCustomizers((connector) -> connector.setProperty("relaxedQueryChars", relaxedChars)); } + private void customizeRejectIllegalHeader(ConfigurableTomcatWebServerFactory factory, boolean rejectIllegalHeader) { + factory.addConnectorCustomizers((connector) -> { + ProtocolHandler handler = connector.getProtocolHandler(); + if (handler instanceof AbstractHttp11Protocol) { + AbstractHttp11Protocol protocol = (AbstractHttp11Protocol) handler; + protocol.setRejectIllegalHeader(rejectIllegalHeader); + } + }); + } + private String joinCharacters(List content) { return content.stream().map(String::valueOf).collect(Collectors.joining()); } diff --git a/spring-boot-project/spring-boot-autoconfigure/src/test/java/org/springframework/boot/autoconfigure/web/ServerPropertiesTests.java b/spring-boot-project/spring-boot-autoconfigure/src/test/java/org/springframework/boot/autoconfigure/web/ServerPropertiesTests.java index 7f1ffd735a2..ec551783eb6 100644 --- a/spring-boot-project/spring-boot-autoconfigure/src/test/java/org/springframework/boot/autoconfigure/web/ServerPropertiesTests.java +++ b/spring-boot-project/spring-boot-autoconfigure/src/test/java/org/springframework/boot/autoconfigure/web/ServerPropertiesTests.java @@ -130,6 +130,7 @@ class ServerPropertiesTests { map.put("server.tomcat.remoteip.protocol-header", "X-Forwarded-Protocol"); map.put("server.tomcat.remoteip.remote-ip-header", "Remote-Ip"); map.put("server.tomcat.remoteip.internal-proxies", "10\\.\\d{1,3}\\.\\d{1,3}\\.\\d{1,3}"); + map.put("server.tomcat.reject-illegal-header", "true"); map.put("server.tomcat.background-processor-delay", "10"); map.put("server.tomcat.relaxed-path-chars", "|,<"); map.put("server.tomcat.relaxed-query-chars", "^ , | "); @@ -152,6 +153,7 @@ class ServerPropertiesTests { assertThat(tomcat.getRemoteip().getRemoteIpHeader()).isEqualTo("Remote-Ip"); assertThat(tomcat.getRemoteip().getProtocolHeader()).isEqualTo("X-Forwarded-Protocol"); assertThat(tomcat.getRemoteip().getInternalProxies()).isEqualTo("10\\.\\d{1,3}\\.\\d{1,3}\\.\\d{1,3}"); + assertThat(tomcat.getRejectIllegalHeader()).isTrue(); assertThat(tomcat.getBackgroundProcessorDelay()).hasSeconds(10); assertThat(tomcat.getRelaxedPathChars()).containsExactly('|', '<'); assertThat(tomcat.getRelaxedQueryChars()).containsExactly('^', '|'); @@ -405,6 +407,11 @@ class ServerPropertiesTests { .isEqualTo(new RemoteIpValve().getInternalProxies()); } + @Test + void tomcatRejectIllegalHeaderDefaultsToNull() { + assertThat(this.properties.getTomcat().getRejectIllegalHeader()).isNull(); + } + @Test void tomcatUseRelativeRedirectsDefaultsToFalse() { assertThat(this.properties.getTomcat().isUseRelativeRedirects()).isFalse(); 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 6a3742a1722..80e5f8fa561 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 @@ -320,6 +320,14 @@ class TomcatWebServerFactoryCustomizerTests { assertThat(factory.getEngineValves()).isEmpty(); } + @Test + void testCustomizeRejectIllegalHeader() { + bind("server.tomcat.reject-illegal-header=false"); + customizeAndRunServer((server) -> assertThat( + ((AbstractHttp11Protocol) server.getTomcat().getConnector().getProtocolHandler()) + .getRejectIllegalHeader()).isFalse()); + } + @Test void errorReportValveIsConfiguredToNotReportStackTraces() { TomcatWebServer server = customizeAndGetServer(); From d847f4c69e982558e72bc64ad899e61bf3986d05 Mon Sep 17 00:00:00 2001 From: Stephane Nicoll Date: Mon, 31 May 2021 09:27:12 +0200 Subject: [PATCH 2/2] Polish "Add configuration property for Tomcat's rejectIllegalHeader" See gh-26311 --- .../autoconfigure/web/ServerProperties.java | 26 +++++++++---------- .../TomcatWebServerFactoryCustomizer.java | 2 +- .../web/ServerPropertiesTests.java | 9 ++++--- 3 files changed, 19 insertions(+), 18 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 508fac0bbef..027551b6c7a 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 @@ -409,6 +409,11 @@ public class ServerProperties { */ private Duration connectionTimeout; + /** + * Whether to reject requests with illegal header names or values. + */ + private boolean rejectIllegalHeader = true; + /** * Static resource configuration. */ @@ -424,11 +429,6 @@ public class ServerProperties { */ private final Remoteip remoteip = new Remoteip(); - /** - * reject illegal header setting. - */ - private Boolean rejectIllegalHeader; - public DataSize getMaxHttpFormPostSize() { return this.maxHttpFormPostSize; } @@ -565,6 +565,14 @@ public class ServerProperties { this.connectionTimeout = connectionTimeout; } + public boolean isRejectIllegalHeader() { + return this.rejectIllegalHeader; + } + + public void setRejectIllegalHeader(boolean rejectIllegalHeader) { + this.rejectIllegalHeader = rejectIllegalHeader; + } + public Resource getResource() { return this.resource; } @@ -577,14 +585,6 @@ public class ServerProperties { return this.remoteip; } - public Boolean getRejectIllegalHeader() { - return this.rejectIllegalHeader; - } - - public void setRejectIllegalHeader(Boolean rejectIllegalHeader) { - this.rejectIllegalHeader = rejectIllegalHeader; - } - /** * Tomcat access log properties. */ 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 1545a3c879e..b55d2d9a66f 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 @@ -117,7 +117,7 @@ public class TomcatWebServerFactoryCustomizer .to((relaxedChars) -> customizeRelaxedPathChars(factory, relaxedChars)); propertyMapper.from(tomcatProperties::getRelaxedQueryChars).as(this::joinCharacters).whenHasText() .to((relaxedChars) -> customizeRelaxedQueryChars(factory, relaxedChars)); - propertyMapper.from(tomcatProperties::getRejectIllegalHeader).whenNonNull() + propertyMapper.from(tomcatProperties::isRejectIllegalHeader) .to((rejectIllegalHeader) -> customizeRejectIllegalHeader(factory, rejectIllegalHeader)); customizeStaticResources(factory); customizeErrorReportValve(properties.getError(), factory); diff --git a/spring-boot-project/spring-boot-autoconfigure/src/test/java/org/springframework/boot/autoconfigure/web/ServerPropertiesTests.java b/spring-boot-project/spring-boot-autoconfigure/src/test/java/org/springframework/boot/autoconfigure/web/ServerPropertiesTests.java index ec551783eb6..6234688ebc5 100644 --- a/spring-boot-project/spring-boot-autoconfigure/src/test/java/org/springframework/boot/autoconfigure/web/ServerPropertiesTests.java +++ b/spring-boot-project/spring-boot-autoconfigure/src/test/java/org/springframework/boot/autoconfigure/web/ServerPropertiesTests.java @@ -130,7 +130,7 @@ class ServerPropertiesTests { map.put("server.tomcat.remoteip.protocol-header", "X-Forwarded-Protocol"); map.put("server.tomcat.remoteip.remote-ip-header", "Remote-Ip"); map.put("server.tomcat.remoteip.internal-proxies", "10\\.\\d{1,3}\\.\\d{1,3}\\.\\d{1,3}"); - map.put("server.tomcat.reject-illegal-header", "true"); + map.put("server.tomcat.reject-illegal-header", "false"); map.put("server.tomcat.background-processor-delay", "10"); map.put("server.tomcat.relaxed-path-chars", "|,<"); map.put("server.tomcat.relaxed-query-chars", "^ , | "); @@ -153,7 +153,7 @@ class ServerPropertiesTests { assertThat(tomcat.getRemoteip().getRemoteIpHeader()).isEqualTo("Remote-Ip"); assertThat(tomcat.getRemoteip().getProtocolHeader()).isEqualTo("X-Forwarded-Protocol"); assertThat(tomcat.getRemoteip().getInternalProxies()).isEqualTo("10\\.\\d{1,3}\\.\\d{1,3}\\.\\d{1,3}"); - assertThat(tomcat.getRejectIllegalHeader()).isTrue(); + assertThat(tomcat.isRejectIllegalHeader()).isFalse(); assertThat(tomcat.getBackgroundProcessorDelay()).hasSeconds(10); assertThat(tomcat.getRelaxedPathChars()).containsExactly('|', '<'); assertThat(tomcat.getRelaxedQueryChars()).containsExactly('^', '|'); @@ -408,8 +408,9 @@ class ServerPropertiesTests { } @Test - void tomcatRejectIllegalHeaderDefaultsToNull() { - assertThat(this.properties.getTomcat().getRejectIllegalHeader()).isNull(); + void tomcatRejectIllegalHeaderMatchesProtocolDefault() throws Exception { + assertThat(getDefaultProtocol()).hasFieldOrPropertyWithValue("rejectIllegalHeader", + this.properties.getTomcat().isRejectIllegalHeader()); } @Test