Polish "Configure ErrorReportValve not to report stack traces"

Closes gh-11790

Author: wilkinsona

spring-boot-autoconfigure/src/main/java/org/springframework/boot/autoconfigure/web/ServerProperties.java

@@ -1,5 +1,5 @@
 /*
- * Copyright 2012-2017 the original author or authors.
+ * Copyright 2012-2018 the original author or authors.
  *
  * Licensed under the Apache License, Version 2.0 (the "License");
  * you may not use this file except in compliance with the License.
@@ -38,7 +38,6 @@
 import org.apache.catalina.valves.AccessLogValve;
 import org.apache.catalina.valves.ErrorReportValve;
 import org.apache.catalina.valves.RemoteIpValve;
-import org.apache.commons.logging.LogFactory;
 import org.apache.coyote.AbstractProtocol;
 import org.apache.coyote.ProtocolHandler;
 import org.apache.coyote.http11.AbstractHttp11Protocol;
@@ -861,27 +860,27 @@ void customizeTomcat(ServerProperties serverProperties,
 			if (!ObjectUtils.isEmpty(this.additionalTldSkipPatterns)) {
 				factory.getTldSkipPatterns().addAll(this.additionalTldSkipPatterns);
 			}
-			if (serverProperties.getError().getIncludeStacktrace() == ErrorProperties.IncludeStacktrace.NEVER) {
-				factory.addContextCustomizers(new TomcatContextCustomizer() {
-					@Override
-					public void customize(Context context) {
-						// org.apache.catalina.core.StandardHost() adds ErrorReportValve
-						// with default options if not there yet, so adding a properly
-						// configured one.
-						ErrorReportValve valve = new ErrorReportValve();
-						valve.setShowServerInfo(false); // disable server name and version
-						valve.setShowReport(false); // disable exception
-						if (context.getParent() != null) {
-							context.getParent().getPipeline().addValve(valve);
-						} else {
-							LogFactory.getLog(context.getClass()).warn("Parent of " + context
-									+ " is not set, skip ErrorReportValve configuration");
-						}
-					}
-				});
+			if (serverProperties.getError()
+					.getIncludeStacktrace() == ErrorProperties.IncludeStacktrace.NEVER) {
+				customizeErrorReportValve(factory);
 			}
 		}
 
+		private void customizeErrorReportValve(
+				TomcatEmbeddedServletContainerFactory factory) {
+			factory.addContextCustomizers(new TomcatContextCustomizer() {
+
+				@Override
+				public void customize(Context context) {
+					ErrorReportValve valve = new ErrorReportValve();
+					valve.setShowServerInfo(false);
+					valve.setShowReport(false);
+					context.getParent().getPipeline().addValve(valve);
+				}
+
+			});
+		}
+
 		private void customizeAcceptCount(TomcatEmbeddedServletContainerFactory factory) {
 			factory.addConnectorCustomizers(new TomcatConnectorCustomizer() {
 

spring-boot-autoconfigure/src/test/java/org/springframework/boot/autoconfigure/web/ServerPropertiesTests.java

@@ -1,5 +1,5 @@
 /*
- * Copyright 2012-2017 the original author or authors.
+ * Copyright 2012-2018 the original author or authors.
  *
  * Licensed under the Apache License, Version 2.0 (the "License");
  * you may not use this file except in compliance with the License.
@@ -32,6 +32,7 @@
 import org.apache.catalina.Context;
 import org.apache.catalina.Valve;
 import org.apache.catalina.valves.AccessLogValve;
+import org.apache.catalina.valves.ErrorReportValve;
 import org.apache.catalina.valves.RemoteIpValve;
 import org.apache.coyote.AbstractProtocol;
 import org.junit.Before;
@@ -45,7 +46,6 @@
 import org.springframework.boot.context.embedded.ConfigurableEmbeddedServletContainer;
 import org.springframework.boot.context.embedded.EmbeddedServletContainer;
 import org.springframework.boot.context.embedded.jetty.JettyEmbeddedServletContainerFactory;
-import org.springframework.boot.context.embedded.tomcat.TomcatContextCustomizer;
 import org.springframework.boot.context.embedded.tomcat.TomcatEmbeddedServletContainer;
 import org.springframework.boot.context.embedded.tomcat.TomcatEmbeddedServletContainerFactory;
 import org.springframework.boot.context.embedded.undertow.UndertowEmbeddedServletContainerFactory;
@@ -235,6 +235,25 @@ public void testTomcatBinding() throws Exception {
 		assertThat(tomcat.getBackgroundProcessorDelay()).isEqualTo(10);
 	}
 
+	@Test
+	public void errorReportValveIsConfiguredToNotReportStackTraces() {
+		TomcatEmbeddedServletContainerFactory tomcatContainer = new TomcatEmbeddedServletContainerFactory();
+		Map<String, String> map = new HashMap<String, String>();
+		bindProperties(map);
+		this.properties.customize(tomcatContainer);
+		Valve[] valves = ((TomcatEmbeddedServletContainer) tomcatContainer
+				.getEmbeddedServletContainer()).getTomcat().getHost().getPipeline()
+						.getValves();
+		assertThat(valves).hasAtLeastOneElementOfType(ErrorReportValve.class);
+		for (Valve valve : valves) {
+			if (valve instanceof ErrorReportValve) {
+				ErrorReportValve errorReportValve = (ErrorReportValve) valve;
+				assertThat(errorReportValve.isShowReport()).isFalse();
+				assertThat(errorReportValve.isShowServerInfo()).isFalse();
+			}
+		}
+	}
+
 	@Test
 	public void redirectContextRootIsNotConfiguredByDefault() throws Exception {
 		bindProperties(new HashMap<String, String>());
@@ -249,14 +268,10 @@ public void redirectContextRootCanBeConfigured() throws Exception {
 		bindProperties(map);
 		ServerProperties.Tomcat tomcat = this.properties.getTomcat();
 		assertThat(tomcat.getRedirectContextRoot()).isEqualTo(false);
-		TomcatEmbeddedServletContainerFactory container = new TomcatEmbeddedServletContainerFactory();
-		this.properties.customize(container);
-		Context context = mock(Context.class);
-		for (TomcatContextCustomizer customizer : container
-				.getTomcatContextCustomizers()) {
-			customizer.customize(context);
-		}
-		verify(context).setMapperContextRootRedirectEnabled(false);
+		TomcatEmbeddedServletContainerFactory factory = new TomcatEmbeddedServletContainerFactory();
+		Context context = (Context) ((TomcatEmbeddedServletContainer) factory
+				.getEmbeddedServletContainer()).getTomcat().getHost().findChildren()[0];
+		assertThat(context.getMapperContextRootRedirectEnabled()).isTrue();
 	}
 
 	@Test

spring-boot/src/main/java/org/springframework/boot/context/embedded/tomcat/TomcatEmbeddedServletContainerFactory.java

@@ -1,5 +1,5 @@
 /*
- * Copyright 2012-2017 the original author or authors.
+ * Copyright 2012-2018 the original author or authors.
  *
  * Licensed under the Apache License, Version 2.0 (the "License");
  * you may not use this file except in compliance with the License.

spring-boot/src/test/java/org/springframework/boot/context/embedded/tomcat/TomcatEmbeddedServletContainerFactoryTests.java

@@ -1,5 +1,5 @@
 /*
- * Copyright 2012-2017 the original author or authors.
+ * Copyright 2012-2018 the original author or authors.
  *
  * Licensed under the Apache License, Version 2.0 (the "License");
  * you may not use this file except in compliance with the License.
@@ -53,6 +53,8 @@
 import org.junit.Rule;
 import org.junit.Test;
 import org.mockito.InOrder;
+import org.mockito.invocation.InvocationOnMock;
+import org.mockito.stubbing.Answer;
 
 import org.springframework.boot.context.embedded.AbstractEmbeddedServletContainerFactory;
 import org.springframework.boot.context.embedded.AbstractEmbeddedServletContainerFactoryTests;
@@ -68,6 +70,7 @@
 import static org.mockito.BDDMockito.given;
 import static org.mockito.Matchers.any;
 import static org.mockito.Matchers.anyObject;
+import static org.mockito.Mockito.doAnswer;
 import static org.mockito.Mockito.inOrder;
 import static org.mockito.Mockito.mock;
 import static org.mockito.Mockito.verify;
@@ -146,6 +149,25 @@ public void tomcatCustomizers() throws Exception {
 		}
 	}
 
+	@Test
+	public void contextIsAddedToHostBeforeCustomizersAreCalled() throws Exception {
+		TomcatEmbeddedServletContainerFactory factory = getFactory();
+		TomcatContextCustomizer customizer = mock(TomcatContextCustomizer.class);
+		doAnswer(new Answer<Void>() {
+
+			@Override
+			public Void answer(InvocationOnMock invocation) throws Throwable {
+				assertThat(((Context) invocation.getArguments()[0]).getParent())
+						.isNotNull();
+				return null;
+			}
+
+		}).when(customizer).customize(any(Context.class));
+		factory.addContextCustomizers(customizer);
+		this.container = factory.getEmbeddedServletContainer();
+		verify(customizer).customize(any(Context.class));
+	}
+
 	@Test
 	public void tomcatConnectorCustomizers() throws Exception {
 		TomcatEmbeddedServletContainerFactory factory = getFactory();