From f823ae488da78f8e3a6d2f506abf4db4937e9956 Mon Sep 17 00:00:00 2001 From: Andrew McGhie Date: Tue, 26 Feb 2019 23:13:49 +1000 Subject: [PATCH] Complete support for customizing Tomcat's access log See gh-16039 --- .../autoconfigure/web/ServerProperties.java | 85 ++++++++++++++++++ .../TomcatWebServerFactoryCustomizer.java | 7 ++ .../web/ServerPropertiesTests.java | 13 +++ ...TomcatWebServerFactoryCustomizerTests.java | 88 +++++++++++++++++++ 4 files changed, 193 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 9e6f48f96f..0f09c85204 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 @@ -56,6 +56,7 @@ import org.springframework.util.unit.DataSize; * @author Olivier Lamy * @author Chentao Qu * @author Artsiom Yudovin + * @author Andrew McGhie */ @ConfigurationProperties(prefix = "server", ignoreUnknownFields = true) public class ServerProperties { @@ -598,6 +599,42 @@ public class ServerProperties { */ private boolean buffered = true; + /** + * Check for log file existence so it can be recreated it if an external + * process/agent has renamed it. + */ + private boolean checkExists = false; + + /** + * If the value returned from ServletRequest.getAttribute(conditionIf) yields + * a null value, logging of the request will be skipped. + */ + private String conditionIf; + + /** + * If the value returned from ServletRequest.getAttribute(conditionUnless) + * yields a non-null value, the logging of the request will be skipped. + */ + private String conditionUnless; + + /** + * Character set used by the log file. If it is null, the system + * default character set will be used. An empty string will be treated as + * null when this property is assigned. + */ + private String encoding; + + /** + * Use IPv6 canonical representation format as defined by RFC 5952 in output. + */ + private boolean ipv6Canonical = false; + + /** + * Set the locale used to format timestamps in log entries and in log file + * name suffix. + */ + private String locale = Locale.getDefault().toString(); + public boolean isEnabled() { return this.enabled; } @@ -686,6 +723,54 @@ public class ServerProperties { this.buffered = buffered; } + public boolean isCheckExists() { + return this.checkExists; + } + + public void setCheckExists(boolean checkExists) { + this.checkExists = checkExists; + } + + public String getConditionIf() { + return this.conditionIf; + } + + public void setConditionIf(String conditionIf) { + this.conditionIf = conditionIf; + } + + public String getConditionUnless() { + return this.conditionUnless; + } + + public void setConditionUnless(String conditionUnless) { + this.conditionUnless = conditionUnless; + } + + public String getEncoding() { + return this.encoding; + } + + public void setEncoding(String encoding) { + this.encoding = encoding; + } + + public boolean isIpv6Canonical() { + return this.ipv6Canonical; + } + + public void setIpv6Canonical(boolean ipv6Canonical) { + this.ipv6Canonical = ipv6Canonical; + } + + public String getLocale() { + return this.locale; + } + + public void setLocale(String locale) { + this.locale = locale; + } + } /** 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 e905737cbe..e674a8d22c 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 @@ -49,6 +49,7 @@ import org.springframework.util.unit.DataSize; * @author Phillip Webb * @author Artsiom Yudovin * @author Chentao Qu + * @author Andrew McGhie * @since 2.0.0 */ public class TomcatWebServerFactoryCustomizer implements @@ -257,6 +258,12 @@ public class TomcatWebServerFactoryCustomizer implements tomcatProperties.getAccesslog().isRequestAttributesEnabled()); valve.setRotatable(tomcatProperties.getAccesslog().isRotate()); valve.setBuffered(tomcatProperties.getAccesslog().isBuffered()); + valve.setCheckExists(tomcatProperties.getAccesslog().isCheckExists()); + valve.setConditionIf(tomcatProperties.getAccesslog().getConditionIf()); + valve.setConditionUnless(tomcatProperties.getAccesslog().getConditionUnless()); + valve.setEncoding(tomcatProperties.getAccesslog().getEncoding()); + valve.setIpv6Canonical(tomcatProperties.getAccesslog().isIpv6Canonical()); + valve.setLocale(tomcatProperties.getAccesslog().getLocale()); factory.addEngineValves(valve); } 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 1d03829723..397244ac80 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 @@ -72,6 +72,7 @@ import static org.assertj.core.api.Assertions.assertThat; * @author EddĂș MelĂ©ndez * @author Quinten De Swaef * @author Venil Noronha + * @author Andrew McGhie */ public class ServerPropertiesTests { @@ -117,6 +118,12 @@ public class ServerPropertiesTests { map.put("server.tomcat.accesslog.rename-on-rotate", "true"); map.put("server.tomcat.accesslog.request-attributes-enabled", "true"); map.put("server.tomcat.accesslog.suffix", "-bar.log"); + map.put("server.tomcat.accesslog.checkExists", "true"); + map.put("server.tomcat.accesslog.conditionIf", "foo"); + map.put("server.tomcat.accesslog.conditionUnless", "bar"); + map.put("server.tomcat.accesslog.encoding", "UTF-8"); + map.put("server.tomcat.accesslog.ipv6Canonical", "true"); + map.put("server.tomcat.accesslog.locale", "en-AU"); map.put("server.tomcat.protocol-header", "X-Forwarded-Protocol"); map.put("server.tomcat.remote-ip-header", "Remote-Ip"); map.put("server.tomcat.internal-proxies", "10\\.\\d{1,3}\\.\\d{1,3}\\.\\d{1,3}"); @@ -129,6 +136,12 @@ public class ServerPropertiesTests { assertThat(tomcat.getAccesslog().isRenameOnRotate()).isTrue(); assertThat(tomcat.getAccesslog().isRequestAttributesEnabled()).isTrue(); assertThat(tomcat.getAccesslog().getSuffix()).isEqualTo("-bar.log"); + assertThat(tomcat.getAccesslog().isCheckExists()).isEqualTo(true); + assertThat(tomcat.getAccesslog().getConditionIf()).isEqualTo("foo"); + assertThat(tomcat.getAccesslog().getConditionUnless()).isEqualTo("bar"); + assertThat(tomcat.getAccesslog().getEncoding()).isEqualTo("UTF-8"); + assertThat(tomcat.getAccesslog().isIpv6Canonical()).isTrue(); + assertThat(tomcat.getAccesslog().getLocale()).isEqualTo("en-AU"); assertThat(tomcat.getRemoteIpHeader()).isEqualTo("Remote-Ip"); assertThat(tomcat.getProtocolHeader()).isEqualTo("X-Forwarded-Protocol"); assertThat(tomcat.getInternalProxies()) 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 06b51d32dc..032e700743 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 @@ -16,6 +16,7 @@ package org.springframework.boot.autoconfigure.web.embedded; +import java.util.Locale; import java.util.function.Consumer; import org.apache.catalina.Context; @@ -49,6 +50,7 @@ import static org.assertj.core.api.Assertions.assertThat; * @author Rob Tompkins * @author Artsiom Yudovin * @author Stephane Nicoll + * @author Andrew McGhie */ public class TomcatWebServerFactoryCustomizerTests { @@ -333,6 +335,92 @@ public class TomcatWebServerFactoryCustomizerTests { .getMaxDays()).isEqualTo(20); } + @Test + public void accessLogCheckExistsDefault() { + bind("server.tomcat.accesslog.enabled=true"); + TomcatServletWebServerFactory factory = customizeAndGetFactory(); + assertThat(((AccessLogValve) factory.getEngineValves().iterator().next()) + .isCheckExists()).isFalse(); + } + + @Test + public void accessLogCheckExistsSpecified() { + bind("server.tomcat.accesslog.enabled=true", + "server.tomcat.accesslog.checkExists=true"); + TomcatServletWebServerFactory factory = customizeAndGetFactory(); + assertThat(((AccessLogValve) factory.getEngineValves().iterator().next()) + .isCheckExists()).isTrue(); + } + + @Test + public void accessLogConditionCanBeSpecified() { + bind("server.tomcat.accesslog.enabled=true", + "server.tomcat.accesslog.conditionIf=foo", + "server.tomcat.accesslog.conditionUnless=bar"); + TomcatServletWebServerFactory factory = customizeAndGetFactory(); + assertThat(((AccessLogValve) factory.getEngineValves().iterator().next()) + .getConditionIf()).isEqualTo("foo"); + assertThat(((AccessLogValve) factory.getEngineValves().iterator().next()) + .getConditionUnless()).isEqualTo("bar"); + assertThat(((AccessLogValve) factory.getEngineValves().iterator().next()) + .getCondition()).describedAs( + "value of condition should equal conditionUnless - provided for backwards compatibility") + .isEqualTo("bar"); + } + + @Test + public void accessLogEncodingIsNullWhenNotSpecified() { + bind("server.tomcat.accesslog.enabled=true"); + TomcatServletWebServerFactory factory = customizeAndGetFactory(); + assertThat(((AccessLogValve) factory.getEngineValves().iterator().next()) + .getEncoding()).isNull(); + } + + @Test + public void accessLogEncodingCanBeSpecified() { + bind("server.tomcat.accesslog.enabled=true", + "server.tomcat.accesslog.encoding=UTF-8"); + TomcatServletWebServerFactory factory = customizeAndGetFactory(); + assertThat(((AccessLogValve) factory.getEngineValves().iterator().next()) + .getEncoding()).isEqualTo("UTF-8"); + } + + @Test + public void accessLogDoesNotUseIpv6CanonicalFormatByDefault() { + bind("server.tomcat.accesslog.enabled=true"); + TomcatServletWebServerFactory factory = customizeAndGetFactory(); + assertThat(((AccessLogValve) factory.getEngineValves().iterator().next()) + .getIpv6Canonical()).isFalse(); + } + + @Test + public void accessLogwithIpv6CanonicalSet() { + bind("server.tomcat.accesslog.enabled=true", + "server.tomcat.accesslog.ipv6Canonical=true"); + TomcatServletWebServerFactory factory = customizeAndGetFactory(); + assertThat(((AccessLogValve) factory.getEngineValves().iterator().next()) + .getIpv6Canonical()).isTrue(); + } + + @Test + public void accessLogWithDefaultLocale() { + bind("server.tomcat.accesslog.enabled=true"); + TomcatServletWebServerFactory factory = customizeAndGetFactory(); + assertThat(((AccessLogValve) factory.getEngineValves().iterator().next()) + .getLocale()).isEqualTo(Locale.getDefault().toString()); + } + + @Test + public void accessLogLocaleCanBeSpecified() { + String locale = "en_AU".equals(Locale.getDefault().toString()) ? "en_US" + : "en_AU"; + bind("server.tomcat.accesslog.enabled=true", + "server.tomcat.accesslog.locale=" + locale); + TomcatServletWebServerFactory factory = customizeAndGetFactory(); + assertThat(((AccessLogValve) factory.getEngineValves().iterator().next()) + .getLocale()).isEqualTo(locale); + } + private void bind(String... inlinedProperties) { TestPropertySourceUtils.addInlinedPropertiesToEnvironment(this.environment, inlinedProperties);