From 217b237b37fddee4929d76de59643f6cb3d16b9c Mon Sep 17 00:00:00 2001 From: hengyunabc Date: Thu, 4 May 2017 20:33:58 +0800 Subject: [PATCH 1/2] Fail startup when Tomcat's context fails to start See gh-9095 --- .../TomcatEmbeddedServletContainer.java | 8 +++++ ...tEmbeddedServletContainerFactoryTests.java | 31 +++++++++++++++++++ 2 files changed, 39 insertions(+) diff --git a/spring-boot/src/main/java/org/springframework/boot/context/embedded/tomcat/TomcatEmbeddedServletContainer.java b/spring-boot/src/main/java/org/springframework/boot/context/embedded/tomcat/TomcatEmbeddedServletContainer.java index b9a59b4fc20..07960d126a1 100644 --- a/spring-boot/src/main/java/org/springframework/boot/context/embedded/tomcat/TomcatEmbeddedServletContainer.java +++ b/spring-boot/src/main/java/org/springframework/boot/context/embedded/tomcat/TomcatEmbeddedServletContainer.java @@ -192,6 +192,7 @@ public class TomcatEmbeddedServletContainer implements EmbeddedServletContainer if (connector != null && this.autoStart) { startConnector(connector); } + checkThatContextHaveStarted(); checkThatConnectorsHaveStarted(); this.started = true; TomcatEmbeddedServletContainer.logger @@ -221,6 +222,13 @@ public class TomcatEmbeddedServletContainer implements EmbeddedServletContainer } } + private void checkThatContextHaveStarted() { + LifecycleState state = this.findContext().getState(); + if (!LifecycleState.STARTED.equals(state)) { + throw new EmbeddedServletContainerException("Context state expect STARTED, but " + state, null); + } + } + private void stopSilently() { try { stopTomcat(); 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 ed5ea62394b..f699c3872b5 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 @@ -26,7 +26,12 @@ import java.util.concurrent.TimeUnit; import javax.naming.InitialContext; import javax.naming.NamingException; +import javax.servlet.Filter; +import javax.servlet.FilterChain; +import javax.servlet.FilterConfig; import javax.servlet.ServletException; +import javax.servlet.ServletRequest; +import javax.servlet.ServletResponse; import org.apache.catalina.Container; import org.apache.catalina.Context; @@ -53,6 +58,7 @@ import org.springframework.boot.context.embedded.AbstractEmbeddedServletContaine import org.springframework.boot.context.embedded.EmbeddedServletContainerException; import org.springframework.boot.context.embedded.Ssl; import org.springframework.boot.testutil.InternalOutputCapture; +import org.springframework.boot.web.servlet.FilterRegistrationBean; import org.springframework.test.util.ReflectionTestUtils; import org.springframework.util.SocketUtils; @@ -509,4 +515,29 @@ public class TomcatEmbeddedServletContainerFactoryTests assertThat(((ConnectorStartFailedException) ex).getPort()).isEqualTo(blockedPort); } + @Test(expected = EmbeddedServletContainerException.class) + public void startServletExceptionFilter() throws Exception { + AbstractEmbeddedServletContainerFactory factory = getFactory(); + + FilterRegistrationBean filterRegistrationBean = new FilterRegistrationBean(); + filterRegistrationBean.setFilter(new Filter() { + @Override + public void init(FilterConfig filterConfig) throws ServletException { + throw new ServletException(); + } + + @Override + public void doFilter(ServletRequest request, ServletResponse response, + FilterChain chain) throws IOException, ServletException { + } + + @Override + public void destroy() { + } + }); + filterRegistrationBean.setUrlPatterns(Arrays.asList("/test")); + + this.container = factory.getEmbeddedServletContainer(filterRegistrationBean); + this.container.start(); + } } From 8c466442312b9cdc05e859c44e0b565d916bc5c1 Mon Sep 17 00:00:00 2001 From: hengyunabc Date: Thu, 4 May 2017 20:33:58 +0800 Subject: [PATCH 2/2] Polish "Fail startup when Tomcat's context fails to start" Closes gh-9095 --- .../TomcatEmbeddedServletContainer.java | 11 +--- ...yEmbeddedServletContainerFactoryTests.java | 40 +++++++++++++ ...tEmbeddedServletContainerFactoryTests.java | 60 +++++++++++-------- 3 files changed, 77 insertions(+), 34 deletions(-) diff --git a/spring-boot/src/main/java/org/springframework/boot/context/embedded/tomcat/TomcatEmbeddedServletContainer.java b/spring-boot/src/main/java/org/springframework/boot/context/embedded/tomcat/TomcatEmbeddedServletContainer.java index 07960d126a1..aaa4b8991f8 100644 --- a/spring-boot/src/main/java/org/springframework/boot/context/embedded/tomcat/TomcatEmbeddedServletContainer.java +++ b/spring-boot/src/main/java/org/springframework/boot/context/embedded/tomcat/TomcatEmbeddedServletContainer.java @@ -163,6 +163,9 @@ public class TomcatEmbeddedServletContainer implements EmbeddedServletContainer throw exception; } } + if (!LifecycleState.STARTED.equals(container.getState())) { + throw new IllegalStateException(container + " failed to start"); + } } } @@ -192,7 +195,6 @@ public class TomcatEmbeddedServletContainer implements EmbeddedServletContainer if (connector != null && this.autoStart) { startConnector(connector); } - checkThatContextHaveStarted(); checkThatConnectorsHaveStarted(); this.started = true; TomcatEmbeddedServletContainer.logger @@ -222,13 +224,6 @@ public class TomcatEmbeddedServletContainer implements EmbeddedServletContainer } } - private void checkThatContextHaveStarted() { - LifecycleState state = this.findContext().getState(); - if (!LifecycleState.STARTED.equals(state)) { - throw new EmbeddedServletContainerException("Context state expect STARTED, but " + state, null); - } - } - private void stopSilently() { try { stopTomcat(); 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 607c2ab68ae..eb10c83b9a3 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 @@ -23,7 +23,13 @@ import java.util.Locale; import java.util.Map; import java.util.concurrent.TimeUnit; +import javax.servlet.Filter; +import javax.servlet.FilterChain; +import javax.servlet.FilterConfig; +import javax.servlet.ServletContext; import javax.servlet.ServletException; +import javax.servlet.ServletRequest; +import javax.servlet.ServletResponse; import javax.servlet.http.HttpServlet; import javax.servlet.http.HttpServletRequest; import javax.servlet.http.HttpServletResponse; @@ -45,8 +51,10 @@ import org.mockito.InOrder; import org.springframework.boot.context.embedded.AbstractEmbeddedServletContainerFactory; import org.springframework.boot.context.embedded.AbstractEmbeddedServletContainerFactoryTests; import org.springframework.boot.context.embedded.Compression; +import org.springframework.boot.context.embedded.EmbeddedServletContainerException; import org.springframework.boot.context.embedded.PortInUseException; import org.springframework.boot.context.embedded.Ssl; +import org.springframework.boot.web.servlet.ServletContextInitializer; import org.springframework.boot.web.servlet.ServletRegistrationBean; import org.springframework.http.HttpHeaders; @@ -282,6 +290,38 @@ public class JettyEmbeddedServletContainerFactoryTests .getThreadPool()).isSameAs(threadPool); } + @Test + public void faultyFilterCausesStartFailure() throws Exception { + AbstractEmbeddedServletContainerFactory factory = getFactory(); + factory.addInitializers(new ServletContextInitializer() { + + @Override + public void onStartup(ServletContext servletContext) throws ServletException { + servletContext.addFilter("faulty", new Filter() { + + @Override + public void init(FilterConfig filterConfig) throws ServletException { + throw new ServletException("Faulty filter"); + } + + @Override + public void doFilter(ServletRequest request, ServletResponse response, + FilterChain chain) throws IOException, ServletException { + chain.doFilter(request, response); + } + + @Override + public void destroy() { + } + + }); + } + + }); + this.thrown.expect(EmbeddedServletContainerException.class); + factory.getEmbeddedServletContainer().start(); + } + @Override @SuppressWarnings("serial") // Workaround for Jetty issue - https://bugs.eclipse.org/bugs/show_bug.cgi?id=470646 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 f699c3872b5..a14e4ff962e 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 @@ -29,6 +29,7 @@ import javax.naming.NamingException; import javax.servlet.Filter; import javax.servlet.FilterChain; import javax.servlet.FilterConfig; +import javax.servlet.ServletContext; import javax.servlet.ServletException; import javax.servlet.ServletRequest; import javax.servlet.ServletResponse; @@ -58,7 +59,7 @@ import org.springframework.boot.context.embedded.AbstractEmbeddedServletContaine import org.springframework.boot.context.embedded.EmbeddedServletContainerException; import org.springframework.boot.context.embedded.Ssl; import org.springframework.boot.testutil.InternalOutputCapture; -import org.springframework.boot.web.servlet.FilterRegistrationBean; +import org.springframework.boot.web.servlet.ServletContextInitializer; import org.springframework.test.util.ReflectionTestUtils; import org.springframework.util.SocketUtils; @@ -466,6 +467,38 @@ public class TomcatEmbeddedServletContainerFactoryTests assertThat(sessionIdGenerator.getJvmRoute()).isEqualTo("test"); } + @Test + public void faultyFilterCausesStartFailure() throws Exception { + AbstractEmbeddedServletContainerFactory factory = getFactory(); + factory.addInitializers(new ServletContextInitializer() { + + @Override + public void onStartup(ServletContext servletContext) throws ServletException { + servletContext.addFilter("faulty", new Filter() { + + @Override + public void init(FilterConfig filterConfig) throws ServletException { + throw new ServletException("Faulty filter"); + } + + @Override + public void doFilter(ServletRequest request, ServletResponse response, + FilterChain chain) throws IOException, ServletException { + chain.doFilter(request, response); + } + + @Override + public void destroy() { + } + + }); + } + + }); + this.thrown.expect(EmbeddedServletContainerException.class); + factory.getEmbeddedServletContainer().start(); + } + @Override protected JspServlet getJspServlet() throws ServletException { Container context = ((TomcatEmbeddedServletContainer) this.container).getTomcat() @@ -515,29 +548,4 @@ public class TomcatEmbeddedServletContainerFactoryTests assertThat(((ConnectorStartFailedException) ex).getPort()).isEqualTo(blockedPort); } - @Test(expected = EmbeddedServletContainerException.class) - public void startServletExceptionFilter() throws Exception { - AbstractEmbeddedServletContainerFactory factory = getFactory(); - - FilterRegistrationBean filterRegistrationBean = new FilterRegistrationBean(); - filterRegistrationBean.setFilter(new Filter() { - @Override - public void init(FilterConfig filterConfig) throws ServletException { - throw new ServletException(); - } - - @Override - public void doFilter(ServletRequest request, ServletResponse response, - FilterChain chain) throws IOException, ServletException { - } - - @Override - public void destroy() { - } - }); - filterRegistrationBean.setUrlPatterns(Arrays.asList("/test")); - - this.container = factory.getEmbeddedServletContainer(filterRegistrationBean); - this.container.start(); - } }