From 570b292df700c3bdd26805b729979e2e6601af6b Mon Sep 17 00:00:00 2001 From: Andy Wilkinson Date: Thu, 20 Oct 2016 21:02:21 +0100 Subject: [PATCH] Disable JspServlet's development mode by default This commit switches off the auto-configured JspServlet's development mode by default. Development mode is then switched on when DevTools is on the class path. Closes gh-7039 --- spring-boot-devtools/pom.xml | 5 +++ ...DevToolsPropertyDefaultsPostProcessor.java | 1 + .../LocalDevToolsAutoConfigurationTests.java | 32 ++++++++++++++++--- .../boot/context/embedded/JspServlet.java | 8 +++-- ...tEmbeddedServletContainerFactoryTests.java | 30 ++++++++++++++++- ...yEmbeddedServletContainerFactoryTests.java | 21 +++++------- ...tEmbeddedServletContainerFactoryTests.java | 25 ++++++--------- ...wEmbeddedServletContainerFactoryTests.java | 3 +- 8 files changed, 89 insertions(+), 36 deletions(-) diff --git a/spring-boot-devtools/pom.xml b/spring-boot-devtools/pom.xml index c467ca4b2ba..7d62b1bef0d 100644 --- a/spring-boot-devtools/pom.xml +++ b/spring-boot-devtools/pom.xml @@ -117,6 +117,11 @@ tomcat-embed-core test + + org.apache.tomcat.embed + tomcat-embed-jasper + test + org.eclipse.jetty.websocket websocket-client diff --git a/spring-boot-devtools/src/main/java/org/springframework/boot/devtools/env/DevToolsPropertyDefaultsPostProcessor.java b/spring-boot-devtools/src/main/java/org/springframework/boot/devtools/env/DevToolsPropertyDefaultsPostProcessor.java index 8f31f4f8749..806a4a637d6 100755 --- a/spring-boot-devtools/src/main/java/org/springframework/boot/devtools/env/DevToolsPropertyDefaultsPostProcessor.java +++ b/spring-boot-devtools/src/main/java/org/springframework/boot/devtools/env/DevToolsPropertyDefaultsPostProcessor.java @@ -52,6 +52,7 @@ public class DevToolsPropertyDefaultsPostProcessor implements EnvironmentPostPro properties.put("spring.resources.cache-period", "0"); properties.put("spring.template.provider.cache", "false"); properties.put("spring.mvc.log-resolved-exception", "true"); + properties.put("server.jsp-servlet.init-parameters.development", "true"); PROPERTIES = Collections.unmodifiableMap(properties); } diff --git a/spring-boot-devtools/src/test/java/org/springframework/boot/devtools/autoconfigure/LocalDevToolsAutoConfigurationTests.java b/spring-boot-devtools/src/test/java/org/springframework/boot/devtools/autoconfigure/LocalDevToolsAutoConfigurationTests.java index 41cebf86daa..eab60621d5d 100644 --- a/spring-boot-devtools/src/test/java/org/springframework/boot/devtools/autoconfigure/LocalDevToolsAutoConfigurationTests.java +++ b/spring-boot-devtools/src/test/java/org/springframework/boot/devtools/autoconfigure/LocalDevToolsAutoConfigurationTests.java @@ -21,6 +21,9 @@ import java.util.Collections; import java.util.HashMap; import java.util.Map; +import org.apache.catalina.Container; +import org.apache.catalina.core.StandardWrapper; +import org.apache.jasper.EmbeddedServletOptions; import org.junit.After; import org.junit.Rule; import org.junit.Test; @@ -30,7 +33,12 @@ import org.thymeleaf.templateresolver.TemplateResolver; import org.springframework.beans.factory.NoSuchBeanDefinitionException; import org.springframework.boot.SpringApplication; import org.springframework.boot.autoconfigure.thymeleaf.ThymeleafAutoConfiguration; +import org.springframework.boot.autoconfigure.web.EmbeddedServletContainerAutoConfiguration; import org.springframework.boot.autoconfigure.web.ResourceProperties; +import org.springframework.boot.autoconfigure.web.ServerProperties; +import org.springframework.boot.context.embedded.EmbeddedWebApplicationContext; +import org.springframework.boot.context.embedded.tomcat.TomcatEmbeddedServletContainer; +import org.springframework.boot.context.properties.EnableConfigurationProperties; import org.springframework.boot.devtools.classpath.ClassPathChangedEvent; import org.springframework.boot.devtools.classpath.ClassPathFileSystemWatcher; import org.springframework.boot.devtools.filewatch.ChangedFiles; @@ -234,6 +242,18 @@ public class LocalDevToolsAutoConfigurationTests { .containsKey(new File("src/test/java").getAbsoluteFile()); } + @Test + public void devToolsSwitchesJspServletToDevelopmentMode() { + this.context = initializeAndRun(Config.class); + TomcatEmbeddedServletContainer tomcatContainer = (TomcatEmbeddedServletContainer) ((EmbeddedWebApplicationContext) this.context) + .getEmbeddedServletContainer(); + Container context = tomcatContainer.getTomcat().getHost().findChildren()[0]; + StandardWrapper jspServletWrapper = (StandardWrapper) context.findChild("jsp"); + EmbeddedServletOptions options = (EmbeddedServletOptions) ReflectionTestUtils + .getField(jspServletWrapper.getServlet(), "options"); + assertThat(options.getDevelopment()).isEqualTo(true); + } + private ConfigurableApplicationContext initializeAndRun(Class config, String... args) { return initializeAndRun(config, Collections.emptyMap(), args); @@ -244,7 +264,6 @@ public class LocalDevToolsAutoConfigurationTests { Restarter.initialize(new String[0], false, new MockRestartInitializer(), false); SpringApplication application = new SpringApplication(config); application.setDefaultProperties(getDefaultProperties(properties)); - application.setWebEnvironment(false); ConfigurableApplicationContext context = application.run(args); return context; } @@ -254,18 +273,22 @@ public class LocalDevToolsAutoConfigurationTests { Map properties = new HashMap(); properties.put("spring.thymeleaf.check-template-location", false); properties.put("spring.devtools.livereload.port", this.liveReloadPort); + properties.put("server.port", 0); properties.putAll(specifiedProperties); return properties; } @Configuration - @Import({ LocalDevToolsAutoConfiguration.class, ThymeleafAutoConfiguration.class }) + @Import({ EmbeddedServletContainerAutoConfiguration.class, + LocalDevToolsAutoConfiguration.class, ThymeleafAutoConfiguration.class }) + @EnableConfigurationProperties(ServerProperties.class) public static class Config { } @Configuration - @Import({ LocalDevToolsAutoConfiguration.class, ThymeleafAutoConfiguration.class }) + @Import({ EmbeddedServletContainerAutoConfiguration.class, + LocalDevToolsAutoConfiguration.class, ThymeleafAutoConfiguration.class }) public static class ConfigWithMockLiveReload { @Bean @@ -276,7 +299,8 @@ public class LocalDevToolsAutoConfigurationTests { } @Configuration - @Import({ LocalDevToolsAutoConfiguration.class, ResourceProperties.class }) + @Import({ EmbeddedServletContainerAutoConfiguration.class, + LocalDevToolsAutoConfiguration.class, ResourceProperties.class }) public static class WebResourcesConfig { } diff --git a/spring-boot/src/main/java/org/springframework/boot/context/embedded/JspServlet.java b/spring-boot/src/main/java/org/springframework/boot/context/embedded/JspServlet.java index fcc51b09030..c44ed653a1b 100644 --- a/spring-boot/src/main/java/org/springframework/boot/context/embedded/JspServlet.java +++ b/spring-boot/src/main/java/org/springframework/boot/context/embedded/JspServlet.java @@ -1,5 +1,5 @@ /* - * Copyright 2012-2015 the original author or authors. + * Copyright 2012-2016 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. @@ -36,7 +36,7 @@ public class JspServlet { private String className = "org.apache.jasper.servlet.JspServlet"; /** - * Init parameters use to configure the JSP servlet. + * Init parameters used to configure the JSP servlet. */ private Map initParameters = new HashMap(); @@ -46,6 +46,10 @@ public class JspServlet { */ private boolean registered = true; + public JspServlet() { + this.initParameters.put("development", "false"); + } + public String getClassName() { return this.className; } diff --git a/spring-boot/src/test/java/org/springframework/boot/context/embedded/AbstractEmbeddedServletContainerFactoryTests.java b/spring-boot/src/test/java/org/springframework/boot/context/embedded/AbstractEmbeddedServletContainerFactoryTests.java index 71962fbb4c7..a0263ab29a1 100644 --- a/spring-boot/src/test/java/org/springframework/boot/context/embedded/AbstractEmbeddedServletContainerFactoryTests.java +++ b/spring-boot/src/test/java/org/springframework/boot/context/embedded/AbstractEmbeddedServletContainerFactoryTests.java @@ -67,8 +67,11 @@ import org.apache.http.impl.client.HttpClientBuilder; import org.apache.http.impl.client.HttpClients; import org.apache.http.protocol.HttpContext; import org.apache.http.ssl.SSLContextBuilder; +import org.apache.jasper.EmbeddedServletOptions; +import org.apache.jasper.servlet.JspServlet; import org.junit.After; import org.junit.AfterClass; +import org.junit.Assume; import org.junit.BeforeClass; import org.junit.Rule; import org.junit.Test; @@ -98,6 +101,7 @@ import org.springframework.util.StreamUtils; import org.springframework.util.concurrent.ListenableFuture; import static org.assertj.core.api.Assertions.assertThat; +import static org.hamcrest.CoreMatchers.notNullValue; import static org.junit.Assert.fail; import static org.mockito.BDDMockito.given; import static org.mockito.Matchers.anyObject; @@ -883,6 +887,29 @@ public abstract class AbstractEmbeddedServletContainerFactoryTests { assertThat(getCharset(Locale.ITALIAN)).isNull(); } + @Test + public void jspServletInitParameters() throws Exception { + Map initParameters = new HashMap(); + initParameters.put("a", "alpha"); + AbstractEmbeddedServletContainerFactory factory = getFactory(); + factory.getJspServlet().setInitParameters(initParameters); + this.container = factory.getEmbeddedServletContainer(); + Assume.assumeThat(getJspServlet(), notNullValue()); + JspServlet jspServlet = getJspServlet(); + assertThat(jspServlet.getInitParameter("a")).isEqualTo("alpha"); + } + + @Test + public void jspServletIsNotInDevelopmentModeByDefault() throws Exception { + AbstractEmbeddedServletContainerFactory factory = getFactory(); + this.container = factory.getEmbeddedServletContainer(); + Assume.assumeThat(getJspServlet(), notNullValue()); + JspServlet jspServlet = getJspServlet(); + EmbeddedServletOptions options = (EmbeddedServletOptions) ReflectionTestUtils + .getField(jspServlet, "options"); + assertThat(options.getDevelopment()).isEqualTo(false); + } + protected abstract void addConnector(int port, AbstractEmbeddedServletContainerFactory factory); @@ -1032,7 +1059,8 @@ public abstract class AbstractEmbeddedServletContainerFactoryTests { protected abstract AbstractEmbeddedServletContainerFactory getFactory(); - protected abstract Object getJspServlet(); + protected abstract org.apache.jasper.servlet.JspServlet getJspServlet() + throws Exception; protected ServletContextInitializer exampleServletRegistration() { return new ServletRegistrationBean(new ExampleServlet(), "/hello"); diff --git a/spring-boot/src/test/java/org/springframework/boot/context/embedded/jetty/JettyEmbeddedServletContainerFactoryTests.java b/spring-boot/src/test/java/org/springframework/boot/context/embedded/jetty/JettyEmbeddedServletContainerFactoryTests.java index cedeaae7846..97b8fbc0607 100644 --- a/spring-boot/src/test/java/org/springframework/boot/context/embedded/jetty/JettyEmbeddedServletContainerFactoryTests.java +++ b/spring-boot/src/test/java/org/springframework/boot/context/embedded/jetty/JettyEmbeddedServletContainerFactoryTests.java @@ -19,7 +19,6 @@ package org.springframework.boot.context.embedded.jetty; import java.io.IOException; import java.nio.charset.Charset; import java.util.Arrays; -import java.util.HashMap; import java.util.Locale; import java.util.Map; import java.util.concurrent.TimeUnit; @@ -29,6 +28,7 @@ import javax.servlet.http.HttpServlet; import javax.servlet.http.HttpServletRequest; import javax.servlet.http.HttpServletResponse; +import org.apache.jasper.servlet.JspServlet; import org.eclipse.jetty.server.Handler; import org.eclipse.jetty.server.Server; import org.eclipse.jetty.server.ServerConnector; @@ -246,16 +246,6 @@ public class JettyEmbeddedServletContainerFactoryTests testBasicSslWithKeyStore("classpath:test.jks"); } - @Test - public void jspServletInitParameters() { - JettyEmbeddedServletContainerFactory factory = getFactory(); - Map initParameters = new HashMap(); - initParameters.put("a", "alpha"); - factory.getJspServlet().setInitParameters(initParameters); - this.container = factory.getEmbeddedServletContainer(); - assertThat(getJspServlet().getInitParameters()).isEqualTo(initParameters); - } - @Test public void useForwardHeaders() throws Exception { JettyEmbeddedServletContainerFactory factory = getFactory(); @@ -316,10 +306,15 @@ public class JettyEmbeddedServletContainerFactoryTests } @Override - protected ServletHolder getJspServlet() { + protected JspServlet getJspServlet() throws Exception { WebAppContext context = (WebAppContext) ((JettyEmbeddedServletContainer) this.container) .getServer().getHandler(); - return context.getServletHandler().getServlet("jsp"); + ServletHolder holder = context.getServletHandler().getServlet("jsp"); + if (holder == null) { + return null; + } + holder.start(); + return (JspServlet) holder.getServlet(); } @Override diff --git a/spring-boot/src/test/java/org/springframework/boot/context/embedded/tomcat/TomcatEmbeddedServletContainerFactoryTests.java b/spring-boot/src/test/java/org/springframework/boot/context/embedded/tomcat/TomcatEmbeddedServletContainerFactoryTests.java index 80289374a44..88f3ea259f7 100644 --- a/spring-boot/src/test/java/org/springframework/boot/context/embedded/tomcat/TomcatEmbeddedServletContainerFactoryTests.java +++ b/spring-boot/src/test/java/org/springframework/boot/context/embedded/tomcat/TomcatEmbeddedServletContainerFactoryTests.java @@ -20,13 +20,13 @@ import java.io.File; import java.io.IOException; import java.nio.charset.Charset; import java.util.Arrays; -import java.util.HashMap; import java.util.Locale; import java.util.Map; import java.util.concurrent.TimeUnit; import javax.naming.InitialContext; import javax.naming.NamingException; +import javax.servlet.ServletException; import org.apache.catalina.Container; import org.apache.catalina.Context; @@ -36,11 +36,12 @@ import org.apache.catalina.LifecycleState; import org.apache.catalina.Service; import org.apache.catalina.SessionIdGenerator; import org.apache.catalina.Valve; -import org.apache.catalina.Wrapper; import org.apache.catalina.connector.Connector; +import org.apache.catalina.core.StandardWrapper; import org.apache.catalina.startup.Tomcat; import org.apache.catalina.util.CharsetMapper; import org.apache.catalina.valves.RemoteIpValve; +import org.apache.jasper.servlet.JspServlet; import org.apache.tomcat.util.net.SSLHostConfig; import org.junit.After; import org.junit.Rule; @@ -359,17 +360,6 @@ public class TomcatEmbeddedServletContainerFactoryTests .addAdditionalTomcatConnectors(connector); } - @Test - public void jspServletInitParameters() { - Map initParameters = new HashMap(); - initParameters.put("a", "alpha"); - TomcatEmbeddedServletContainerFactory factory = getFactory(); - factory.getJspServlet().setInitParameters(initParameters); - this.container = factory.getEmbeddedServletContainer(); - Wrapper jspServlet = getJspServlet(); - assertThat(jspServlet.findInitParameter("a")).isEqualTo("alpha"); - } - @Test public void useForwardHeaders() throws Exception { TomcatEmbeddedServletContainerFactory factory = getFactory(); @@ -462,10 +452,15 @@ public class TomcatEmbeddedServletContainerFactoryTests } @Override - protected Wrapper getJspServlet() { + protected JspServlet getJspServlet() throws ServletException { Container context = ((TomcatEmbeddedServletContainer) this.container).getTomcat() .getHost().findChildren()[0]; - return (Wrapper) context.findChild("jsp"); + StandardWrapper standardWrapper = (StandardWrapper) context.findChild("jsp"); + if (standardWrapper == null) { + return null; + } + standardWrapper.load(); + return (JspServlet) standardWrapper.getServlet(); } @SuppressWarnings("unchecked") diff --git a/spring-boot/src/test/java/org/springframework/boot/context/embedded/undertow/UndertowEmbeddedServletContainerFactoryTests.java b/spring-boot/src/test/java/org/springframework/boot/context/embedded/undertow/UndertowEmbeddedServletContainerFactoryTests.java index b3ad0fe8b26..ab77946d26e 100644 --- a/spring-boot/src/test/java/org/springframework/boot/context/embedded/undertow/UndertowEmbeddedServletContainerFactoryTests.java +++ b/spring-boot/src/test/java/org/springframework/boot/context/embedded/undertow/UndertowEmbeddedServletContainerFactoryTests.java @@ -34,6 +34,7 @@ import io.undertow.Undertow.Builder; import io.undertow.servlet.api.DeploymentInfo; import io.undertow.servlet.api.DeploymentManager; import io.undertow.servlet.api.ServletContainer; +import org.apache.jasper.servlet.JspServlet; import org.junit.Test; import org.mockito.InOrder; @@ -250,7 +251,7 @@ public class UndertowEmbeddedServletContainerFactoryTests } @Override - protected Object getJspServlet() { + protected JspServlet getJspServlet() { return null; // Undertow does not support JSPs }