From c3571d416ad4e4bec688f7272e3840b1d758edef Mon Sep 17 00:00:00 2001
From: Andy Wilkinson <awilkinson@pivotal.io>
Date: Mon, 30 Mar 2015 10:56:07 +0100
Subject: [PATCH] Improve YAML-based configuration of Tomcat compression
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Tomcat uses the strings “on” and “off” to enable and disable
compression. YAML interprets on as true and off as false, leaving
ServerProperties.Tomcat.compression configured with “true” and “false”
respectively. One solution is to use “on” rather than on and “off”
rather than off in the YAML file but users may not realise that they
need to do so.

This commit updates the connector customiser that configures compression
to map “true” to “on” and “false” to “off”.

Closes gh-2737
---
 .../autoconfigure/web/ServerProperties.java   | 12 ++++-
 .../web/ServerPropertiesTests.java            | 53 +++++++++++++------
 2 files changed, 47 insertions(+), 18 deletions(-)

diff --git a/spring-boot-autoconfigure/src/main/java/org/springframework/boot/autoconfigure/web/ServerProperties.java b/spring-boot-autoconfigure/src/main/java/org/springframework/boot/autoconfigure/web/ServerProperties.java
index 56d89c2acd..fa171d6718 100644
--- a/spring-boot-autoconfigure/src/main/java/org/springframework/boot/autoconfigure/web/ServerProperties.java
+++ b/spring-boot-autoconfigure/src/main/java/org/springframework/boot/autoconfigure/web/ServerProperties.java
@@ -483,10 +483,20 @@ public class ServerProperties implements EmbeddedServletContainerCustomizer, Ord
 					if (handler instanceof AbstractHttp11Protocol) {
 						@SuppressWarnings("rawtypes")
 						AbstractHttp11Protocol protocol = (AbstractHttp11Protocol) handler;
-						protocol.setCompression(Tomcat.this.compression);
+						protocol.setCompression(coerceCompression(Tomcat.this.compression));
 						protocol.setCompressableMimeTypes(Tomcat.this.compressableMimeTypes);
 					}
 				}
+
+				private String coerceCompression(String compression) {
+					if (Boolean.toString(true).equals(compression)) {
+						return "on";
+					}
+					else if (Boolean.toString(false).equals(compression)) {
+						return "off";
+					}
+					return compression;
+				}
 			});
 
 			if (this.accessLogEnabled) {
diff --git a/spring-boot-autoconfigure/src/test/java/org/springframework/boot/autoconfigure/web/ServerPropertiesTests.java b/spring-boot-autoconfigure/src/test/java/org/springframework/boot/autoconfigure/web/ServerPropertiesTests.java
index bd2a13d065..5ce1f3d812 100644
--- a/spring-boot-autoconfigure/src/test/java/org/springframework/boot/autoconfigure/web/ServerPropertiesTests.java
+++ b/spring-boot-autoconfigure/src/test/java/org/springframework/boot/autoconfigure/web/ServerPropertiesTests.java
@@ -31,6 +31,8 @@ import org.springframework.boot.context.embedded.ConfigurableEmbeddedServletCont
 import org.springframework.boot.context.embedded.tomcat.TomcatEmbeddedServletContainer;
 import org.springframework.boot.context.embedded.tomcat.TomcatEmbeddedServletContainerFactory;
 
+import static org.hamcrest.Matchers.equalTo;
+import static org.hamcrest.Matchers.is;
 import static org.hamcrest.core.IsInstanceOf.instanceOf;
 import static org.junit.Assert.assertEquals;
 import static org.junit.Assert.assertFalse;
@@ -197,25 +199,19 @@ public class ServerPropertiesTests {
 
 	@Test
 	public void customTomcatCompression() throws Exception {
-		Map<String, String> map = new HashMap<String, String>();
-		map.put("server.port", "0");
-		map.put("server.tomcat.compression", "on");
-		bindProperties(map);
+		assertThat("on", is(equalTo(configureCompression("on"))));
+	}
 
-		TomcatEmbeddedServletContainerFactory factory = new TomcatEmbeddedServletContainerFactory();
-		this.properties.customize(factory);
+	@Test
+	public void disableTomcatCompressionWithYaml() throws Exception {
+		// YAML interprets "off" as false, check that it's mapped back to off
+		assertThat("off", is(equalTo(configureCompression("false"))));
+	}
 
-		TomcatEmbeddedServletContainer container = (TomcatEmbeddedServletContainer) factory
-				.getEmbeddedServletContainer();
-
-		try {
-			AbstractHttp11Protocol<?> protocol = (AbstractHttp11Protocol<?>) container
-					.getTomcat().getConnector().getProtocolHandler();
-			assertEquals("on", protocol.getCompression());
-		}
-		finally {
-			container.stop();
-		}
+	@Test
+	public void enableTomcatCompressionWithYaml() throws Exception {
+		// YAML interprets "on" as true, check that it's mapped back to on
+		assertThat("on", is(equalTo(configureCompression("true"))));
 	}
 
 	@Test
@@ -246,4 +242,27 @@ public class ServerPropertiesTests {
 				map));
 	}
 
+	private String configureCompression(String compression) {
+		Map<String, String> map = new HashMap<String, String>();
+		map.put("server.port", "0");
+		// YAML interprets "on" as true
+		map.put("server.tomcat.compression", compression);
+		bindProperties(map);
+
+		TomcatEmbeddedServletContainerFactory factory = new TomcatEmbeddedServletContainerFactory();
+		this.properties.customize(factory);
+
+		TomcatEmbeddedServletContainer container = (TomcatEmbeddedServletContainer) factory
+				.getEmbeddedServletContainer();
+
+		try {
+			AbstractHttp11Protocol<?> protocol = (AbstractHttp11Protocol<?>) container
+					.getTomcat().getConnector().getProtocolHandler();
+			return protocol.getCompression();
+		}
+		finally {
+			container.stop();
+		}
+	}
+
 }