Author: jboynes Date: Sat Jul 20 20:22:41 2013 New Revision: 1505199 URL: http://svn.apache.org/r1505199 Log: Fix for https://issues.apache.org/bugzilla/show_bug.cgi?id=55285 Have JspConfigDescriptor related classes clone collections when returning them to application code. As ContextConfig can no longer mutate the config stored in StandardContext, add a setter to Context to provide a fully configured JspConfigDescriptor
Added: tomcat/trunk/test/org/apache/tomcat/util/descriptor/web/TestJspConfigDescriptorImpl.java (with props) tomcat/trunk/test/org/apache/tomcat/util/descriptor/web/TestJspPropertyGroupDescriptorImpl.java (with props) Modified: tomcat/trunk/java/org/apache/catalina/Context.java tomcat/trunk/java/org/apache/catalina/core/ApplicationContext.java tomcat/trunk/java/org/apache/catalina/core/StandardContext.java tomcat/trunk/java/org/apache/catalina/startup/ContextConfig.java tomcat/trunk/java/org/apache/catalina/startup/FailedContext.java tomcat/trunk/java/org/apache/catalina/startup/TldConfig.java tomcat/trunk/java/org/apache/tomcat/util/descriptor/web/JspConfigDescriptorImpl.java tomcat/trunk/java/org/apache/tomcat/util/descriptor/web/JspPropertyGroupDescriptorImpl.java tomcat/trunk/java/org/apache/tomcat/util/descriptor/web/WebXml.java tomcat/trunk/test/org/apache/catalina/core/TestApplicationContext.java tomcat/trunk/test/org/apache/catalina/core/TesterContext.java Modified: tomcat/trunk/java/org/apache/catalina/Context.java URL: http://svn.apache.org/viewvc/tomcat/trunk/java/org/apache/catalina/Context.java?rev=1505199&r1=1505198&r2=1505199&view=diff ============================================================================== --- tomcat/trunk/java/org/apache/catalina/Context.java (original) +++ tomcat/trunk/java/org/apache/catalina/Context.java Sat Jul 20 20:22:41 2013 @@ -1277,9 +1277,15 @@ public interface Context extends Contain /** * Obtain the JSP configuration for this context. + * Will be null if there is no JSP configuration. */ public JspConfigDescriptor getJspConfigDescriptor(); + /** + * Set the JspConfigDescriptor for this context. + * A null value indicates there is not JSP configuration. + */ + public void setJspConfigDescriptor(JspConfigDescriptor descriptor); /** * Add a ServletContainerInitializer instance to this web application. Modified: tomcat/trunk/java/org/apache/catalina/core/ApplicationContext.java URL: http://svn.apache.org/viewvc/tomcat/trunk/java/org/apache/catalina/core/ApplicationContext.java?rev=1505199&r1=1505198&r2=1505199&view=diff ============================================================================== --- tomcat/trunk/java/org/apache/catalina/core/ApplicationContext.java (original) +++ tomcat/trunk/java/org/apache/catalina/core/ApplicationContext.java Sat Jul 20 20:22:41 2013 @@ -1399,14 +1399,7 @@ public class ApplicationContext @Override public JspConfigDescriptor getJspConfigDescriptor() { - JspConfigDescriptor jspConfigDescriptor = context - .getJspConfigDescriptor(); - if (jspConfigDescriptor.getJspPropertyGroups().isEmpty() - && jspConfigDescriptor.getTaglibs().isEmpty()) { - return null; - } else { - return jspConfigDescriptor; - } + return context.getJspConfigDescriptor(); } Modified: tomcat/trunk/java/org/apache/catalina/core/StandardContext.java URL: http://svn.apache.org/viewvc/tomcat/trunk/java/org/apache/catalina/core/StandardContext.java?rev=1505199&r1=1505198&r2=1505199&view=diff ============================================================================== --- tomcat/trunk/java/org/apache/catalina/core/StandardContext.java (original) +++ tomcat/trunk/java/org/apache/catalina/core/StandardContext.java Sat Jul 20 20:22:41 2013 @@ -821,8 +821,7 @@ public class StandardContext extends Con private int effectiveMinorVersion = 0; - private JspConfigDescriptor jspConfigDescriptor = - new JspConfigDescriptorImpl(); + private JspConfigDescriptor jspConfigDescriptor = null; private Set<String> resourceOnlyServlets = new HashSet<>(); @@ -2514,6 +2513,10 @@ public class StandardContext extends Con return jspConfigDescriptor; } + @Override + public void setJspConfigDescriptor(JspConfigDescriptor descriptor) { + this.jspConfigDescriptor = descriptor; + } // ------------------------------------------------------ Public Properties @@ -5697,7 +5700,7 @@ public class StandardContext extends Con applicationListeners = new ApplicationListener[0]; applicationEventListenersObjects = new Object[0]; applicationLifecycleListenersObjects = new Object[0]; - jspConfigDescriptor = new JspConfigDescriptorImpl(); + jspConfigDescriptor = null; initializers.clear(); Modified: tomcat/trunk/java/org/apache/catalina/startup/ContextConfig.java URL: http://svn.apache.org/viewvc/tomcat/trunk/java/org/apache/catalina/startup/ContextConfig.java?rev=1505199&r1=1505198&r2=1505199&view=diff ============================================================================== --- tomcat/trunk/java/org/apache/catalina/startup/ContextConfig.java (original) +++ tomcat/trunk/java/org/apache/catalina/startup/ContextConfig.java Sat Jul 20 20:22:41 2013 @@ -1275,13 +1275,7 @@ public class ContextConfig implements Li for (FilterMap filterMap : webxml.getFilterMappings()) { context.addFilterMap(filterMap); } - for (JspPropertyGroup jspPropertyGroup : - webxml.getJspPropertyGroups()) { - JspPropertyGroupDescriptor descriptor = - new JspPropertyGroupDescriptorImpl(jspPropertyGroup); - context.getJspConfigDescriptor().getJspPropertyGroups().add( - descriptor); - } + context.setJspConfigDescriptor(webxml.getJspConfigDescriptor()); for (String listener : webxml.getListeners()) { context.addApplicationListener( new ApplicationListener(listener, false)); @@ -1409,11 +1403,6 @@ public class ContextConfig implements Li sessionConfig.getSessionTrackingModes()); } } - for (Entry<String, String> entry : webxml.getTaglibs().entrySet()) { - TaglibDescriptor descriptor = new TaglibDescriptorImpl( - entry.getValue(), entry.getKey()); - context.getJspConfigDescriptor().getTaglibs().add(descriptor); - } // Context doesn't use version directly Modified: tomcat/trunk/java/org/apache/catalina/startup/FailedContext.java URL: http://svn.apache.org/viewvc/tomcat/trunk/java/org/apache/catalina/startup/FailedContext.java?rev=1505199&r1=1505198&r2=1505199&view=diff ============================================================================== --- tomcat/trunk/java/org/apache/catalina/startup/FailedContext.java (original) +++ tomcat/trunk/java/org/apache/catalina/startup/FailedContext.java Sat Jul 20 20:22:41 2013 @@ -640,6 +640,9 @@ public class FailedContext extends Lifec public JspConfigDescriptor getJspConfigDescriptor() { return null; } @Override + public void setJspConfigDescriptor(JspConfigDescriptor descriptor) { /* NO-OP */ } + + @Override public void addServletContainerInitializer(ServletContainerInitializer sci, Set<Class<?>> classes) { /* NO-OP */ } Modified: tomcat/trunk/java/org/apache/catalina/startup/TldConfig.java URL: http://svn.apache.org/viewvc/tomcat/trunk/java/org/apache/catalina/startup/TldConfig.java?rev=1505199&r1=1505198&r2=1505199&view=diff ============================================================================== --- tomcat/trunk/java/org/apache/catalina/startup/TldConfig.java (original) +++ tomcat/trunk/java/org/apache/catalina/startup/TldConfig.java Sat Jul 20 20:22:41 2013 @@ -28,6 +28,7 @@ import java.util.Iterator; import java.util.Set; import javax.servlet.ServletContext; +import javax.servlet.descriptor.JspConfigDescriptor; import javax.servlet.descriptor.TaglibDescriptor; import org.apache.catalina.Context; @@ -284,9 +285,12 @@ public final class TldConfig implements log.trace(sm.getString("tldConfig.webxmlStart")); } - Collection<TaglibDescriptor> descriptors = - context.getJspConfigDescriptor().getTaglibs(); + JspConfigDescriptor jspConfigDescriptor = context.getJspConfigDescriptor(); + if (jspConfigDescriptor == null) { + return; + } + Collection<TaglibDescriptor> descriptors = jspConfigDescriptor.getTaglibs(); for (TaglibDescriptor descriptor : descriptors) { String resourcePath = descriptor.getTaglibLocation(); // Note: Whilst the Servlet 2.4 DTD implies that the location must Modified: tomcat/trunk/java/org/apache/tomcat/util/descriptor/web/JspConfigDescriptorImpl.java URL: http://svn.apache.org/viewvc/tomcat/trunk/java/org/apache/tomcat/util/descriptor/web/JspConfigDescriptorImpl.java?rev=1505199&r1=1505198&r2=1505199&view=diff ============================================================================== --- tomcat/trunk/java/org/apache/tomcat/util/descriptor/web/JspConfigDescriptorImpl.java (original) +++ tomcat/trunk/java/org/apache/tomcat/util/descriptor/web/JspConfigDescriptorImpl.java Sat Jul 20 20:22:41 2013 @@ -17,29 +17,30 @@ package org.apache.tomcat.util.descriptor.web; +import java.util.ArrayList; import java.util.Collection; -import java.util.HashSet; -import java.util.LinkedHashSet; - import javax.servlet.descriptor.JspConfigDescriptor; import javax.servlet.descriptor.JspPropertyGroupDescriptor; import javax.servlet.descriptor.TaglibDescriptor; public class JspConfigDescriptorImpl implements JspConfigDescriptor { - private final Collection<JspPropertyGroupDescriptor> jspPropertyGroups = - new LinkedHashSet<>(); + private final Collection<JspPropertyGroupDescriptor> jspPropertyGroups; + private final Collection<TaglibDescriptor> taglibs; - private final Collection<TaglibDescriptor> taglibs = new HashSet<>(); + public JspConfigDescriptorImpl(Collection<JspPropertyGroupDescriptor> jspPropertyGroups, + Collection<TaglibDescriptor> taglibs) { + this.jspPropertyGroups = jspPropertyGroups; + this.taglibs = taglibs; + } @Override public Collection<JspPropertyGroupDescriptor> getJspPropertyGroups() { - return jspPropertyGroups; + return new ArrayList<>(jspPropertyGroups); } @Override public Collection<TaglibDescriptor> getTaglibs() { - return taglibs; + return new ArrayList<>(taglibs); } - } Modified: tomcat/trunk/java/org/apache/tomcat/util/descriptor/web/JspPropertyGroupDescriptorImpl.java URL: http://svn.apache.org/viewvc/tomcat/trunk/java/org/apache/tomcat/util/descriptor/web/JspPropertyGroupDescriptorImpl.java?rev=1505199&r1=1505198&r2=1505199&view=diff ============================================================================== --- tomcat/trunk/java/org/apache/tomcat/util/descriptor/web/JspPropertyGroupDescriptorImpl.java (original) +++ tomcat/trunk/java/org/apache/tomcat/util/descriptor/web/JspPropertyGroupDescriptorImpl.java Sat Jul 20 20:22:41 2013 @@ -16,6 +16,7 @@ */ package org.apache.tomcat.util.descriptor.web; +import java.util.ArrayList; import java.util.Collection; import javax.servlet.descriptor.JspPropertyGroupDescriptor; @@ -97,13 +98,13 @@ public class JspPropertyGroupDescriptorI @Override public Collection<String> getIncludeCodas() { - return jspPropertyGroup.getIncludeCodas(); + return new ArrayList<>(jspPropertyGroup.getIncludeCodas()); } @Override public Collection<String> getIncludePreludes() { - return jspPropertyGroup.getIncludePreludes(); + return new ArrayList<>(jspPropertyGroup.getIncludePreludes()); } @@ -157,6 +158,6 @@ public class JspPropertyGroupDescriptorI @Override public Collection<String> getUrlPatterns() { - return jspPropertyGroup.getUrlPatterns(); + return new ArrayList<>(jspPropertyGroup.getUrlPatterns()); } } Modified: tomcat/trunk/java/org/apache/tomcat/util/descriptor/web/WebXml.java URL: http://svn.apache.org/viewvc/tomcat/trunk/java/org/apache/tomcat/util/descriptor/web/WebXml.java?rev=1505199&r1=1505198&r2=1505199&view=diff ============================================================================== --- tomcat/trunk/java/org/apache/tomcat/util/descriptor/web/WebXml.java (original) +++ tomcat/trunk/java/org/apache/tomcat/util/descriptor/web/WebXml.java Sat Jul 20 20:22:41 2013 @@ -18,6 +18,7 @@ package org.apache.tomcat.util.descripto import java.net.URL; import java.util.ArrayList; +import java.util.Collection; import java.util.EnumSet; import java.util.HashMap; import java.util.HashSet; @@ -564,19 +565,23 @@ public class WebXml { if (jspPropertyGroups.isEmpty() && taglibs.isEmpty()) { return null; } - JspConfigDescriptorImpl jspConfigDescriptor = new JspConfigDescriptorImpl(); + + Collection<JspPropertyGroupDescriptor> descriptors = + new ArrayList<>(jspPropertyGroups.size()); for (JspPropertyGroup jspPropertyGroup : jspPropertyGroups) { JspPropertyGroupDescriptor descriptor = new JspPropertyGroupDescriptorImpl(jspPropertyGroup); - jspConfigDescriptor.getJspPropertyGroups().add(descriptor); + descriptors.add(descriptor); } + + Collection<TaglibDescriptor> tlds = new HashSet<>(taglibs.size()); for (Entry<String, String> entry : taglibs.entrySet()) { TaglibDescriptor descriptor = new TaglibDescriptorImpl( entry.getValue(), entry.getKey()); - jspConfigDescriptor.getTaglibs().add(descriptor); + tlds.add(descriptor); } - return jspConfigDescriptor; + return new JspConfigDescriptorImpl(descriptors, tlds); } // Attributes not defined in web.xml or web-fragment.xml Modified: tomcat/trunk/test/org/apache/catalina/core/TestApplicationContext.java URL: http://svn.apache.org/viewvc/tomcat/trunk/test/org/apache/catalina/core/TestApplicationContext.java?rev=1505199&r1=1505198&r2=1505199&view=diff ============================================================================== --- tomcat/trunk/test/org/apache/catalina/core/TestApplicationContext.java (original) +++ tomcat/trunk/test/org/apache/catalina/core/TestApplicationContext.java Sat Jul 20 20:22:41 2013 @@ -121,7 +121,6 @@ public class TestApplicationContext exte } @Test - @Ignore("Bug 55285") public void testJspPropertyGroupsAreIsolated() throws Exception { Tomcat tomcat = getTomcatInstance(); Modified: tomcat/trunk/test/org/apache/catalina/core/TesterContext.java URL: http://svn.apache.org/viewvc/tomcat/trunk/test/org/apache/catalina/core/TesterContext.java?rev=1505199&r1=1505198&r2=1505199&view=diff ============================================================================== --- tomcat/trunk/test/org/apache/catalina/core/TesterContext.java (original) +++ tomcat/trunk/test/org/apache/catalina/core/TesterContext.java Sat Jul 20 20:22:41 2013 @@ -1011,6 +1011,11 @@ public class TesterContext implements Co } @Override + public void setJspConfigDescriptor(JspConfigDescriptor descriptor) { + // NO-OP + } + + @Override public void addServletContainerInitializer(ServletContainerInitializer sci, Set<Class<?>> classes) { // NO-OP Added: tomcat/trunk/test/org/apache/tomcat/util/descriptor/web/TestJspConfigDescriptorImpl.java URL: http://svn.apache.org/viewvc/tomcat/trunk/test/org/apache/tomcat/util/descriptor/web/TestJspConfigDescriptorImpl.java?rev=1505199&view=auto ============================================================================== --- tomcat/trunk/test/org/apache/tomcat/util/descriptor/web/TestJspConfigDescriptorImpl.java (added) +++ tomcat/trunk/test/org/apache/tomcat/util/descriptor/web/TestJspConfigDescriptorImpl.java Sat Jul 20 20:22:41 2013 @@ -0,0 +1,50 @@ +/* + * 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. + */ +package org.apache.tomcat.util.descriptor.web; + +import java.util.ArrayList; +import java.util.Collections; +import java.util.List; +import javax.servlet.descriptor.JspConfigDescriptor; +import javax.servlet.descriptor.JspPropertyGroupDescriptor; +import javax.servlet.descriptor.TaglibDescriptor; + +import org.junit.Assert; +import org.junit.Test; + +public class TestJspConfigDescriptorImpl { + + @Test + public void testTaglibsAreIsolate() { + List<TaglibDescriptor> taglibs = new ArrayList<>(); + taglibs.add(new TaglibDescriptorImpl("location", "uri")); + List<JspPropertyGroupDescriptor> propertyGroups = Collections.emptyList(); + JspConfigDescriptor descriptor = new JspConfigDescriptorImpl(propertyGroups, taglibs); + descriptor.getTaglibs().clear(); + Assert.assertEquals(taglibs, descriptor.getTaglibs()); + } + + @Test + public void testPropertyGroupsAreIsolate() { + List<TaglibDescriptor> taglibs = Collections.emptyList(); + List<JspPropertyGroupDescriptor> propertyGroups = new ArrayList<>(); + propertyGroups.add(new JspPropertyGroupDescriptorImpl(new JspPropertyGroup())); + JspConfigDescriptor descriptor = new JspConfigDescriptorImpl(propertyGroups, taglibs); + descriptor.getJspPropertyGroups().clear(); + Assert.assertEquals(propertyGroups, descriptor.getJspPropertyGroups()); + } +} Propchange: tomcat/trunk/test/org/apache/tomcat/util/descriptor/web/TestJspConfigDescriptorImpl.java ------------------------------------------------------------------------------ svn:eol-style = native Added: tomcat/trunk/test/org/apache/tomcat/util/descriptor/web/TestJspPropertyGroupDescriptorImpl.java URL: http://svn.apache.org/viewvc/tomcat/trunk/test/org/apache/tomcat/util/descriptor/web/TestJspPropertyGroupDescriptorImpl.java?rev=1505199&view=auto ============================================================================== --- tomcat/trunk/test/org/apache/tomcat/util/descriptor/web/TestJspPropertyGroupDescriptorImpl.java (added) +++ tomcat/trunk/test/org/apache/tomcat/util/descriptor/web/TestJspPropertyGroupDescriptorImpl.java Sat Jul 20 20:22:41 2013 @@ -0,0 +1,50 @@ +/* + * 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. + */ +package org.apache.tomcat.util.descriptor.web; + +import org.junit.Assert; +import org.junit.Test; + +public class TestJspPropertyGroupDescriptorImpl { + + @Test + public void testPreludesAreIsolated() { + JspPropertyGroup jpg = new JspPropertyGroup(); + jpg.addIncludePrelude("prelude"); + JspPropertyGroupDescriptorImpl descriptor = new JspPropertyGroupDescriptorImpl(jpg); + descriptor.getIncludePreludes().clear(); + Assert.assertEquals(1, descriptor.getIncludePreludes().size()); + } + + @Test + public void testCodasAreIsolated() { + JspPropertyGroup jpg = new JspPropertyGroup(); + jpg.addIncludeCoda("coda"); + JspPropertyGroupDescriptorImpl descriptor = new JspPropertyGroupDescriptorImpl(jpg); + descriptor.getIncludeCodas().clear(); + Assert.assertEquals(1, descriptor.getIncludeCodas().size()); + } + + @Test + public void testUrlPatternsAreIsolated() { + JspPropertyGroup jpg = new JspPropertyGroup(); + jpg.addUrlPattern("pattern"); + JspPropertyGroupDescriptorImpl descriptor = new JspPropertyGroupDescriptorImpl(jpg); + descriptor.getUrlPatterns().clear(); + Assert.assertEquals(1, descriptor.getUrlPatterns().size()); + } +} Propchange: tomcat/trunk/test/org/apache/tomcat/util/descriptor/web/TestJspPropertyGroupDescriptorImpl.java ------------------------------------------------------------------------------ svn:eol-style = native --------------------------------------------------------------------- To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org For additional commands, e-mail: dev-h...@tomcat.apache.org