Skip to content

Added support for additional Tomcat access log configuration properties #16039

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
wants to merge 1 commit into from
Closed

Added support for additional Tomcat access log configuration properties #16039

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
85 changes: 85 additions & 0 deletions ...oconfigure/src/main/java/org/springframework/boot/autoconfigure/web/ServerProperties.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -57,6 +57,7 @@
* @author Olivier Lamy
* @author Chentao Qu
* @author Artsiom Yudovin
* @author Andrew McGhie
*/
@ConfigurationProperties(prefix = "server", ignoreUnknownFields = true)
public class ServerProperties {
Expand Down Expand Up @@ -614,6 +615,42 @@ public static class Accesslog {
*/
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 <code>null</code>, the system
* default character set will be used. An empty string will be treated as
* <code>null</code> 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();
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We can't really do that as the sole purpose of the field init is to express a default value that isn't dependant of the environment. I'll remove that as part of the polish.


public boolean isEnabled() {
return this.enabled;
}
Expand Down Expand Up @@ -702,6 +739,54 @@ public void setBuffered(boolean buffered) {
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;
}

}

/**
Expand Down
7 changes: 7 additions & 0 deletions ...org/springframework/boot/autoconfigure/web/embedded/TomcatWebServerFactoryCustomizer.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,7 @@
* @author Phillip Webb
* @author Artsiom Yudovin
* @author Chentao Qu
* @author Andrew McGhie
* @since 2.0.0
*/
public class TomcatWebServerFactoryCustomizer implements
Expand Down Expand Up @@ -264,6 +265,12 @@ private void customizeAccessLog(ConfigurableTomcatWebServerFactory factory) {
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);
}

Expand Down
13 changes: 13 additions & 0 deletions ...igure/src/test/java/org/springframework/boot/autoconfigure/web/ServerPropertiesTests.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -72,6 +72,7 @@
* @author Eddú Meléndez
* @author Quinten De Swaef
* @author Venil Noronha
* @author Andrew McGhie
*/
public class ServerPropertiesTests {

Expand Down Expand Up @@ -117,6 +118,12 @@ public void testTomcatBinding() {
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}");
Expand All @@ -129,6 +136,12 @@ public void testTomcatBinding() {
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())
Expand Down
90 changes: 89 additions & 1 deletion ...pringframework/boot/autoconfigure/web/embedded/TomcatWebServerFactoryCustomizerTests.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -49,6 +50,7 @@
* @author Rob Tompkins
* @author Artsiom Yudovin
* @author Stephane Nicoll
* @author Andrew McGhie
*/
public class TomcatWebServerFactoryCustomizerTests {

Expand Down Expand Up @@ -335,14 +337,100 @@ public void accessLogMaxDaysDefault() {
}

@Test
public void accessLoMaxDaysCanBeRedefined() {
public void accessLogMaxDaysCanBeRedefined() {
bind("server.tomcat.accesslog.enabled=true",
"server.tomcat.accesslog.max-days=20");
TomcatServletWebServerFactory factory = customizeAndGetFactory();
assertThat(((AccessLogValve) factory.getEngineValves().iterator().next())
.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);
Expand Down