From 145d8d2673cb9d09691f6db6d3ea17ab4c293c64 Mon Sep 17 00:00:00 2001 From: Andy Wilkinson Date: Wed, 14 Feb 2018 16:25:32 +0000 Subject: [PATCH] Defer removal of Connectors until after ServletContext initialization Previously, we removed the Connectors from Tomcat's Service before the Context was started. The removal of the Connectors is required as it prevents Tomcat from accepting requests before we're ready to handle them. Part of starting the Context is creating and initializing the ServletContext. ServerProperties uses a ServletContextInitializer to set the session tracking modes and Tomcat rejects the SSL tracking mode if there is no SSL-enabled connector available. With the previous arrangement this led to a failure as the Connectors had been removed so the SSL-enabled connector could not be found. This commit updates the embedded Tomcat container to defer the removal of the Connectors until after the context has been started but still at a point that is before the Connectors themselves would have been started. Closes gh-12058 --- .../web/ServerPropertiesTests.java | 125 +++++++++++++----- .../src/test/resources/test.jks | Bin 0 -> 4464 bytes .../TomcatEmbeddedServletContainer.java | 24 +++- 3 files changed, 110 insertions(+), 39 deletions(-) create mode 100644 spring-boot-autoconfigure/src/test/resources/test.jks 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 8f4ffcf3e01..c15666133b5 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 @@ -23,6 +23,7 @@ import java.util.Collections; import java.util.EnumSet; import java.util.HashMap; import java.util.Map; +import java.util.concurrent.atomic.AtomicReference; import javax.servlet.ServletContext; import javax.servlet.ServletException; @@ -43,6 +44,7 @@ import org.mockito.MockitoAnnotations; import org.springframework.beans.MutablePropertyValues; import org.springframework.boot.bind.RelaxedDataBinder; +import org.springframework.boot.context.embedded.AbstractEmbeddedServletContainerFactory; import org.springframework.boot.context.embedded.ConfigurableEmbeddedServletContainer; import org.springframework.boot.context.embedded.EmbeddedServletContainer; import org.springframework.boot.context.embedded.jetty.JettyEmbeddedServletContainerFactory; @@ -53,9 +55,7 @@ import org.springframework.boot.web.servlet.ServletContextInitializer; import org.springframework.mock.env.MockEnvironment; import static org.assertj.core.api.Assertions.assertThat; -import static org.mockito.BDDMockito.given; import static org.mockito.Matchers.anyBoolean; -import static org.mockito.Mockito.atLeastOnce; import static org.mockito.Mockito.mock; import static org.mockito.Mockito.never; import static org.mockito.Mockito.spy; @@ -314,7 +314,22 @@ public class ServerPropertiesTests { } @Test - public void customizeSessionProperties() throws Exception { + public void customizeSessionPropertiesWithJetty() throws Exception { + customizeSessionProperties(new TomcatEmbeddedServletContainerFactory(0)); + } + + @Test + public void customizeSessionPropertiesWithTomcat() throws Exception { + customizeSessionProperties(new TomcatEmbeddedServletContainerFactory(0)); + } + + @Test + public void customizeSessionPropertiesWithUndertow() throws Exception { + customizeSessionProperties(new UndertowEmbeddedServletContainerFactory(0)); + } + + private void customizeSessionProperties( + AbstractEmbeddedServletContainerFactory factory) { Map map = new HashMap(); map.put("server.session.timeout", "123"); map.put("server.session.tracking-modes", "cookie,url"); @@ -326,38 +341,80 @@ public class ServerPropertiesTests { map.put("server.session.cookie.secure", "true"); map.put("server.session.cookie.max-age", "60"); bindProperties(map); - ConfigurableEmbeddedServletContainer factory = mock( - ConfigurableEmbeddedServletContainer.class); - ServletContext servletContext = mock(ServletContext.class); - SessionCookieConfig sessionCookieConfig = mock(SessionCookieConfig.class); - given(servletContext.getSessionCookieConfig()).willReturn(sessionCookieConfig); this.properties.customize(factory); - triggerInitializers(factory, servletContext); - verify(factory).setSessionTimeout(123); - verify(servletContext).setSessionTrackingModes( - EnumSet.of(SessionTrackingMode.COOKIE, SessionTrackingMode.URL)); - verify(sessionCookieConfig).setName("testname"); - verify(sessionCookieConfig).setDomain("testdomain"); - verify(sessionCookieConfig).setPath("/testpath"); - verify(sessionCookieConfig).setComment("testcomment"); - verify(sessionCookieConfig).setHttpOnly(true); - verify(sessionCookieConfig).setSecure(true); - verify(sessionCookieConfig).setMaxAge(60); - } - - private void triggerInitializers(ConfigurableEmbeddedServletContainer container, - ServletContext servletContext) throws ServletException { - verify(container, atLeastOnce()) - .addInitializers(this.initializersCaptor.capture()); - for (Object initializers : this.initializersCaptor.getAllValues()) { - if (initializers instanceof ServletContextInitializer) { - ((ServletContextInitializer) initializers).onStartup(servletContext); - } - else { - for (ServletContextInitializer initializer : (ServletContextInitializer[]) initializers) { - initializer.onStartup(servletContext); - } - } + final AtomicReference servletContextReference = new AtomicReference(); + EmbeddedServletContainer container = factory + .getEmbeddedServletContainer(new ServletContextInitializer() { + + @Override + public void onStartup(ServletContext servletContext) + throws ServletException { + servletContextReference.set(servletContext); + } + + }); + try { + container.start(); + SessionCookieConfig sessionCookieConfig = servletContextReference.get() + .getSessionCookieConfig(); + assertThat(factory.getSessionTimeout()).isEqualTo(123); + assertThat(sessionCookieConfig.getName()).isEqualTo("testname"); + assertThat(sessionCookieConfig.getDomain()).isEqualTo("testdomain"); + assertThat(sessionCookieConfig.getPath()).isEqualTo("/testpath"); + assertThat(sessionCookieConfig.getComment()).isEqualTo("testcomment"); + assertThat(sessionCookieConfig.isHttpOnly()).isTrue(); + assertThat(sessionCookieConfig.isSecure()).isTrue(); + assertThat(sessionCookieConfig.getMaxAge()).isEqualTo(60); + assertThat(servletContextReference.get().getEffectiveSessionTrackingModes()) + .isEqualTo(EnumSet.of(SessionTrackingMode.COOKIE, + SessionTrackingMode.URL)); + } + finally { + container.stop(); + } + } + + @Test + public void sslSessionTrackingWithJetty() throws Exception { + sslSessionTracking(new JettyEmbeddedServletContainerFactory(0)); + } + + @Test + public void sslSessionTrackingWithTomcat() throws Exception { + sslSessionTracking(new TomcatEmbeddedServletContainerFactory(0)); + } + + @Test + public void sslSessionTrackingWithUndertow() throws Exception { + sslSessionTracking(new UndertowEmbeddedServletContainerFactory(0)); + } + + private void sslSessionTracking(AbstractEmbeddedServletContainerFactory factory) { + Map map = new HashMap(); + map.put("server.ssl.enabled", "true"); + map.put("server.ssl.key-store", "src/test/resources/test.jks"); + map.put("server.ssl.key-password", "password"); + map.put("server.session.tracking-modes", "ssl"); + bindProperties(map); + this.properties.customize(factory); + final AtomicReference servletContextReference = new AtomicReference(); + EmbeddedServletContainer container = factory + .getEmbeddedServletContainer(new ServletContextInitializer() { + + @Override + public void onStartup(ServletContext servletContext) + throws ServletException { + servletContextReference.set(servletContext); + } + + }); + try { + container.start(); + assertThat(servletContextReference.get().getEffectiveSessionTrackingModes()) + .isEqualTo(EnumSet.of(SessionTrackingMode.SSL)); + } + finally { + container.stop(); } } diff --git a/spring-boot-autoconfigure/src/test/resources/test.jks b/spring-boot-autoconfigure/src/test/resources/test.jks new file mode 100644 index 0000000000000000000000000000000000000000..1bce90bba66f697b2051dc839377de1f016a563d GIT binary patch literal 4464 zcmeH}XH-*rm&VgbCh{iXciaQlwArJNKQr?|9dmPc!r7e%R}r{d_oUujlOl@BbVu9xMU?01%_V zi~<6nH%adHwl^8C7q1-;u>b&=Ko|<-0K*Ptm4pHzKnc)MAdm?Fp+M%C-upfh)ig4F zk2bYO)$;1B`%75OLb$$2HP`<%SXBaZKo!5}*(whI7G^b7fiQX0ZEFcAyV55V&{n7* z)N#Kd?%G5-AH+ddk0nWL9{E z2h5ylj3uqD%lU(kF5nR19Ju@NL!fV8@PqpkPde|)WrP)Ja;z+^eqirct7L>@L|9@&m#H7JCS>bmZ) zJOy^2{M7xBuZopUjnkz}^bn6aRu=E3Hg;lO&;hgPv;Y-Uh@qoH>(u3pSV7t=VeL1W z`cejVD?6_t7gLzA!rdrLK~CB={K$_JEr!(P^g3*qPQEn1ajnDEU=x0XV5 zH=xA`0lB8y`!S+VqVgjboiW^f#D^S;U0KgR+G=R)z9Gw1sLCZ>6mag*D~4f^de@q7vO5#C+8iH3Nkg$+qG%6Z7+yQQrlc_vDYh*#+x2Z zx6+=5Cw4wu(z*z}teq5-<13ndAzmOyE?q^lI$fZMd1o!^DPnCva&X|J;y7J5>%iSu zAP!a7k-NG*qin9C5%xA;-GNE!8lEOESJLQpOvo$Vvl4R0x2l>-c=@r;(d^TDWM@F2 zJ5c9|T8(<3{h5p89etKfRGob*;kcLO4OAYVaABA6{Po7w$hDGIsz(q_GwZ4)x5Pt> z=+c2TF&cT>XSe6civdM&@y^(&Xo_pGM^xsvS1ghw^ZQ%>IyEnU-Sr z5G9wD4av5A;v`q32eD(hXpVUPXyH7nt3_bPl_>C9ue=MC3&3kQj@w5@UO7FM7I45e z{rH@<6F+mpOy3LtCs{E=z?@yfl*@%?y&-2D7YaECcXhIvZGNC|-0Zo7TPGCzW>{mZ z<%|56jVsim6`}O|mUA*=H_JNInMvx4sxE=_rJ)V|#4*aV7=pN4;pu4c{ZC38#F`V0 z(10rw9|pGZ!p}P1?hG_3o9S-i{oW?rL=-r_a=t`>TA@b`d3^$Ss~ zG6<>vAS08xj66n(!O36@1?-QZfb5@uK|l})Vjh!C!@!{|5)p)mEjSR!#st84U^yAc z1e-vhTrd+4R}Yf!e=5wsD6n4?)?XC1|5A=&IDh{#n2W`YWN+(sjpTh3gF?cwau`LN z92Sek%gbB--eNI0+#l`#`8*V$;GZpF+!$aAkOP3B09ip4AQ13y6CG?&Ipx&a9JqW7 zcgH22yOMKr`9xCb#gy)oc?I2=Z&+a0P|^n^bxbk*?CrZFs)@4-Q5na$)Pvar1g)MM zqsw^FSCM;@A-mx~GE{G!U|yha<5=aM<9FN@8U^1ce%D^n*A*liqKH>Umg;7i62NSz zAJ}L&QK_m)waA)eZL#yKQE0CF?2#qBD=Mn39lJ*YG-B_Z+p<-4Qs>HjkZ7%I^VN%rMZ=b%`YaP4A&nlSsfg-kn zzRk9LKE*GCJ_0Cs`w`%zt`3LO9dwJxOrz3TY&{}+&IKSa5CF_Mjp4_fV*JZf><}IZ zf?A7@om+j^-z$Xo_xBBe8fvtJ|Jg*wUNKAr0^;IX&U{$f{2bnLLCANOps-Z6oSm@& zQ!`XibtykDi@8BpJ-*dK`jSAzXBSTCya-m>JWX}vb<)IjI^%X`3k=m4W?lC}990B^ zkzP&Hdj?k>la9=rJc{1yF&##-f!cM3*k4@!Ij!*SYM@H5!WnGnoK(!-*Hm6)kj8Cy zgOLlh$Ubu)Vu@5yyw`~aOME%`vqbp&#HY?t4nY$^G`~6{TxSJurci;UHIsJ|xxF<9 zVDyGn@w4Gpocv6SRJ4Fycbg|b2pR(U?L6}9JA909tcAl}ttJyG+k?CN7v5bLzD$~I z8%ljM^e5v}{v~9kNDIf*NFhN%+;O)id5AG26+)Eagan2Z0?I*2MM+n~qXL z=!MY=?+UzOk-u=F0{I}JrdfQLRx^ikLrKb))OyojuX}P zeO$o%GwqnDLxdtJT4ib$QKOK|rJcN%|AvqVEP7r(k*icnCE#g~OA*}}bxJW843x6w zX5>@okNE)=2RJ$Ek6U(#K6J5ge`bD=%NI@fc>VOgX*6?8k+MPZ)6Z&_)EcYddV%C~ znk7EYru$;;waVMo{6Afcg2Q$B3qNjIDz%-BQGLv!iEtd2f43o_Wn6T_z7FquL2=wh zW5l6V>umC}=6?J36jdtCUR)Aj@EGx~kt1@bidL4D5-Bqv^K63#yP+;(`^ZT3 zTushWK^f(qnDpSe=;1KaMUBbPyQ8#qks1EmGbw>s-aAQkHqp0+G;gHTaYoL);)&Rn=L17GVcH$vm1~zYF|9~6x zPjLIE#{3r=>@PIdztGtJLi+`viWpfe28)+f`VW9AViYj=KidDt$tXZ7W2>e9X%mpq z@Po)?z<%%*@+qHV;+MyDGGrE}zil3+URGJw1T>)3Lf#pPT-UNbFKD!Dp&DbpQ$ydV z#B8>wo45^;l|xR+k=C4SsIF;-&!1&weTVpS3f3FYUJe~loA`bzn|*7_%jM>P?1pak z(NPT`d1Ee1o_O4*&yPY$+aZbNH9;_MW)A__4B`f)*cd+GMzeLiO{kz`&xf(Cn4jGE zeYCB~C*gzXs$=a{1^=c{ZdT!!SVY@7J4e|z(_>SGQJjq|%wjA1EevKnVVvZUQX-Eh z`n#SO4;QtI1sC0tJE_8RC(1Y=Mrl5#h6U?@ATHbDT*X|BbNV((LYIUAb z+`YWe@kz_CS82i`WSGiKPL*; z?653ALWG<8a#xpD&ho-{58dd?T{VXquZ$o&`EzZsoZ}*Q*@DNV*Fw!|+qyDFy7=?$ zx=?Lp|BdfI_2P;X3H_gz@x<(2DL{ItbI9?3S${AFfRP3vEbbJZA86MFPj%7XU|C=R zFIm#5472YJSR$?0MWFH4-Y>3=po`dfw(S__3j|