From 71c0e474468dd01764a0ca43f282d16ea751dfe9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bj=C3=B8rn=20Christian=20Seime?= Date: Mon, 10 Dec 2018 13:23:15 +0100 Subject: [PATCH 01/11] Change access of LowResourceCheck to public MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This interface is currently package-private while it's in the signature of several public methods (e.g. addLowResourceCheck, getLowResourceChecks) Signed-off-by: Bjørn Christian Seime --- .../main/java/org/eclipse/jetty/server/LowResourceMonitor.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/jetty-server/src/main/java/org/eclipse/jetty/server/LowResourceMonitor.java b/jetty-server/src/main/java/org/eclipse/jetty/server/LowResourceMonitor.java index 742958ddba7..896e1bdc3dd 100644 --- a/jetty-server/src/main/java/org/eclipse/jetty/server/LowResourceMonitor.java +++ b/jetty-server/src/main/java/org/eclipse/jetty/server/LowResourceMonitor.java @@ -442,7 +442,7 @@ public class LowResourceMonitor extends ContainerLifeCycle { } - interface LowResourceCheck + public interface LowResourceCheck { boolean isLowOnResources(); From 6af567a4b99ebd48df43715f55b2a046269eac86 Mon Sep 17 00:00:00 2001 From: Jan Bartel Date: Tue, 11 Dec 2018 06:33:44 +0100 Subject: [PATCH 02/11] Handle ServletContainerInitializer from WEB-INF/classes #3146 (#3158) * Handle the case where a ServletContainerInitializer comes from WEB-INF/classes Signed-off-by: Jan Bartel --- .../annotations/AnnotationConfiguration.java | 109 ++++++++-- .../test/jar/test-sci-for-container-path.jar | Bin 0 -> 3129 bytes .../src/test/jar/test-sci-with-ordering.jar | Bin 0 -> 3120 bytes .../TestAnnotationConfiguration.java | 198 +++++++++++++----- ...bInfClassServletContainerInitializer.java} | 4 +- .../javax.servlet.ServletContainerInitializer | 2 +- 6 files changed, 231 insertions(+), 82 deletions(-) create mode 100644 jetty-annotations/src/test/jar/test-sci-for-container-path.jar create mode 100644 jetty-annotations/src/test/jar/test-sci-with-ordering.jar rename jetty-annotations/src/test/java/org/eclipse/jetty/annotations/{ServerServletContainerInitializer.java => WebInfClassServletContainerInitializer.java} (90%) diff --git a/jetty-annotations/src/main/java/org/eclipse/jetty/annotations/AnnotationConfiguration.java b/jetty-annotations/src/main/java/org/eclipse/jetty/annotations/AnnotationConfiguration.java index 9d1cc3661bc..e9a39abd080 100644 --- a/jetty-annotations/src/main/java/org/eclipse/jetty/annotations/AnnotationConfiguration.java +++ b/jetty-annotations/src/main/java/org/eclipse/jetty/annotations/AnnotationConfiguration.java @@ -667,6 +667,21 @@ public class AnnotationConfiguration extends AbstractConfiguration if (context == null) throw new IllegalArgumentException("WebAppContext null"); + //if we don't know where its from it can't be excluded + if (sciResource == null) + { + if (LOG.isDebugEnabled()) + LOG.debug("!Excluded {} null resource", sci); + return false; + } + + //A ServletContainerInitialier that came from WEB-INF/classes or equivalent cannot be excluded by an ordering + if (isFromWebInfClasses(context, sciResource)) + { + if (LOG.isDebugEnabled()) + LOG.debug("!Excluded {} from web-inf/classes", sci); + return false; + } //A ServletContainerInitializer that came from the container's classpath cannot be excluded by an ordering //of WEB-INF/lib jars @@ -695,14 +710,7 @@ public class AnnotationConfiguration extends AbstractConfiguration return true; } - if (sciResource == null) - { - //not from a jar therefore not from WEB-INF so not excludable - if (LOG.isDebugEnabled()) - LOG.debug("!Excluded {} not from jar", sci); - return false; - } - + //Check if it is excluded by an ordering URI loadingJarURI = sciResource.getURI(); boolean found = false; Iterator itor = orderedJars.iterator(); @@ -748,7 +756,46 @@ public class AnnotationConfiguration extends AbstractConfiguration { if (sci == null) return false; - return sci.getClass().getClassLoader()==context.getClassLoader().getParent(); + + ClassLoader sciLoader = sci.getClass().getClassLoader(); + + //if loaded by bootstrap loader, then its the container classpath + if ( sciLoader == null) + return true; + + //if there is no context classloader, then its the container classpath + if (context.getClassLoader() == null) + return true; + + ClassLoader loader = sciLoader; + while (loader != null) + { + if (loader == context.getClassLoader()) + return false; //the webapp classloader is in the ancestry of the classloader for the sci + else + loader = loader.getParent(); + } + + return true; + } + + /** + * Test if the ServletContainerInitializer is from WEB-INF/classes + * + * @param context the webapp to test + * @param sci a Resource representing the SCI + * @return true if the sci Resource is inside a WEB-INF/classes directory, false otherwise + */ + public boolean isFromWebInfClasses (WebAppContext context, Resource sci) + { + for (Resource dir : context.getMetaData().getWebInfClassesDirs()) + { + if (dir.equals(sci)) + { + return true; + } + } + return false; } /** @@ -834,27 +881,47 @@ public class AnnotationConfiguration extends AbstractConfiguration //No jetty-specific ordering specified, or just the wildcard value "*" specified. //Fallback to ordering the ServletContainerInitializers according to: //container classpath first, WEB-INF/classes then WEB-INF/lib (obeying any web.xml jar ordering) - - //no web.xml ordering defined, add SCIs in any order + + //First add in all SCIs that can't be excluded + int lastContainerSCI = -1; + for (Map.Entry entry:sciResourceMap.entrySet()) + { + if (entry.getKey().getClass().getClassLoader()==context.getClassLoader().getParent()) + { + nonExcludedInitializers.add(++lastContainerSCI, entry.getKey()); //add all container SCIs before any webapp SCIs + } + else if (entry.getValue() == null) //can't work out provenance of SCI, so can't be ordered/excluded + { + nonExcludedInitializers.add(entry.getKey()); //add at end of list + } + else + { + for (Resource dir : context.getMetaData().getWebInfClassesDirs()) + { + if (dir.equals(entry.getValue()))//from WEB-INF/classes so can't be ordered/excluded + { + nonExcludedInitializers.add(entry.getKey()); + } + } + } + } + + //throw out the ones we've already accounted for + for (ServletContainerInitializer s:nonExcludedInitializers) + sciResourceMap.remove(s); + if (context.getMetaData().getOrdering() == null) { if (LOG.isDebugEnabled()) LOG.debug("No web.xml ordering, ServletContainerInitializers in random order"); + //add the rest of the scis nonExcludedInitializers.addAll(sciResourceMap.keySet()); } else { if (LOG.isDebugEnabled()) LOG.debug("Ordering ServletContainerInitializers with ordering {}",context.getMetaData().getOrdering()); - for (Map.Entry entry:sciResourceMap.entrySet()) - { - //add in SCIs from the container classpath - if (entry.getKey().getClass().getClassLoader()==context.getClassLoader().getParent()) - nonExcludedInitializers.add(entry.getKey()); - else if (entry.getValue() == null) //add in SCIs not in a jar, as they must be from WEB-INF/classes and can't be ordered - nonExcludedInitializers.add(entry.getKey()); - } - + //add SCIs according to the ordering of its containing jar for (Resource webInfJar:context.getMetaData().getOrderedWebInfJars()) { @@ -874,7 +941,7 @@ public class AnnotationConfiguration extends AbstractConfiguration ListIterator it = nonExcludedInitializers.listIterator(); while (it.hasNext()) { - ServletContainerInitializer sci = it.next(); + ServletContainerInitializer sci = it.next(); if (!isFromContainerClassPath(context, sci)) { if (LOG.isDebugEnabled()) diff --git a/jetty-annotations/src/test/jar/test-sci-for-container-path.jar b/jetty-annotations/src/test/jar/test-sci-for-container-path.jar new file mode 100644 index 0000000000000000000000000000000000000000..d6fa63a78d67a2e0f627ac751f5d37a201ebd30d GIT binary patch literal 3129 zcmWIWW@h1H0D)VTwZ32ml;8x?zOEsTx}JV+`T~t1dKY=Ub+yi&Ilno`;EM5sr$stm=T7K&>pF3;KUoqsMNIpE zO2y7O-9f_MLBS<+wbqKra0^W~4ic>qJ{|N~r01*8S0A9fmvOPzflZN}m7W51UF*WV z%r0;_-@W1zrx&Nk0CO6q4_JUsOU}=QJ04R7$d4cuiOIRCc$5ht`>{B+s4O!%wHU6R z0~~Z=I}TWOF)}b%Gczz4AR8MDRG(S|B6Ctpob&Ta5;OBsi#+o(OEMF4GOJRH^pbND zi;H`MPWv5l5V2M6XWAG4!!0&rg2d}XvMsz9kF;d)wEYcGw^*_z#me+W{6?d~hwMKB zk6E)iKI(RTUwm$6@$&C)|2`ExAX#SIk$CygwANg1Yo3#nzAVu*KJv-Ew5{Ukeqo-^ zp)WgEZ@4S)YMOb>DxJ{U_T8oHmYifix?|sT#>0hIKi)PgzjbIG=U0U(8`oVBy?=R5 z#}(CB#hu#oTeOUp)UIE)^HO5_LzDR2kL-EvQM*{K9o2Yy%W?j}e}`0d?aELwI{rf1 zePWG!xnxoOVaZL8lWofE+$|;2b-o5qulC*j=#zqm=&7Sdf+^>A_r1t?S>4zw!y7!= zxBqp3;BnVFrAhzY&s_NvVEM>NDz*LpzL%Yfa~zpG+YUY1-;|iRmK zXeQ(R3*yFG<3xHIHVYQ4zc0h!YtsI6g+Fuh#C1_pmuBBHx+&WA_(}N0-Xm)Q_sF?Q z%fk{BBwjbD&fD@97@yyOaVv?O4$4!Lbkm9w({oevO7tpnb6O`D<~JJ%w3gSge(LL= zvefC=O{=BB(JP}%48;pXZclv6QdhT9JJ3V>soLv5_saeKbKHy1IZ^O*N(` z!}}f2-%FJE5m5Ntx*Z7Vuc=v z3(N$1crt_cx${@Oea`t_<@VFn^44XVXt`4L+|NqY&Y6`fUFTKKRGryrIWu&nYYd)z z&yVbc+{CifJa}HkECM7j6$48IJpv_y2_9V~sl_F_#mSkvY57IE$*>HsTaZ|i0e1%n z!vzl*lyHMM8tx8!CKdo43rc+!e`US1be`^8t=m@1r%PO4!`e9`t%loUrjWh8?W1gy&&yRZzqd?# zdar9=?)KXY?<$$ZoJ}kK^KnPrNA)W@;+L${EO~CPjkpyMt9>p;;&c4ZFG9)r-5k#v z+y9iiHA}1vWxQHl($uDPP+8}a*J7!^0Rii>j#TinlzXf(ete{@@Hr#(Ya?0szas}094)ZARy>1{3^%jF{9->V@lEL0cQ zpSS8*!Gi7gj{o>(ebz9z$@)TH{`0IK{FY72-O4)NmX+LbpY+4)kKu(U?5KI=2T$v% zB4Fw%17asKQxPc63ySg!Qj1D5Q;T`eY~%tKJPa59?t65=@{wn2Sbwn@Y78ms{WFat-2>VqYcwgE_#Pkx7IZce4TLEg%qJ zc9|MVOHQDy$(ufZ_lDUC0)}if-Id0V=!^nxA88 zh8LmuoB%4!5vItpBKehsf*4@~_QD-z$CAc;GHrks^~e^18tv#M3c{jfcJjP}TD}B$ Tv$BEA<`|Nw>w2!y0-bG$-U9EFx&TkGfxMKX^X_1cCxf43xx=tMIPnLvD5z{`P zQn7PRcaX4mP;kjyt+gUD+(MI$gG6hDPX~P#>G|sO)dwi=WnAoaU{hpgrKdn$*Sc^o zvkP3#cdxj_>BZ?Wz?_EZ0~VmulJj%nj>l91@*_w^VsdUOk}{AVfiiF)1Z2U3qBymv zEHgQ^7_OcJ9CW2IfzNvw85m5N85s0{>fy#Z0?iHv>d8qhan8>xNzBYkE%MCEEXhpF z$*f8((o4=sEH3U1I_-DJLBv+sL+~iO*1kYtUoP&~hh$rh2>AMR9J{hX{uN96lxbUn zoBxSw3yaq^$fxcJ4pFg7xcGZc@v}QGYiq0P7z>*2@(A59*^ruW`v9NwDka;Mt9Up! z-$?Md_9yz_hJfkwcJx^J3f}p!%zJ9g-SQo`A2eS1>2A34UCN1oRn~RKANjdQEKkW1 zytCWsYK?#4B2K**OebT1YBU)g?sq@Z&01LCg<0zRIFKkX;)qR*`FMB$1Ve`^}1Nxq84xN6(jT%Ngo!o(%z_0;fYveG3 z=asC)vcw8K5EqyY^zdW{?{nv`di$L7y;{s7ba16>s^v=6b3ZFpJ7-p|be&f@Q*~yi z<;>8Pt}%GBJwLJwaudr^^Wa$(vjmX9R1D0ydT0d#df8}mRoc1HU%e8qDQ;q9+;(}tKKI6CYv1eY6`#}YeMn3BRgg7%6Z`Y2 z7k_7Un}>2nNvtf{yS}cb-sSsMX6+TF&p4E(OkDeSpH)jjdCQhP?7cgmK4Q4BW7eLU zYPIP^_k8BL@hx_Hyoc9@1*tOKOy||o^*+q!-jVU~ zM0MRExtRM75nnAQruXXn=!h=8`)rC?Sw;JL>3JS;65KlP{MM^@czyc&iD^^aAs)~C zvk%_;nHtw-&0g0xdy~-V7eX;N1fSclbGp}EJXiYMn!6{ng^%ZjGV?usa+_~$%&(%$ zOI<|V_OIMkoOy8l`SJ`WTz zob;c2%@3rU0!l<*A8q(v2#o7eAa*1_&I^k23sQ?pGE<9rPaNbrWWd94p?bg4hT`M5 z8eb%+3w|{4|J``wnC_t{&&^%+>)-#FrL8Iy()HoTp_ZwWGM|;USFd~E|KQGr-v?zM z)XY3s{w_o8qt zg@Nr5xZBX{Ds*Ge3vh%n=D?tZ8w2+u(ZxB!BFsnwmD3O)!0`Y7PGpN`7!N%OfCr5G6~1H!B;+NM0a(2~13e93UP5v8xm( literal 0 HcmV?d00001 diff --git a/jetty-annotations/src/test/java/org/eclipse/jetty/annotations/TestAnnotationConfiguration.java b/jetty-annotations/src/test/java/org/eclipse/jetty/annotations/TestAnnotationConfiguration.java index 4a0b25f79ce..275366c1e1f 100644 --- a/jetty-annotations/src/test/java/org/eclipse/jetty/annotations/TestAnnotationConfiguration.java +++ b/jetty-annotations/src/test/java/org/eclipse/jetty/annotations/TestAnnotationConfiguration.java @@ -28,6 +28,7 @@ import java.net.URL; import java.net.URLClassLoader; import java.util.ArrayList; import java.util.Arrays; +import java.util.Collections; import java.util.HashSet; import java.util.List; import java.util.Set; @@ -37,6 +38,7 @@ import javax.servlet.ServletContainerInitializer; import org.eclipse.jetty.toolchain.test.MavenTestingUtils; import org.eclipse.jetty.util.resource.Resource; import org.eclipse.jetty.webapp.FragmentDescriptor; +import org.eclipse.jetty.webapp.RelativeOrdering; import org.eclipse.jetty.webapp.WebAppContext; import org.junit.jupiter.api.Test; @@ -126,68 +128,148 @@ public class TestAnnotationConfiguration File web25 = MavenTestingUtils.getTestResourceFile("web25.xml"); File web31false = MavenTestingUtils.getTestResourceFile("web31false.xml"); File web31true = MavenTestingUtils.getTestResourceFile("web31true.xml"); - Set sciNames = new HashSet<>(Arrays.asList("org.eclipse.jetty.annotations.ServerServletContainerInitializer", "com.acme.initializer.FooInitializer")); - + //prepare an sci that will be on the webapp's classpath File jarDir = new File(MavenTestingUtils.getTestResourcesDir().getParentFile(), "jar"); File testSciJar = new File(jarDir, "test-sci.jar"); - assertTrue(testSciJar.exists()); - URLClassLoader webAppLoader = new URLClassLoader(new URL[] {testSciJar.toURI().toURL()}, Thread.currentThread().getContextClassLoader()); - - //test 3.1 webapp loads both server and app scis - AnnotationConfiguration config = new AnnotationConfiguration(); - WebAppContext context = new WebAppContext(); - context.setClassLoader(webAppLoader); - context.getMetaData().setWebXml(Resource.newResource(web31true)); - context.getServletContext().setEffectiveMajorVersion(3); - context.getServletContext().setEffectiveMinorVersion(1); - List scis = config.getNonExcludedInitializers(context); - assertNotNull(scis); - assertEquals(2, scis.size()); - assertTrue (sciNames.contains(scis.get(0).getClass().getName())); - assertTrue (sciNames.contains(scis.get(1).getClass().getName())); - - //test a 3.1 webapp with metadata-complete=false loads both server and webapp scis - config = new AnnotationConfiguration(); - context = new WebAppContext(); - context.setClassLoader(webAppLoader); - context.getMetaData().setWebXml(Resource.newResource(web31false)); - context.getServletContext().setEffectiveMajorVersion(3); - context.getServletContext().setEffectiveMinorVersion(1); - scis = config.getNonExcludedInitializers(context); - assertNotNull(scis); - assertEquals(2, scis.size()); - assertTrue (sciNames.contains(scis.get(0).getClass().getName())); - assertTrue (sciNames.contains(scis.get(1).getClass().getName())); - - - //test 2.5 webapp with configurationDiscovered=false loads only server scis - config = new AnnotationConfiguration(); - context = new WebAppContext(); - context.setClassLoader(webAppLoader); - context.getMetaData().setWebXml(Resource.newResource(web25)); - context.getServletContext().setEffectiveMajorVersion(2); - context.getServletContext().setEffectiveMinorVersion(5); - scis = config.getNonExcludedInitializers(context); - assertNotNull(scis); - assertEquals(1, scis.size()); - assertTrue ("org.eclipse.jetty.annotations.ServerServletContainerInitializer".equals(scis.get(0).getClass().getName())); - - //test 2.5 webapp with configurationDiscovered=true loads both server and webapp scis - config = new AnnotationConfiguration(); - context = new WebAppContext(); - context.setConfigurationDiscovered(true); - context.setClassLoader(webAppLoader); - context.getMetaData().setWebXml(Resource.newResource(web25)); - context.getServletContext().setEffectiveMajorVersion(2); - context.getServletContext().setEffectiveMinorVersion(5); - scis = config.getNonExcludedInitializers(context); - assertNotNull(scis); - assertEquals(2, scis.size()); - assertTrue (sciNames.contains(scis.get(0).getClass().getName())); - assertTrue (sciNames.contains(scis.get(1).getClass().getName())); + assertTrue(testSciJar.exists()); + + File testContainerSciJar = new File(jarDir, "test-sci-for-container-path.jar"); + URLClassLoader containerLoader = new URLClassLoader(new URL[] {testContainerSciJar.toURI().toURL()}, Thread.currentThread().getContextClassLoader()); + URLClassLoader webAppLoader = new URLClassLoader(new URL[] {testSciJar.toURI().toURL()}, containerLoader); + Resource targetClasses = Resource.newResource(MavenTestingUtils.getTargetDir().toURI()).addPath("/test-classes"); + + ClassLoader old = Thread.currentThread().getContextClassLoader(); + Thread.currentThread().setContextClassLoader(containerLoader); + try + { + + AnnotationConfiguration config = new AnnotationConfiguration(); + WebAppContext context = new WebAppContext(); + List scis; + + //test 3.1 webapp loads both server and app scis + context.setClassLoader(webAppLoader); + context.getMetaData().addWebInfJar(Resource.newResource(testSciJar.toURI().toURL())); + context.getMetaData().setWebXml(Resource.newResource(web31true)); + context.getMetaData().setWebInfClassesDirs(Collections.singletonList(targetClasses)); + context.getServletContext().setEffectiveMajorVersion(3); + context.getServletContext().setEffectiveMinorVersion(1); + scis = config.getNonExcludedInitializers(context); + assertNotNull(scis); + assertEquals(3, scis.size()); + assertEquals("com.acme.ServerServletContainerInitializer", scis.get(0).getClass().getName()); //container path + assertEquals("org.eclipse.jetty.annotations.WebInfClassServletContainerInitializer", scis.get(1).getClass().getName()); //web-inf classes + assertEquals("com.acme.initializer.FooInitializer", scis.get(2).getClass().getName()); //web-inf jar no web-fragment + + //test a 3.1 webapp with metadata-complete=false loads both server and webapp scis + config = new AnnotationConfiguration(); + context = new WebAppContext(); + context.setClassLoader(webAppLoader); + context.getMetaData().setWebXml(Resource.newResource(web31false)); + context.getMetaData().setWebInfClassesDirs(Collections.singletonList(targetClasses)); + context.getMetaData().addWebInfJar(Resource.newResource(testSciJar.toURI().toURL())); + context.getServletContext().setEffectiveMajorVersion(3); + context.getServletContext().setEffectiveMinorVersion(1); + scis = config.getNonExcludedInitializers(context); + assertNotNull(scis); + assertEquals(3, scis.size()); + assertEquals("com.acme.ServerServletContainerInitializer", scis.get(0).getClass().getName()); //container path + assertEquals("org.eclipse.jetty.annotations.WebInfClassServletContainerInitializer", scis.get(1).getClass().getName()); //web-inf classes + assertEquals("com.acme.initializer.FooInitializer", scis.get(2).getClass().getName()); //web-inf jar no web-fragment + + + //test a 3.1 webapp with RELATIVE ORDERING loads sci from equivalent of WEB-INF/classes as well as container path + File orderedFragmentJar = new File(jarDir, "test-sci-with-ordering.jar"); + assertTrue(orderedFragmentJar.exists()); + URLClassLoader orderedLoader = new URLClassLoader(new URL[] {orderedFragmentJar.toURI().toURL(), testSciJar.toURI().toURL()}, Thread.currentThread().getContextClassLoader()); + config = new AnnotationConfiguration(); + context = new WebAppContext(); + context.setClassLoader(orderedLoader); + context.getMetaData().setWebXml(Resource.newResource(web31true)); + RelativeOrdering ordering = new RelativeOrdering(context.getMetaData()); + context.getMetaData().setOrdering(ordering); + context.getMetaData().addWebInfJar(Resource.newResource(orderedFragmentJar.toURI().toURL())); + context.getMetaData().addWebInfJar(Resource.newResource(testSciJar.toURI().toURL())); + context.getMetaData().setWebInfClassesDirs(Collections.singletonList(targetClasses)); + context.getMetaData().orderFragments(); + context.getServletContext().setEffectiveMajorVersion(3); + context.getServletContext().setEffectiveMinorVersion(1); + scis = config.getNonExcludedInitializers(context); + assertNotNull(scis); + assertEquals(4, scis.size()); + assertEquals("com.acme.ServerServletContainerInitializer", scis.get(0).getClass().getName()); //container path + assertEquals("org.eclipse.jetty.annotations.WebInfClassServletContainerInitializer", scis.get(1).getClass().getName()); //web-inf classes + assertEquals("com.acme.AcmeServletContainerInitializer", scis.get(2).getClass().getName()); //first in ordering + assertEquals("com.acme.initializer.FooInitializer", scis.get(3).getClass().getName()); //other in ordering + + + //test 3.1 webapp with a specific SCI ordering + config = new AnnotationConfiguration(); + context = new WebAppContext(); + context.setClassLoader(webAppLoader); + context.getMetaData().setWebXml(Resource.newResource(web31false)); + context.getMetaData().setWebInfClassesDirs(Collections.singletonList(targetClasses)); + context.getMetaData().addWebInfJar(Resource.newResource(testSciJar.toURI().toURL())); + context.getServletContext().setEffectiveMajorVersion(3); + context.getServletContext().setEffectiveMinorVersion(1); + context.setAttribute("org.eclipse.jetty.containerInitializerOrder", "com.acme.initializer.FooInitializer,com.acme.ServerServletContainerInitializer, *"); + scis = config.getNonExcludedInitializers(context); + assertNotNull(scis); + assertEquals(3, scis.size()); + assertEquals("com.acme.initializer.FooInitializer", scis.get(0).getClass().getName()); //web-inf jar no web-fragment + assertEquals("com.acme.ServerServletContainerInitializer", scis.get(1).getClass().getName()); //container path + assertEquals("org.eclipse.jetty.annotations.WebInfClassServletContainerInitializer", scis.get(2).getClass().getName()); //web-inf classes + + + + + //test 2.5 webapp with configurationDiscovered=false loads only server scis + config = new AnnotationConfiguration(); + context = new WebAppContext(); + context.setClassLoader(webAppLoader); + context.getMetaData().setWebXml(Resource.newResource(web25)); + context.getMetaData().setWebInfClassesDirs(Collections.singletonList(targetClasses)); + context.getMetaData().addWebInfJar(Resource.newResource(testSciJar.toURI().toURL())); + context.getServletContext().setEffectiveMajorVersion(2); + context.getServletContext().setEffectiveMinorVersion(5); + scis = config.getNonExcludedInitializers(context); + assertNotNull(scis); + for (ServletContainerInitializer s:scis) + { + //should not have any of the web-inf lib scis in here + assertFalse(s.getClass().getName().equals("com.acme.AcmeServletContainerInitializer")); + assertFalse(s.getClass().getName().equals("com.acme.initializer.FooInitializer")); + //NOTE: should also not have the web-inf classes scis in here either, but due to the + //way the test is set up, the sci we're pretending is in web-inf classes will actually + //NOT be loaded by the webapp's classloader, but rather by the junit classloader, so + //it looks as if it is a container class. + } + + //test 2.5 webapp with configurationDiscovered=true loads both server and webapp scis + config = new AnnotationConfiguration(); + context = new WebAppContext(); + context.setConfigurationDiscovered(true); + context.setClassLoader(webAppLoader); + context.getMetaData().setWebXml(Resource.newResource(web25)); + context.getMetaData().setWebInfClassesDirs(Collections.singletonList(targetClasses)); + context.getMetaData().addWebInfJar(Resource.newResource(testSciJar.toURI().toURL())); + context.getServletContext().setEffectiveMajorVersion(2); + context.getServletContext().setEffectiveMinorVersion(5); + scis = config.getNonExcludedInitializers(context); + assertNotNull(scis); + assertEquals(3, scis.size()); + assertEquals("com.acme.ServerServletContainerInitializer", scis.get(0).getClass().getName()); //container path + assertEquals("org.eclipse.jetty.annotations.WebInfClassServletContainerInitializer", scis.get(1).getClass().getName()); //web-inf classes + assertEquals("com.acme.initializer.FooInitializer", scis.get(2).getClass().getName()); //web-inf jar no web-fragment + + } + finally + { + Thread.currentThread().setContextClassLoader(old); + } } - + @Test public void testGetFragmentFromJar() throws Exception diff --git a/jetty-annotations/src/test/java/org/eclipse/jetty/annotations/ServerServletContainerInitializer.java b/jetty-annotations/src/test/java/org/eclipse/jetty/annotations/WebInfClassServletContainerInitializer.java similarity index 90% rename from jetty-annotations/src/test/java/org/eclipse/jetty/annotations/ServerServletContainerInitializer.java rename to jetty-annotations/src/test/java/org/eclipse/jetty/annotations/WebInfClassServletContainerInitializer.java index 3fbfadce4c5..f770b0d19db 100644 --- a/jetty-annotations/src/test/java/org/eclipse/jetty/annotations/ServerServletContainerInitializer.java +++ b/jetty-annotations/src/test/java/org/eclipse/jetty/annotations/WebInfClassServletContainerInitializer.java @@ -30,13 +30,13 @@ import javax.servlet.ServletException; * * */ -public class ServerServletContainerInitializer implements ServletContainerInitializer +public class WebInfClassServletContainerInitializer implements ServletContainerInitializer { /** * */ - public ServerServletContainerInitializer() + public WebInfClassServletContainerInitializer() { // TODO Auto-generated constructor stub } diff --git a/jetty-annotations/src/test/resources/META-INF/services/javax.servlet.ServletContainerInitializer b/jetty-annotations/src/test/resources/META-INF/services/javax.servlet.ServletContainerInitializer index 8193f60e4a7..cc22e474171 100644 --- a/jetty-annotations/src/test/resources/META-INF/services/javax.servlet.ServletContainerInitializer +++ b/jetty-annotations/src/test/resources/META-INF/services/javax.servlet.ServletContainerInitializer @@ -1 +1 @@ -org.eclipse.jetty.annotations.ServerServletContainerInitializer \ No newline at end of file +org.eclipse.jetty.annotations.WebInfClassServletContainerInitializer \ No newline at end of file From fb06b821725a1c58b00cf927a0a70df6fb3a82d1 Mon Sep 17 00:00:00 2001 From: Greg Wilkins Date: Thu, 13 Dec 2018 13:42:08 +1100 Subject: [PATCH 03/11] Issue #3030 ignore identity content encoding during parameter extraction Signed-off-by: Greg Wilkins --- .../org/eclipse/jetty/server/Request.java | 9 ++++-- .../org/eclipse/jetty/server/RequestTest.java | 28 +++++++++++++------ 2 files changed, 26 insertions(+), 11 deletions(-) diff --git a/jetty-server/src/main/java/org/eclipse/jetty/server/Request.java b/jetty-server/src/main/java/org/eclipse/jetty/server/Request.java index d29d0431d54..314242ec218 100644 --- a/jetty-server/src/main/java/org/eclipse/jetty/server/Request.java +++ b/jetty-server/src/main/java/org/eclipse/jetty/server/Request.java @@ -68,6 +68,7 @@ import org.eclipse.jetty.http.HttpCookie; import org.eclipse.jetty.http.HttpField; import org.eclipse.jetty.http.HttpFields; import org.eclipse.jetty.http.HttpHeader; +import org.eclipse.jetty.http.HttpHeaderValue; import org.eclipse.jetty.http.HttpMethod; import org.eclipse.jetty.http.HttpScheme; import org.eclipse.jetty.http.HttpStatus; @@ -468,8 +469,12 @@ public class Request implements HttpServletRequest if (MimeTypes.Type.FORM_ENCODED.is(contentType) && _channel.getHttpConfiguration().isFormEncodedMethod(getMethod())) { - if (_metaData!=null && getHttpFields().contains(HttpHeader.CONTENT_ENCODING)) - throw new BadMessageException(HttpStatus.NOT_IMPLEMENTED_501,"Unsupported Content-Encoding"); + if (_metaData!=null) + { + String contentEncoding = getHttpFields().get(HttpHeader.CONTENT_ENCODING); + if (contentEncoding!=null && !HttpHeaderValue.IDENTITY.is(contentEncoding)) + throw new BadMessageException(HttpStatus.NOT_IMPLEMENTED_501, "Unsupported Content-Encoding"); + } extractFormParameters(_contentParameters); } else if (MimeTypes.Type.MULTIPART_FORM_DATA.is(contentType) && diff --git a/jetty-server/src/test/java/org/eclipse/jetty/server/RequestTest.java b/jetty-server/src/test/java/org/eclipse/jetty/server/RequestTest.java index 802e7b10a28..4b8ec31ac4f 100644 --- a/jetty-server/src/test/java/org/eclipse/jetty/server/RequestTest.java +++ b/jetty-server/src/test/java/org/eclipse/jetty/server/RequestTest.java @@ -647,18 +647,29 @@ public class RequestTest assertThat(responses,startsWith("HTTP/1.1 200")); } + @Test + public void testIdentityParamExtraction() throws Exception + { + _handler._checker = (request, response) -> "bar".equals(request.getParameter("foo")); + + //Send a request with encoded form content + String request="POST / HTTP/1.1\r\n"+ + "Host: whatever\r\n"+ + "Content-Type: application/x-www-form-urlencoded; charset=utf-8\n"+ + "Content-Length: 7\n"+ + "Content-Encoding: identity\n"+ + "Connection: close\n"+ + "\n"+ + "foo=bar\n"; + + String responses=_connector.getResponse(request); + assertThat(responses,startsWith("HTTP/1.1 200")); + } @Test public void testEncodedNotParams() throws Exception { - _handler._checker = new RequestTester() - { - @Override - public boolean check(HttpServletRequest request,HttpServletResponse response) - { - return request.getParameter("param")==null; - } - }; + _handler._checker = (request, response) -> request.getParameter("param")==null; //Send a request with encoded form content String request="POST / HTTP/1.1\r\n"+ @@ -674,7 +685,6 @@ public class RequestTest assertThat(responses,startsWith("HTTP/1.1 200")); } - @Test public void testInvalidHostHeader() throws Exception { From 3acb98f47730e79f7439e1e48b3a66edbdb4e437 Mon Sep 17 00:00:00 2001 From: Jan Bartel Date: Thu, 13 Dec 2018 14:57:14 +1100 Subject: [PATCH 04/11] Issue #3161 Update to Apache JSP 8.5.35 (#3163) * Issue #3161 Update to Apache JSP 8.5.35 Signed-off-by: Jan Bartel --- pom.xml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pom.xml b/pom.xml index 6964d749aa7..e8e918d8cf0 100644 --- a/pom.xml +++ b/pom.xml @@ -24,7 +24,7 @@ 1.2.3 1.2 1.1.3.v20160715 - 8.5.33.1 + 8.5.35.1 undefined 1.4.1 From 7c4d12fb7dae8874df2bb3200d63bc1e735ac46e Mon Sep 17 00:00:00 2001 From: WalkerWatch Date: Thu, 13 Dec 2018 13:43:22 -0500 Subject: [PATCH 05/11] Updating for customrequestlog impl Signed-off-by: WalkerWatch --- .../configuring-jetty-request-logs.adoc | 24 ++++++++++++++++++- 1 file changed, 23 insertions(+), 1 deletion(-) diff --git a/jetty-documentation/src/main/asciidoc/administration/logging/configuring-jetty-request-logs.adoc b/jetty-documentation/src/main/asciidoc/administration/logging/configuring-jetty-request-logs.adoc index dd452a02efd..44c7334c761 100644 --- a/jetty-documentation/src/main/asciidoc/administration/logging/configuring-jetty-request-logs.adoc +++ b/jetty-documentation/src/main/asciidoc/administration/logging/configuring-jetty-request-logs.adoc @@ -91,7 +91,29 @@ By default, log files are kept for 90 days before being deleted. The value for `retainDays` (xml) or `setRetainDays` (Java) should be configured as _1 + n_ days. For example, if you wanted to keep the logs for the current day and the day prior you would set the `retainDays` (or `setRetainDays`) value to 2. -To examine more configuration options, see link:{JDURL}/org/eclipse/jetty/server/NCSARequestLog.html[NCSARequestLog.java]. + +[[request-log-custom-writer]] +==== Introducing RequestLog.Writer + +The concept of a `RequestLog.Writer`, introduced in Jetty 9.4.15, manages the writing to a log the string generated by the `RequestLog`. +This allows the `CustomRequestLog` to match the functionality of other `RequestLogger` implementations by plugging in any `RequestLog.Writer` and a custom format string. +Jetty currently has implementations of `RequestLog.Writer`, `RequestLogWriter`, `AsyncRequestLogWriter`, and `Slf4jRequestLogWriter`. + +So, the way to create an asynchronous `RequestLog` using the extended NCSA format has been changed from: + +`new AsyncNcsaRequestLog(filename)` + +to: + +`new CustomRequestLog(new AsyncRequestLogWriter(filename), CustomRequestLog.EXTENDED_NCSA_FORMAT)` + +Additionally, there are now two settings for the log timezone to be configured. +There is the configuration for logging the request time, which is set in the `timeZone` parameter in the `%t` format code of the string, given in the format `%{format|timeZone|locale}t`. + +The other `timeZone` parameter relates to the generation of the log file name (both at creation and roll over). +This is configured in the `requestlog` module file, or can be used as a setter on `RequestLogWriter` via XML. + +Both timezones are set to GMT by default. [[configuring-separate-request-log-for-web-application]] ==== Configuring a Separate Request Log For a Web Application From f26b9f7c156721788e23fe8b322e529c4cf7f1f5 Mon Sep 17 00:00:00 2001 From: Olivier Lamy Date: Sat, 15 Dec 2018 12:35:16 +1000 Subject: [PATCH 06/11] issue #3186 add it to override javax.annotation provided by maven core (#3194) Signed-off-by: olivier lamy --- Jenkinsfile | 18 ++- .../javax-annotation-api/invoker.properties | 1 + .../src/it/javax-annotation-api/pom.xml | 103 ++++++++++++++++++ .../it/javax-annotation-api/postbuild.groovy | 21 ++++ .../src/main/java/test/App.java | 64 +++++++++++ .../src/main/resources/my.properties | 1 + 6 files changed, 197 insertions(+), 11 deletions(-) create mode 100644 jetty-maven-plugin/src/it/javax-annotation-api/invoker.properties create mode 100644 jetty-maven-plugin/src/it/javax-annotation-api/pom.xml create mode 100644 jetty-maven-plugin/src/it/javax-annotation-api/postbuild.groovy create mode 100644 jetty-maven-plugin/src/it/javax-annotation-api/src/main/java/test/App.java create mode 100644 jetty-maven-plugin/src/it/javax-annotation-api/src/main/resources/my.properties diff --git a/Jenkinsfile b/Jenkinsfile index cc9bec25fd4..538bb9b8ac1 100644 --- a/Jenkinsfile +++ b/Jenkinsfile @@ -9,7 +9,7 @@ pipeline { agent { node { label 'linux' } } options { timeout(time: 120, unit: 'MINUTES') } steps { - mavenBuild("jdk8", "-Pmongodb install") + mavenBuild("jdk8", "-Pmongodb install", "maven3") // Collect up the jacoco execution results (only on main build) jacoco inclusionPattern: '**/org/eclipse/jetty/**/*.class', exclusionPattern: '' + @@ -34,11 +34,7 @@ pipeline { classPattern: '**/target/classes', sourcePattern: '**/src/main/java' warnings consoleParsers: [[parserName: 'Maven'], [parserName: 'Java']] - - script { - step([$class : 'MavenInvokerRecorder', reportsFilenamePattern: "**/target/invoker-reports/BUILD*.xml", - invokerBuildDir: "**/target/its"]) - } + maven_invoker reportsFilenamePattern: "**/target/invoker-reports/BUILD*.xml", invokerBuildDir: "**/target/its" } } @@ -46,9 +42,10 @@ pipeline { agent { node { label 'linux' } } options { timeout(time: 120, unit: 'MINUTES') } steps { - mavenBuild("jdk11", "-Pmongodb install") + mavenBuild("jdk11", "-Pmongodb install", "maven3") junit '**/target/surefire-reports/TEST-*.xml,**/target/failsafe-reports/TEST-*.xml' warnings consoleParsers: [[parserName: 'Maven'], [parserName: 'Java']] + maven_invoker reportsFilenamePattern: "**/target/invoker-reports/BUILD*.xml", invokerBuildDir: "**/target/its" } } @@ -56,7 +53,7 @@ pipeline { agent { node { label 'linux' } } options { timeout(time: 30, unit: 'MINUTES') } steps { - mavenBuild("jdk8", "install javadoc:javadoc -DskipTests") + mavenBuild("jdk8", "install javadoc:javadoc -DskipTests", "maven3") warnings consoleParsers: [[parserName: 'Maven'], [parserName: 'JavaDoc'], [parserName: 'Java']] } } @@ -65,7 +62,7 @@ pipeline { agent { node { label 'linux' } } options { timeout(time: 120, unit: 'MINUTES') } steps { - mavenBuild("jdk8", "-Pcompact3 install -DskipTests") + mavenBuild("jdk8", "-Pcompact3 install -DskipTests", "maven3") warnings consoleParsers: [[parserName: 'Maven'], [parserName: 'Java']] } } @@ -83,8 +80,7 @@ pipeline { * @param cmdline the command line in " "`format. * @return the Jenkinsfile step representing a maven build */ -def mavenBuild(jdk, cmdline) { - def mvnName = 'maven3.5' +def mavenBuild(jdk, cmdline, mvnName) { def localRepo = "${env.JENKINS_HOME}/${env.EXECUTOR_NUMBER}" // ".repository" // def settingsName = 'oss-settings.xml' def mavenOpts = '-Xms1g -Xmx4g -Djava.awt.headless=true' diff --git a/jetty-maven-plugin/src/it/javax-annotation-api/invoker.properties b/jetty-maven-plugin/src/it/javax-annotation-api/invoker.properties new file mode 100644 index 00000000000..e0222d4d54e --- /dev/null +++ b/jetty-maven-plugin/src/it/javax-annotation-api/invoker.properties @@ -0,0 +1 @@ +invoker.goals = test \ No newline at end of file diff --git a/jetty-maven-plugin/src/it/javax-annotation-api/pom.xml b/jetty-maven-plugin/src/it/javax-annotation-api/pom.xml new file mode 100644 index 00000000000..4ef25cb17c3 --- /dev/null +++ b/jetty-maven-plugin/src/it/javax-annotation-api/pom.xml @@ -0,0 +1,103 @@ + + + + 4.0.0 + + + + + org.eclipse.jetty.its + it-parent-pom + 0.0.1-SNAPSHOT + + + javax-annotation-api-test + 1.0.0-SNAPSHOT + war + + + + UTF-8 + 8 + 8 + + ${project.build.directory}/jetty-run-mojo-annotation.txt + 1.3.2 + + + + + javax.servlet + javax.servlet-api + 4.0.1 + provided + + + javax.annotation + javax.annotation-api + ${annotation-api.version} + + + org.springframework.boot + spring-boot-autoconfigure + 2.1.1.RELEASE + + + org.springframework.boot + spring-boot-starter-web + 2.1.1.RELEASE + + + org.springframework.boot + spring-boot-starter-tomcat + + + + + + + + + + org.eclipse.jetty + jetty-maven-plugin + + + start-jetty + test-compile + + start + + + + + jetty.port.file + ${jetty.port.file} + + + true + + + + + + + + + javax.annotation + javax.annotation-api + ${annotation-api.version} + + + javax.annotation + jsr250-api + 1.0 + + + + diff --git a/jetty-maven-plugin/src/it/javax-annotation-api/postbuild.groovy b/jetty-maven-plugin/src/it/javax-annotation-api/postbuild.groovy new file mode 100644 index 00000000000..9533c9ecc2c --- /dev/null +++ b/jetty-maven-plugin/src/it/javax-annotation-api/postbuild.groovy @@ -0,0 +1,21 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ +File buildLog = new File( basedir, 'build.log' ) +assert buildLog.text.contains( 'Started Jetty Server' ) +assert buildLog.text.contains( 'all good guys get a good Beer') diff --git a/jetty-maven-plugin/src/it/javax-annotation-api/src/main/java/test/App.java b/jetty-maven-plugin/src/it/javax-annotation-api/src/main/java/test/App.java new file mode 100644 index 00000000000..d03ad70999b --- /dev/null +++ b/jetty-maven-plugin/src/it/javax-annotation-api/src/main/java/test/App.java @@ -0,0 +1,64 @@ +// +// ======================================================================== +// Copyright (c) 1995-2018 Mort Bay Consulting Pty. Ltd. +// ------------------------------------------------------------------------ +// All rights reserved. This program and the accompanying materials +// are made available under the terms of the Eclipse Public License v1.0 +// and Apache License v2.0 which accompanies this distribution. +// +// The Eclipse Public License is available at +// http://www.eclipse.org/legal/epl-v10.html +// +// The Apache License v2.0 is available at +// http://www.opensource.org/licenses/apache2.0.php +// +// You may elect to redistribute this code under either of these licenses. +// ======================================================================== +// + +package test; + +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; +import org.springframework.boot.builder.SpringApplicationBuilder; +import org.springframework.boot.web.servlet.support.SpringBootServletInitializer; +import org.springframework.context.annotation.Bean; + +import javax.annotation.PostConstruct; +import javax.annotation.Resource; +import java.io.InputStream; +import java.util.Properties; + +/** + * Hello world! + * + */ +public class App extends SpringBootServletInitializer { + + private Logger logger = LoggerFactory.getLogger( getClass() ); + + @Resource(name="my.properties") + private Properties somePropertyFile; + + @Override + protected SpringApplicationBuilder configure(SpringApplicationBuilder builder ) { + return builder.sources( App.class ); + } + + @PostConstruct + public void done(){ + logger.info( "all good guys get a good {}", somePropertyFile.get( "drink" ) ); + + } + + @Bean(name = "my.properties") + public Properties getSomeProperties() throws Exception{ + Properties properties = new Properties( ); + try(InputStream inputStream = Thread.currentThread().getContextClassLoader().getResourceAsStream( "my.properties" )) + { + properties.load( inputStream ); + } + return properties; + } + +} diff --git a/jetty-maven-plugin/src/it/javax-annotation-api/src/main/resources/my.properties b/jetty-maven-plugin/src/it/javax-annotation-api/src/main/resources/my.properties new file mode 100644 index 00000000000..5c07b0ed01e --- /dev/null +++ b/jetty-maven-plugin/src/it/javax-annotation-api/src/main/resources/my.properties @@ -0,0 +1 @@ +drink=Beer From 471dab752bad764112393c40666e419b14cc4022 Mon Sep 17 00:00:00 2001 From: olivier lamy Date: Thu, 13 Dec 2018 18:04:03 +1000 Subject: [PATCH 07/11] Update to Maven JAR Plugin 3.1.1 to pick up the fix of MJAR-241 surefire plugin 3.0.0-M2 remove use of Automatic-Module-Name jar plugin property so we avoid failure on jpms module name validation by jar plugin 3.1.1 Signed-off-by: olivier lamy --- jetty-jndi/pom.xml | 10 +++++----- pom.xml | 6 ++---- 2 files changed, 7 insertions(+), 9 deletions(-) diff --git a/jetty-jndi/pom.xml b/jetty-jndi/pom.xml index f36f315c200..8a62a86eee5 100644 --- a/jetty-jndi/pom.xml +++ b/jetty-jndi/pom.xml @@ -18,11 +18,11 @@ org.apache.felix maven-bundle-plugin true - - - javax.mail.*;resolution:=optional,* - - + + + javax.mail.*;resolution:=optional,* + + org.apache.maven.plugins diff --git a/pom.xml b/pom.xml index e8e918d8cf0..8037056b5b5 100644 --- a/pom.xml +++ b/pom.xml @@ -41,10 +41,9 @@ false - ${bundle-symbolic-name} - 2.22.1 + 3.0.0-M2 3.8.0 3.1.1 3.1.0 @@ -485,12 +484,11 @@ org.apache.maven.plugins maven-jar-plugin - 3.1.0 + 3.1.1 ${project.build.outputDirectory}/META-INF/MANIFEST.MF - ${jpms-module-name} ${project.version} Eclipse Jetty Project ${jetty.url} From 9af977fa932071f0d094a86e129de62195bf71aa Mon Sep 17 00:00:00 2001 From: olivier lamy Date: Mon, 17 Dec 2018 09:24:11 +1000 Subject: [PATCH 08/11] fix async-rest-jar bundle name typo Signed-off-by: olivier lamy --- examples/async-rest/async-rest-jar/pom.xml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/examples/async-rest/async-rest-jar/pom.xml b/examples/async-rest/async-rest-jar/pom.xml index 9829c3940ab..2935041bd8c 100644 --- a/examples/async-rest/async-rest-jar/pom.xml +++ b/examples/async-rest/async-rest-jar/pom.xml @@ -11,7 +11,7 @@ Example Async Rest :: Jar http://www.eclipse.org/jetty - ${project.groupId}.examples.asyc.rest + ${project.groupId}.examples.async.rest From 34b6ecec6c53e3f94b4247dbac2f25d145bee727 Mon Sep 17 00:00:00 2001 From: Jan Bartel Date: Tue, 18 Dec 2018 09:32:12 +1100 Subject: [PATCH 09/11] Issue #3202 Ensure sessions created during async are cleaned up. Signed-off-by: Jan Bartel --- .../jetty/server/session/SessionHandler.java | 18 ++- .../jetty/server/session/TestFooServlet.java | 20 ++- .../jetty/server/session/AsyncTest.java | 119 ++++++++++++++++++ 3 files changed, 150 insertions(+), 7 deletions(-) create mode 100644 tests/test-sessions/test-sessions-common/src/test/java/org/eclipse/jetty/server/session/AsyncTest.java diff --git a/jetty-server/src/main/java/org/eclipse/jetty/server/session/SessionHandler.java b/jetty-server/src/main/java/org/eclipse/jetty/server/session/SessionHandler.java index abcd95afb3a..af5232ee7c6 100644 --- a/jetty-server/src/main/java/org/eclipse/jetty/server/session/SessionHandler.java +++ b/jetty-server/src/main/java/org/eclipse/jetty/server/session/SessionHandler.java @@ -407,6 +407,9 @@ public class SessionHandler extends ScopedHandler */ public void complete(HttpSession session) { + if (LOG.isDebugEnabled()) + LOG.debug("Complete called with session {}", session); + if (session == null) return; @@ -428,6 +431,8 @@ public class SessionHandler extends ScopedHandler { if (request.isAsyncStarted() && request.getDispatcherType() == DispatcherType.REQUEST) { + if (LOG.isDebugEnabled()) + LOG.debug("Adding AsyncListener for {}", request); request.getAsyncContext().addListener(_sessionAsyncListener); } else @@ -1599,6 +1604,9 @@ public class SessionHandler extends ScopedHandler try { + if (LOG.isDebugEnabled()) + LOG.debug("SessionHandler.doScope"); + old_session_manager = baseRequest.getSessionHandler(); old_session = baseRequest.getSession(false); @@ -1622,10 +1630,7 @@ public class SessionHandler extends ScopedHandler } if (LOG.isDebugEnabled()) - { - LOG.debug("sessionHandler=" + this); - LOG.debug("session=" + existingSession); - } + LOG.debug("sessionHandler={} session={}",this, existingSession); if (_nextScope != null) _nextScope.doScope(target,baseRequest,request,response); @@ -1638,8 +1643,9 @@ public class SessionHandler extends ScopedHandler { //if there is a session that was created during handling this context, then complete it HttpSession finalSession = baseRequest.getSession(false); - if (LOG.isDebugEnabled()) LOG.debug("FinalSession="+finalSession+" old_session_manager="+old_session_manager+" this="+this); - if ((finalSession != null) && (old_session_manager != this)) + if (LOG.isDebugEnabled()) + LOG.debug("FinalSession={}, old_session_manager={}, this={}, calling complete={}", finalSession, old_session_manager, this, (old_session_manager != this)); + if (old_session_manager != this) { complete((Session)finalSession, baseRequest); } diff --git a/tests/test-sessions/test-sessions-common/src/main/java/org/eclipse/jetty/server/session/TestFooServlet.java b/tests/test-sessions/test-sessions-common/src/main/java/org/eclipse/jetty/server/session/TestFooServlet.java index 548a5498106..621c95b74c4 100644 --- a/tests/test-sessions/test-sessions-common/src/main/java/org/eclipse/jetty/server/session/TestFooServlet.java +++ b/tests/test-sessions/test-sessions-common/src/main/java/org/eclipse/jetty/server/session/TestFooServlet.java @@ -21,6 +21,7 @@ package org.eclipse.jetty.server.session; import java.io.IOException; import java.lang.reflect.Proxy; +import javax.servlet.AsyncContext; import javax.servlet.ServletException; import javax.servlet.http.HttpServlet; import javax.servlet.http.HttpServletRequest; @@ -54,6 +55,23 @@ public class TestFooServlet extends HttpServlet if (foo == null || foo.getInt() != 33) response.sendError(500, "Foo not deserialized"); } - + else if ("async".equals(action)) + { + if (request.getAttribute("async-test") == null) + { + request.setAttribute("async-test", Boolean.TRUE); + AsyncContext acontext = request.startAsync(); + System.err.println("Starting async and dispatching"); + + acontext.dispatch(); + return; + } + else + { + HttpSession session = request.getSession(true); + System.err.println("After dispatch and finishing response"); + response.getWriter().println("OK"); + } + } } } diff --git a/tests/test-sessions/test-sessions-common/src/test/java/org/eclipse/jetty/server/session/AsyncTest.java b/tests/test-sessions/test-sessions-common/src/test/java/org/eclipse/jetty/server/session/AsyncTest.java new file mode 100644 index 00000000000..cfc683d2530 --- /dev/null +++ b/tests/test-sessions/test-sessions-common/src/test/java/org/eclipse/jetty/server/session/AsyncTest.java @@ -0,0 +1,119 @@ +// +// ======================================================================== +// Copyright (c) 1995-2018 Mort Bay Consulting Pty. Ltd. +// ------------------------------------------------------------------------ +// All rights reserved. This program and the accompanying materials +// are made available under the terms of the Eclipse Public License v1.0 +// and Apache License v2.0 which accompanies this distribution. +// +// The Eclipse Public License is available at +// http://www.eclipse.org/legal/epl-v10.html +// +// The Apache License v2.0 is available at +// http://www.opensource.org/licenses/apache2.0.php +// +// You may elect to redistribute this code under either of these licenses. +// ======================================================================== +// + + +package org.eclipse.jetty.server.session; + +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertFalse; +import static org.junit.jupiter.api.Assertions.assertTrue; + +import java.io.IOException; + +import javax.servlet.ServletException; +import javax.servlet.http.HttpServlet; +import javax.servlet.http.HttpServletRequest; +import javax.servlet.http.HttpServletResponse; + +import org.eclipse.jetty.client.HttpClient; +import org.eclipse.jetty.client.api.ContentResponse; +import org.eclipse.jetty.servlet.ServletContextHandler; +import org.eclipse.jetty.servlet.ServletHolder; +import org.eclipse.jetty.util.log.Log; +import org.eclipse.jetty.util.log.StacklessLogging; +import org.junit.jupiter.api.Test; + + + +/** + * AsyncTest + * + * Tests async handling wrt sessions. + */ +public class AsyncTest +{ + + public static class LatchServlet extends HttpServlet + { + + @Override + protected void doGet(HttpServletRequest req, HttpServletResponse resp) throws ServletException, IOException + { + resp.getWriter().println("Latched"); + } + + } + + + /** + * Test async with a session. + * + * @throws Exception + */ + @Test + public void testSessionWithAsync() throws Exception + { + + + + DefaultSessionCacheFactory cacheFactory = new DefaultSessionCacheFactory(); + cacheFactory.setEvictionPolicy(SessionCache.EVICT_ON_SESSION_EXIT); + SessionDataStoreFactory storeFactory = new TestSessionDataStoreFactory(); + TestServer server1 = new TestServer(0, -1, -1, cacheFactory, storeFactory); + + String contextPath = ""; + String fooMapping = "/server"; + TestFooServlet servlet = new TestFooServlet(); + ServletHolder holder = new ServletHolder(servlet); + ServletContextHandler contextHandler = server1.addContext(contextPath); + contextHandler.addServlet(holder, fooMapping); + LatchServlet latchServlet = new LatchServlet(); + ServletHolder latchHolder = new ServletHolder(latchServlet); + contextHandler.addServlet(latchHolder, "/latch"); + + server1.start(); + int port1 = server1.getPort(); + + try (StacklessLogging stackless = new StacklessLogging(Log.getLogger("org.eclipse.jetty.server.session"))) + { + HttpClient client = new HttpClient(); + client.start(); + String url = "http://localhost:" + port1 + contextPath + fooMapping+"?action=async"; + + //make a request to set up a session on the server + ContentResponse response = client.GET(url); + assertEquals(HttpServletResponse.SC_OK,response.getStatus()); + + String sessionCookie = response.getHeaders().get("Set-Cookie"); + assertTrue(sessionCookie != null); + + //make another request, when this is handled, the first request is definitely finished being handled + response = client.GET("http://localhost:" + port1 + contextPath + "/latch"); + assertEquals(HttpServletResponse.SC_OK,response.getStatus()); + + //session should now be evicted from the cache after request exited + String id = TestServer.extractSessionId(sessionCookie); + assertFalse(contextHandler.getSessionHandler().getSessionCache().contains(id)); + assertTrue(contextHandler.getSessionHandler().getSessionCache().getSessionDataStore().exists(id)); + } + finally + { + server1.stop(); + } + } +} From 06bbab50f9a96f5e206aa1dc26743dcc428c5ed5 Mon Sep 17 00:00:00 2001 From: Greg Wilkins Date: Tue, 18 Dec 2018 14:07:16 +1100 Subject: [PATCH 10/11] Issue #3202 Handle async cross context session completion --- .../jetty/server/AsyncContextState.java | 69 +++-- .../jetty/server/HttpChannelState.java | 19 ++ .../session/DefaultSessionIdManager.java | 2 +- .../eclipse/jetty/server/session/Session.java | 14 + .../jetty/server/session/SessionHandler.java | 54 ++-- .../nosql/mongodb/AttributeNameTest.java | 2 +- .../jetty/server/session/TestFooServlet.java | 77 ----- .../jetty/server/session/AsyncTest.java | 284 ++++++++++++++++-- .../session/DeleteUnloadableSessionTest.java | 7 +- .../session/SessionEvictionFailureTest.java | 9 +- 10 files changed, 370 insertions(+), 167 deletions(-) delete mode 100644 tests/test-sessions/test-sessions-common/src/main/java/org/eclipse/jetty/server/session/TestFooServlet.java diff --git a/jetty-server/src/main/java/org/eclipse/jetty/server/AsyncContextState.java b/jetty-server/src/main/java/org/eclipse/jetty/server/AsyncContextState.java index 4498db8f6e4..596d6055069 100644 --- a/jetty-server/src/main/java/org/eclipse/jetty/server/AsyncContextState.java +++ b/jetty-server/src/main/java/org/eclipse/jetty/server/AsyncContextState.java @@ -58,32 +58,7 @@ public class AsyncContextState implements AsyncContext @Override public void addListener(final AsyncListener listener, final ServletRequest request, final ServletResponse response) { - AsyncListener wrap = new AsyncListener() - { - @Override - public void onTimeout(AsyncEvent event) throws IOException - { - listener.onTimeout(new AsyncEvent(event.getAsyncContext(),request,response,event.getThrowable())); - } - - @Override - public void onStartAsync(AsyncEvent event) throws IOException - { - listener.onStartAsync(new AsyncEvent(event.getAsyncContext(),request,response,event.getThrowable())); - } - - @Override - public void onError(AsyncEvent event) throws IOException - { - listener.onError(new AsyncEvent(event.getAsyncContext(),request,response,event.getThrowable())); - } - - @Override - public void onComplete(AsyncEvent event) throws IOException - { - listener.onComplete(new AsyncEvent(event.getAsyncContext(),request,response,event.getThrowable())); - } - }; + AsyncListener wrap = new WrappedAsyncListener(listener, request, response); state().addListener(wrap); } @@ -188,6 +163,46 @@ public class AsyncContextState implements AsyncContext return state(); } - + public static class WrappedAsyncListener implements AsyncListener + { + private final AsyncListener _listener; + private final ServletRequest _request; + private final ServletResponse _response; + public WrappedAsyncListener(AsyncListener listener, ServletRequest request, ServletResponse response) + { + _listener = listener; + _request = request; + _response = response; + } + + public AsyncListener getListener() + { + return _listener; + } + + @Override + public void onTimeout(AsyncEvent event) throws IOException + { + _listener.onTimeout(new AsyncEvent(event.getAsyncContext(), _request, _response,event.getThrowable())); + } + + @Override + public void onStartAsync(AsyncEvent event) throws IOException + { + _listener.onStartAsync(new AsyncEvent(event.getAsyncContext(), _request, _response,event.getThrowable())); + } + + @Override + public void onError(AsyncEvent event) throws IOException + { + _listener.onError(new AsyncEvent(event.getAsyncContext(), _request, _response,event.getThrowable())); + } + + @Override + public void onComplete(AsyncEvent event) throws IOException + { + _listener.onComplete(new AsyncEvent(event.getAsyncContext(), _request, _response,event.getThrowable())); + } + } } diff --git a/jetty-server/src/main/java/org/eclipse/jetty/server/HttpChannelState.java b/jetty-server/src/main/java/org/eclipse/jetty/server/HttpChannelState.java index cbe1c3934a6..78e009ea839 100644 --- a/jetty-server/src/main/java/org/eclipse/jetty/server/HttpChannelState.java +++ b/jetty-server/src/main/java/org/eclipse/jetty/server/HttpChannelState.java @@ -149,6 +149,25 @@ public class HttpChannelState } } + public boolean hasListener(AsyncListener listener) + { + try(Locker.Lock lock= _locker.lock()) + { + if (_asyncListeners==null) + return false; + for (AsyncListener l : _asyncListeners) + { + if (l==listener) + return true; + + if (l instanceof AsyncContextState.WrappedAsyncListener && ((AsyncContextState.WrappedAsyncListener)l).getListener()==listener) + return true; + } + + return false; + } + } + public void setTimeout(long ms) { try(Locker.Lock lock= _locker.lock()) diff --git a/jetty-server/src/main/java/org/eclipse/jetty/server/session/DefaultSessionIdManager.java b/jetty-server/src/main/java/org/eclipse/jetty/server/session/DefaultSessionIdManager.java index b61af4cbef1..70770c6fa7a 100644 --- a/jetty-server/src/main/java/org/eclipse/jetty/server/session/DefaultSessionIdManager.java +++ b/jetty-server/src/main/java/org/eclipse/jetty/server/session/DefaultSessionIdManager.java @@ -55,7 +55,7 @@ public class DefaultSessionIdManager extends ContainerLifeCycle implements Sessi { private final static Logger LOG = Log.getLogger("org.eclipse.jetty.server.session"); - private final static String __NEW_SESSION_ID="org.eclipse.jetty.server.newSessionId"; + public final static String __NEW_SESSION_ID="org.eclipse.jetty.server.newSessionId"; protected static final AtomicLong COUNTER = new AtomicLong(); diff --git a/jetty-server/src/main/java/org/eclipse/jetty/server/session/Session.java b/jetty-server/src/main/java/org/eclipse/jetty/server/session/Session.java index 673b400ecd4..cd0fe8c3d03 100644 --- a/jetty-server/src/main/java/org/eclipse/jetty/server/session/Session.java +++ b/jetty-server/src/main/java/org/eclipse/jetty/server/session/Session.java @@ -1148,4 +1148,18 @@ public class Session implements SessionHandler.SessionIf return _resident; } + @Override + public String toString() + { + try (Lock lock = _lock.lock()) + { + return String.format("%s@%x{id=%s,x=%s,req=%d,res=%b}", + getClass().getSimpleName(), + hashCode(), + _sessionData.getId(), + _extendedId, + _requests, + _resident); + } + } } diff --git a/jetty-server/src/main/java/org/eclipse/jetty/server/session/SessionHandler.java b/jetty-server/src/main/java/org/eclipse/jetty/server/session/SessionHandler.java index af5232ee7c6..ac197b78999 100644 --- a/jetty-server/src/main/java/org/eclipse/jetty/server/session/SessionHandler.java +++ b/jetty-server/src/main/java/org/eclipse/jetty/server/session/SessionHandler.java @@ -162,8 +162,22 @@ public class SessionHandler extends ScopedHandler @Override public void onComplete(AsyncEvent event) throws IOException { - //An async request has completed, so we can complete the session - complete(Request.getBaseRequest(event.getAsyncContext().getRequest()).getSession(false)); + // An async request has completed, so we can complete the session, + // but we must locate the session instance for this context + Request request = Request.getBaseRequest(event.getAsyncContext().getRequest()); + HttpSession session = request.getSession(false); + String id; + if (session!=null) + id = session.getId(); + else + { + id = (String)request.getAttribute(DefaultSessionIdManager.__NEW_SESSION_ID); + if (id==null) + id = request.getRequestedSessionId(); + } + + if (id!=null) + complete(getSession(id)); } @Override @@ -412,7 +426,7 @@ public class SessionHandler extends ScopedHandler if (session == null) return; - + Session s = ((SessionIf)session).getSession(); try @@ -425,25 +439,26 @@ public class SessionHandler extends ScopedHandler LOG.warn(e); } } - - - public void complete (Session session, Request request) + + @Deprecated + public void complete(Session session, Request baseRequest) { - if (request.isAsyncStarted() && request.getDispatcherType() == DispatcherType.REQUEST) + ensureCompletion(baseRequest); + } + + private void ensureCompletion(Request baseRequest) + { + if (baseRequest.isAsyncStarted()) { if (LOG.isDebugEnabled()) - LOG.debug("Adding AsyncListener for {}", request); - request.getAsyncContext().addListener(_sessionAsyncListener); + LOG.debug("Adding AsyncListener for {}", baseRequest); + if (!baseRequest.getHttpChannelState().hasListener(_sessionAsyncListener)) + baseRequest.getAsyncContext().addListener(_sessionAsyncListener); } else { - complete(session); + complete(baseRequest.getSession(false)); } - //if dispatcher type is not async and not request, complete immediately (its a forward or an include) - - //else if dispatcher type is request and not async, complete immediately - - //else register an async callback completion listener that will complete the session } @@ -460,7 +475,6 @@ public class SessionHandler extends ScopedHandler _context=ContextHandler.getCurrentContext(); _loader=Thread.currentThread().getContextClassLoader(); - synchronized (server) { //Get a SessionDataStore and a SessionDataStore, falling back to in-memory sessions only @@ -477,7 +491,6 @@ public class SessionHandler extends ScopedHandler _sessionCache.setSessionDataStore(sds); } - if (_sessionIdManager==null) { @@ -1642,13 +1655,10 @@ public class SessionHandler extends ScopedHandler finally { //if there is a session that was created during handling this context, then complete it - HttpSession finalSession = baseRequest.getSession(false); if (LOG.isDebugEnabled()) - LOG.debug("FinalSession={}, old_session_manager={}, this={}, calling complete={}", finalSession, old_session_manager, this, (old_session_manager != this)); + LOG.debug("FinalSession={}, old_session_manager={}, this={}, calling complete={}", baseRequest.getSession(false), old_session_manager, this, (old_session_manager != this)); if (old_session_manager != this) - { - complete((Session)finalSession, baseRequest); - } + ensureCompletion(baseRequest); if (old_session_manager != null && old_session_manager != this) { diff --git a/tests/test-sessions/test-mongodb-sessions/src/test/java/org/eclipse/jetty/nosql/mongodb/AttributeNameTest.java b/tests/test-sessions/test-mongodb-sessions/src/test/java/org/eclipse/jetty/nosql/mongodb/AttributeNameTest.java index d06871b6caa..876bd7e134e 100644 --- a/tests/test-sessions/test-mongodb-sessions/src/test/java/org/eclipse/jetty/nosql/mongodb/AttributeNameTest.java +++ b/tests/test-sessions/test-mongodb-sessions/src/test/java/org/eclipse/jetty/nosql/mongodb/AttributeNameTest.java @@ -117,7 +117,7 @@ public class AttributeNameTest //Mangle the cookie, replacing Path with $Path, etc. sessionCookie = sessionCookie.replaceFirst("(\\W)(P|p)ath=","$1\\$Path="); - //Make a request to the 2nd server which will do a refresh, use TestFooServlet to ensure that the + //Make a request to the 2nd server which will do a refresh, use TestServlet to ensure that the //session attribute with dotted name is not removed Request request2 = client.newRequest("http://localhost:" + port2 + contextPath + servletMapping + "?action=get"); request2.header("Cookie", sessionCookie); diff --git a/tests/test-sessions/test-sessions-common/src/main/java/org/eclipse/jetty/server/session/TestFooServlet.java b/tests/test-sessions/test-sessions-common/src/main/java/org/eclipse/jetty/server/session/TestFooServlet.java deleted file mode 100644 index 621c95b74c4..00000000000 --- a/tests/test-sessions/test-sessions-common/src/main/java/org/eclipse/jetty/server/session/TestFooServlet.java +++ /dev/null @@ -1,77 +0,0 @@ -// -// ======================================================================== -// Copyright (c) 1995-2018 Mort Bay Consulting Pty. Ltd. -// ------------------------------------------------------------------------ -// All rights reserved. This program and the accompanying materials -// are made available under the terms of the Eclipse Public License v1.0 -// and Apache License v2.0 which accompanies this distribution. -// -// The Eclipse Public License is available at -// http://www.eclipse.org/legal/epl-v10.html -// -// The Apache License v2.0 is available at -// http://www.opensource.org/licenses/apache2.0.php -// -// You may elect to redistribute this code under either of these licenses. -// ======================================================================== -// - -package org.eclipse.jetty.server.session; - -import java.io.IOException; -import java.lang.reflect.Proxy; - -import javax.servlet.AsyncContext; -import javax.servlet.ServletException; -import javax.servlet.http.HttpServlet; -import javax.servlet.http.HttpServletRequest; -import javax.servlet.http.HttpServletResponse; -import javax.servlet.http.HttpSession; - - - -public class TestFooServlet extends HttpServlet -{ - @Override - protected void doGet(HttpServletRequest request, HttpServletResponse response) throws ServletException, IOException - { - String action = request.getParameter("action"); - - if ("create".equals(action)) - { - HttpSession session = request.getSession(true); - TestFoo testFoo = new TestFoo(); - testFoo.setInt(33); - FooInvocationHandler handler = new FooInvocationHandler(testFoo); - Foo foo = (Foo)Proxy.newProxyInstance(Thread.currentThread().getContextClassLoader(), new Class[] {Foo.class}, handler); - session.setAttribute("foo", foo); - } - else if ("test".equals(action)) - { - HttpSession session = request.getSession(false); - if (session == null) - response.sendError(500, "Session not activated"); - Foo foo = (Foo)session.getAttribute("foo"); - if (foo == null || foo.getInt() != 33) - response.sendError(500, "Foo not deserialized"); - } - else if ("async".equals(action)) - { - if (request.getAttribute("async-test") == null) - { - request.setAttribute("async-test", Boolean.TRUE); - AsyncContext acontext = request.startAsync(); - System.err.println("Starting async and dispatching"); - - acontext.dispatch(); - return; - } - else - { - HttpSession session = request.getSession(true); - System.err.println("After dispatch and finishing response"); - response.getWriter().println("OK"); - } - } - } -} diff --git a/tests/test-sessions/test-sessions-common/src/test/java/org/eclipse/jetty/server/session/AsyncTest.java b/tests/test-sessions/test-sessions-common/src/test/java/org/eclipse/jetty/server/session/AsyncTest.java index cfc683d2530..90fd07e9a2e 100644 --- a/tests/test-sessions/test-sessions-common/src/test/java/org/eclipse/jetty/server/session/AsyncTest.java +++ b/tests/test-sessions/test-sessions-common/src/test/java/org/eclipse/jetty/server/session/AsyncTest.java @@ -24,11 +24,16 @@ import static org.junit.jupiter.api.Assertions.assertFalse; import static org.junit.jupiter.api.Assertions.assertTrue; import java.io.IOException; +import java.lang.reflect.Proxy; +import javax.servlet.AsyncContext; import javax.servlet.ServletException; +import javax.servlet.ServletOutputStream; +import javax.servlet.WriteListener; import javax.servlet.http.HttpServlet; import javax.servlet.http.HttpServletRequest; import javax.servlet.http.HttpServletResponse; +import javax.servlet.http.HttpSession; import org.eclipse.jetty.client.HttpClient; import org.eclipse.jetty.client.api.ContentResponse; @@ -58,42 +63,35 @@ public class AsyncTest } } - - - /** - * Test async with a session. - * - * @throws Exception - */ + + @Test - public void testSessionWithAsync() throws Exception + public void testSessionWithAsyncDispatch() throws Exception { - - - DefaultSessionCacheFactory cacheFactory = new DefaultSessionCacheFactory(); cacheFactory.setEvictionPolicy(SessionCache.EVICT_ON_SESSION_EXIT); - SessionDataStoreFactory storeFactory = new TestSessionDataStoreFactory(); - TestServer server1 = new TestServer(0, -1, -1, cacheFactory, storeFactory); - + SessionDataStoreFactory storeFactory = new TestSessionDataStoreFactory(); + TestServer server = new TestServer(0, -1, -1, cacheFactory, storeFactory); + String contextPath = ""; - String fooMapping = "/server"; - TestFooServlet servlet = new TestFooServlet(); + String mapping = "/server"; + + ServletContextHandler contextHandler = server.addContext(contextPath); + TestServlet servlet = new TestServlet(); ServletHolder holder = new ServletHolder(servlet); - ServletContextHandler contextHandler = server1.addContext(contextPath); - contextHandler.addServlet(holder, fooMapping); + contextHandler.addServlet(holder, mapping); LatchServlet latchServlet = new LatchServlet(); ServletHolder latchHolder = new ServletHolder(latchServlet); contextHandler.addServlet(latchHolder, "/latch"); - server1.start(); - int port1 = server1.getPort(); + server.start(); + int port = server.getPort(); try (StacklessLogging stackless = new StacklessLogging(Log.getLogger("org.eclipse.jetty.server.session"))) { HttpClient client = new HttpClient(); client.start(); - String url = "http://localhost:" + port1 + contextPath + fooMapping+"?action=async"; + String url = "http://localhost:" + port + contextPath + mapping+"?action=async"; //make a request to set up a session on the server ContentResponse response = client.GET(url); @@ -101,11 +99,11 @@ public class AsyncTest String sessionCookie = response.getHeaders().get("Set-Cookie"); assertTrue(sessionCookie != null); - + //make another request, when this is handled, the first request is definitely finished being handled - response = client.GET("http://localhost:" + port1 + contextPath + "/latch"); + response = client.GET("http://localhost:" + port + contextPath + "/latch"); assertEquals(HttpServletResponse.SC_OK,response.getStatus()); - + //session should now be evicted from the cache after request exited String id = TestServer.extractSessionId(sessionCookie); assertFalse(contextHandler.getSessionHandler().getSessionCache().contains(id)); @@ -113,7 +111,243 @@ public class AsyncTest } finally { - server1.stop(); + server.stop(); + } + } + + @Test + public void testSessionWithAsyncComplete() throws Exception + { + DefaultSessionCacheFactory cacheFactory = new DefaultSessionCacheFactory(); + cacheFactory.setEvictionPolicy(SessionCache.EVICT_ON_SESSION_EXIT); + SessionDataStoreFactory storeFactory = new TestSessionDataStoreFactory(); + TestServer server = new TestServer(0, -1, -1, cacheFactory, storeFactory); + + String contextPath = ""; + String mapping = "/server"; + + ServletContextHandler contextHandler = server.addContext(contextPath); + TestServlet servlet = new TestServlet(); + ServletHolder holder = new ServletHolder(servlet); + contextHandler.addServlet(holder, mapping); + LatchServlet latchServlet = new LatchServlet(); + ServletHolder latchHolder = new ServletHolder(latchServlet); + contextHandler.addServlet(latchHolder, "/latch"); + + server.start(); + int port = server.getPort(); + + try (StacklessLogging stackless = new StacklessLogging(Log.getLogger("org.eclipse.jetty.server.session"))) + { + HttpClient client = new HttpClient(); + client.start(); + String url = "http://localhost:" + port + contextPath + mapping+"?action=asyncComplete"; + + //make a request to set up a session on the server + ContentResponse response = client.GET(url); + assertEquals(HttpServletResponse.SC_OK,response.getStatus()); + + String sessionCookie = response.getHeaders().get("Set-Cookie"); + assertTrue(sessionCookie != null); + + //make another request, when this is handled, the first request is definitely finished being handled + response = client.GET("http://localhost:" + port + contextPath + "/latch"); + assertEquals(HttpServletResponse.SC_OK,response.getStatus()); + + //session should now be evicted from the cache after request exited + String id = TestServer.extractSessionId(sessionCookie); + assertFalse(contextHandler.getSessionHandler().getSessionCache().contains(id)); + assertTrue(contextHandler.getSessionHandler().getSessionCache().getSessionDataStore().exists(id)); + } + finally + { + server.stop(); + } + } + + @Test + public void testSessionWithCrossContextAsync() throws Exception + { + DefaultSessionCacheFactory cacheFactory = new DefaultSessionCacheFactory(); + cacheFactory.setEvictionPolicy(SessionCache.EVICT_ON_SESSION_EXIT); + SessionDataStoreFactory storeFactory = new TestSessionDataStoreFactory(); + TestServer server = new TestServer(0, -1, -1, cacheFactory, storeFactory); + + ServletContextHandler contextA = server.addContext("/ctxA"); + CrossContextServlet ccServlet = new CrossContextServlet(); + ServletHolder ccHolder = new ServletHolder(ccServlet); + contextA.addServlet(ccHolder, "/*"); + + ServletContextHandler contextB = server.addContext("/ctxB"); + TestServlet testServlet = new TestServlet(); + ServletHolder testHolder = new ServletHolder(testServlet); + contextB.addServlet(testHolder, "/*"); + LatchServlet latchServlet = new LatchServlet(); + ServletHolder latchHolder = new ServletHolder(latchServlet); + contextB.addServlet(latchHolder, "/latch"); + + + server.start(); + int port = server.getPort(); + + try (StacklessLogging stackless = new StacklessLogging(Log.getLogger("org.eclipse.jetty.server.session"))) + { + HttpClient client = new HttpClient(); + client.start(); + String url = "http://localhost:" + port + "/ctxA/test?action=async"; + + //make a request to set up a session on the server + ContentResponse response = client.GET(url); + assertEquals(HttpServletResponse.SC_OK,response.getStatus()); + + String sessionCookie = response.getHeaders().get("Set-Cookie"); + assertTrue(sessionCookie != null); + + //make another request, when this is handled, the first request is definitely finished being handled + response = client.GET("http://localhost:" + port + "/ctxB/latch"); + assertEquals(HttpServletResponse.SC_OK,response.getStatus()); + + //session should now be evicted from the cache after request exited + String id = TestServer.extractSessionId(sessionCookie); + assertFalse(contextB.getSessionHandler().getSessionCache().contains(id)); + assertTrue(contextB.getSessionHandler().getSessionCache().getSessionDataStore().exists(id)); + } + finally + { + server.stop(); + } + } + + + @Test + public void testSessionWithCrossContextAsyncComplete() throws Exception + { + DefaultSessionCacheFactory cacheFactory = new DefaultSessionCacheFactory(); + cacheFactory.setEvictionPolicy(SessionCache.EVICT_ON_SESSION_EXIT); + SessionDataStoreFactory storeFactory = new TestSessionDataStoreFactory(); + TestServer server = new TestServer(0, -1, -1, cacheFactory, storeFactory); + + ServletContextHandler contextA = server.addContext("/ctxA"); + CrossContextServlet ccServlet = new CrossContextServlet(); + ServletHolder ccHolder = new ServletHolder(ccServlet); + contextA.addServlet(ccHolder, "/*"); + + ServletContextHandler contextB = server.addContext("/ctxB"); + TestServlet testServlet = new TestServlet(); + ServletHolder testHolder = new ServletHolder(testServlet); + contextB.addServlet(testHolder, "/*"); + LatchServlet latchServlet = new LatchServlet(); + ServletHolder latchHolder = new ServletHolder(latchServlet); + contextB.addServlet(latchHolder, "/latch"); + + + server.start(); + int port = server.getPort(); + + try (StacklessLogging stackless = new StacklessLogging(Log.getLogger("org.eclipse.jetty.server.session"))) + { + HttpClient client = new HttpClient(); + client.start(); + String url = "http://localhost:" + port + "/ctxA/test?action=asyncComplete"; + + //make a request to set up a session on the server + ContentResponse response = client.GET(url); + assertEquals(HttpServletResponse.SC_OK,response.getStatus()); + + String sessionCookie = response.getHeaders().get("Set-Cookie"); + assertTrue(sessionCookie != null); + + //make another request, when this is handled, the first request is definitely finished being handled + response = client.GET("http://localhost:" + port + "/ctxB/latch"); + assertEquals(HttpServletResponse.SC_OK,response.getStatus()); + + //session should now be evicted from the cache A after request exited + String id = TestServer.extractSessionId(sessionCookie); + assertFalse(contextA.getSessionHandler().getSessionCache().contains(id)); + assertTrue(contextA.getSessionHandler().getSessionCache().getSessionDataStore().exists(id)); + } + finally + { + server.stop(); + } + } + + public static class TestServlet extends HttpServlet + { + @Override + protected void doGet(HttpServletRequest request, HttpServletResponse response) throws ServletException, IOException + { + String action = request.getParameter("action"); + + if ("create".equals(action)) + { + HttpSession session = request.getSession(true); + TestFoo testFoo = new TestFoo(); + testFoo.setInt(33); + FooInvocationHandler handler = new FooInvocationHandler(testFoo); + Foo foo = (Foo)Proxy + .newProxyInstance(Thread.currentThread().getContextClassLoader(), new Class[] {Foo.class}, handler); + session.setAttribute("foo", foo); + } + else if ("test".equals(action)) + { + HttpSession session = request.getSession(false); + if (session == null) + response.sendError(500, "Session not activated"); + Foo foo = (Foo)session.getAttribute("foo"); + if (foo == null || foo.getInt() != 33) + response.sendError(500, "Foo not deserialized"); + } + else if ("async".equals(action)) + { + if (request.getAttribute("async-test") == null) + { + request.setAttribute("async-test", Boolean.TRUE); + AsyncContext acontext = request.startAsync(); + acontext.dispatch(); + return; + } + else + { + HttpSession session = request.getSession(true); + response.getWriter().println("OK"); + } + } + else if ("asyncComplete".equals(action)) + { + AsyncContext acontext = request.startAsync(); + ServletOutputStream out = response.getOutputStream(); + out.setWriteListener(new WriteListener() + { + @Override + public void onWritePossible() throws IOException + { + if (out.isReady()) + { + request.getSession(true); + out.print("OK\n"); + acontext.complete(); + } + } + + @Override + public void onError(Throwable t) + { + + } + }); + } + } + } + + public static class CrossContextServlet extends HttpServlet + { + @Override + protected void doGet(HttpServletRequest request, HttpServletResponse response) throws ServletException, IOException + { + AsyncContext acontext = request.startAsync(); + + acontext.dispatch(request.getServletContext().getContext("/ctxB"),"/test"); } } } diff --git a/tests/test-sessions/test-sessions-common/src/test/java/org/eclipse/jetty/server/session/DeleteUnloadableSessionTest.java b/tests/test-sessions/test-sessions-common/src/test/java/org/eclipse/jetty/server/session/DeleteUnloadableSessionTest.java index 1565fb3f96b..be60e5f0824 100644 --- a/tests/test-sessions/test-sessions-common/src/test/java/org/eclipse/jetty/server/session/DeleteUnloadableSessionTest.java +++ b/tests/test-sessions/test-sessions-common/src/test/java/org/eclipse/jetty/server/session/DeleteUnloadableSessionTest.java @@ -128,12 +128,7 @@ public class DeleteUnloadableSessionTest } } - - /** - * TestFooServlet - * - * - */ + public static class TestServlet extends HttpServlet { private static final long serialVersionUID = 1L; diff --git a/tests/test-sessions/test-sessions-common/src/test/java/org/eclipse/jetty/server/session/SessionEvictionFailureTest.java b/tests/test-sessions/test-sessions-common/src/test/java/org/eclipse/jetty/server/session/SessionEvictionFailureTest.java index 27687b4c82a..214d3e59985 100644 --- a/tests/test-sessions/test-sessions-common/src/test/java/org/eclipse/jetty/server/session/SessionEvictionFailureTest.java +++ b/tests/test-sessions/test-sessions-common/src/test/java/org/eclipse/jetty/server/session/SessionEvictionFailureTest.java @@ -136,16 +136,9 @@ public class SessionEvictionFailureTest } - - - /** - * TestFooServlet - * - * - */ + public static class TestServlet extends HttpServlet { - @Override protected void doGet(HttpServletRequest request, HttpServletResponse httpServletResponse) throws ServletException, IOException { From 503bd71d4cd0fdb5e45404ee2308aa4e67d57992 Mon Sep 17 00:00:00 2001 From: Greg Wilkins Date: Tue, 18 Dec 2018 16:00:46 +1100 Subject: [PATCH 11/11] Issue #3202 Handle async cross context session completion --- .../jetty/server/session/SessionHandler.java | 22 +++++++++++-------- .../jetty/server/session/AsyncTest.java | 17 +++++++++----- 2 files changed, 24 insertions(+), 15 deletions(-) diff --git a/jetty-server/src/main/java/org/eclipse/jetty/server/session/SessionHandler.java b/jetty-server/src/main/java/org/eclipse/jetty/server/session/SessionHandler.java index ac197b78999..0879636f0d8 100644 --- a/jetty-server/src/main/java/org/eclipse/jetty/server/session/SessionHandler.java +++ b/jetty-server/src/main/java/org/eclipse/jetty/server/session/SessionHandler.java @@ -1611,7 +1611,7 @@ public class SessionHandler extends ScopedHandler @Override public void doScope(String target, Request baseRequest, HttpServletRequest request, HttpServletResponse response) throws IOException, ServletException { - SessionHandler old_session_manager = null; + SessionHandler old_session_handler = null; HttpSession old_session = null; HttpSession existingSession = null; @@ -1620,10 +1620,10 @@ public class SessionHandler extends ScopedHandler if (LOG.isDebugEnabled()) LOG.debug("SessionHandler.doScope"); - old_session_manager = baseRequest.getSessionHandler(); + old_session_handler = baseRequest.getSessionHandler(); old_session = baseRequest.getSession(false); - if (old_session_manager != this) + if (old_session_handler != this) { // new session context baseRequest.setSessionHandler(this); @@ -1634,7 +1634,7 @@ public class SessionHandler extends ScopedHandler // access any existing session for this context existingSession = baseRequest.getSession(false); - if ((existingSession != null) && (old_session_manager != this)) + if ((existingSession != null) && (old_session_handler != this)) { HttpCookie cookie = access(existingSession,request.isSecure()); // Handle changed ID or max-age refresh, but only if this is not a redispatched request @@ -1656,13 +1656,17 @@ public class SessionHandler extends ScopedHandler { //if there is a session that was created during handling this context, then complete it if (LOG.isDebugEnabled()) - LOG.debug("FinalSession={}, old_session_manager={}, this={}, calling complete={}", baseRequest.getSession(false), old_session_manager, this, (old_session_manager != this)); - if (old_session_manager != this) + LOG.debug("FinalSession={}, old_session_handler={}, this={}, calling complete={}", baseRequest.getSession(false), old_session_handler, this, (old_session_handler != this)); + + // If we are leaving the scope of this session handler, ensure the session is completed + if (old_session_handler != this) ensureCompletion(baseRequest); - - if (old_session_manager != null && old_session_manager != this) + + // revert the session handler to the previous, unless it was null, in which case remember it as + // the first session handler encountered. + if (old_session_handler != null && old_session_handler != this) { - baseRequest.setSessionHandler(old_session_manager); + baseRequest.setSessionHandler(old_session_handler); baseRequest.setSession(old_session); } } diff --git a/tests/test-sessions/test-sessions-common/src/test/java/org/eclipse/jetty/server/session/AsyncTest.java b/tests/test-sessions/test-sessions-common/src/test/java/org/eclipse/jetty/server/session/AsyncTest.java index 90fd07e9a2e..2c8a105d3fc 100644 --- a/tests/test-sessions/test-sessions-common/src/test/java/org/eclipse/jetty/server/session/AsyncTest.java +++ b/tests/test-sessions/test-sessions-common/src/test/java/org/eclipse/jetty/server/session/AsyncTest.java @@ -52,22 +52,20 @@ import org.junit.jupiter.api.Test; */ public class AsyncTest { - public static class LatchServlet extends HttpServlet { - @Override protected void doGet(HttpServletRequest req, HttpServletResponse resp) throws ServletException, IOException { resp.getWriter().println("Latched"); } - } - @Test public void testSessionWithAsyncDispatch() throws Exception { + // Test async dispatch back to same context, which then creates a session. + DefaultSessionCacheFactory cacheFactory = new DefaultSessionCacheFactory(); cacheFactory.setEvictionPolicy(SessionCache.EVICT_ON_SESSION_EXIT); SessionDataStoreFactory storeFactory = new TestSessionDataStoreFactory(); @@ -118,6 +116,8 @@ public class AsyncTest @Test public void testSessionWithAsyncComplete() throws Exception { + // Test async write, which creates a session and completes outside of a dispatch + DefaultSessionCacheFactory cacheFactory = new DefaultSessionCacheFactory(); cacheFactory.setEvictionPolicy(SessionCache.EVICT_ON_SESSION_EXIT); SessionDataStoreFactory storeFactory = new TestSessionDataStoreFactory(); @@ -168,6 +168,9 @@ public class AsyncTest @Test public void testSessionWithCrossContextAsync() throws Exception { + // Test async dispatch from context A to context B then + // async dispatch back to context B, which then creates a session (in context B). + DefaultSessionCacheFactory cacheFactory = new DefaultSessionCacheFactory(); cacheFactory.setEvictionPolicy(SessionCache.EVICT_ON_SESSION_EXIT); SessionDataStoreFactory storeFactory = new TestSessionDataStoreFactory(); @@ -218,10 +221,13 @@ public class AsyncTest } } - @Test public void testSessionWithCrossContextAsyncComplete() throws Exception { + // Test async dispatch from context A to context B, which then does an + // async write, which creates a session (in context A) and completes outside of a + // dispatch + DefaultSessionCacheFactory cacheFactory = new DefaultSessionCacheFactory(); cacheFactory.setEvictionPolicy(SessionCache.EVICT_ON_SESSION_EXIT); SessionDataStoreFactory storeFactory = new TestSessionDataStoreFactory(); @@ -240,7 +246,6 @@ public class AsyncTest ServletHolder latchHolder = new ServletHolder(latchServlet); contextB.addServlet(latchHolder, "/latch"); - server.start(); int port = server.getPort();