tomcat-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From ma...@apache.org
Subject svn commit: r1022068 - in /tomcat/trunk: java/org/apache/catalina/core/ java/org/apache/catalina/deploy/ java/org/apache/catalina/startup/ test/org/apache/catalina/core/ test/org/apache/catalina/startup/ test/webapp-3.0/WEB-INF/ webapps/docs/
Date Wed, 13 Oct 2010 11:10:30 GMT
Author: markt
Date: Wed Oct 13 11:10:30 2010
New Revision: 1022068

URL: http://svn.apache.org/viewvc?rev=1022068&view=rev
Log:
Better fix for https://issues.apache.org/bugzilla/show_bug.cgi?id=49922
Revert my approach and go with patch suggested by heyoulin.
Extend test cases

Modified:
    tomcat/trunk/java/org/apache/catalina/core/ApplicationFilterChain.java
    tomcat/trunk/java/org/apache/catalina/deploy/WebXml.java
    tomcat/trunk/java/org/apache/catalina/startup/ContextConfig.java
    tomcat/trunk/test/org/apache/catalina/core/TestStandardContext.java
    tomcat/trunk/test/org/apache/catalina/startup/TestContextConfigAnnotation.java
    tomcat/trunk/test/webapp-3.0/WEB-INF/web.xml
    tomcat/trunk/webapps/docs/changelog.xml

Modified: tomcat/trunk/java/org/apache/catalina/core/ApplicationFilterChain.java
URL: http://svn.apache.org/viewvc/tomcat/trunk/java/org/apache/catalina/core/ApplicationFilterChain.java?rev=1022068&r1=1022067&r2=1022068&view=diff
==============================================================================
--- tomcat/trunk/java/org/apache/catalina/core/ApplicationFilterChain.java (original)
+++ tomcat/trunk/java/org/apache/catalina/core/ApplicationFilterChain.java Wed Oct 13 11:10:30
2010
@@ -525,6 +525,11 @@ final class ApplicationFilterChain imple
      */
     void addFilter(ApplicationFilterConfig filterConfig) {
 
+        // Prevent the same filter being added multiple times
+        for(ApplicationFilterConfig filter:filters)
+            if(filter==filterConfig)
+                return;
+
         if (n == filters.length) {
             ApplicationFilterConfig[] newFilters =
                 new ApplicationFilterConfig[n + INCREMENT];

Modified: tomcat/trunk/java/org/apache/catalina/deploy/WebXml.java
URL: http://svn.apache.org/viewvc/tomcat/trunk/java/org/apache/catalina/deploy/WebXml.java?rev=1022068&r1=1022067&r2=1022068&view=diff
==============================================================================
--- tomcat/trunk/java/org/apache/catalina/deploy/WebXml.java (original)
+++ tomcat/trunk/java/org/apache/catalina/deploy/WebXml.java Wed Oct 13 11:10:30 2010
@@ -278,37 +278,13 @@ public class WebXml {
     public Map<String,FilterDef> getFilters() { return filters; }
     
     // filter-mapping
-    private Map<String,FilterMap> filterMaps =
-        new LinkedHashMap<String,FilterMap>();
+    private Set<FilterMap> filterMaps = new LinkedHashSet<FilterMap>();
+    private Set<String> filterMappingNames = new HashSet<String>();
     public void addFilterMapping(FilterMap filterMap) {
-        FilterMap fm = filterMaps.get(filterMap.getFilterName());
-        if (fm == null) {
-            filterMaps.put(filterMap.getFilterName(), filterMap);
-        } else {
-            for (String dispatcher : filterMap.getDispatcherNames()) {
-                fm.setDispatcher(dispatcher);
-            }
-            if (!fm.getMatchAllServletNames()) {
-                if (filterMap.getMatchAllServletNames()) {
-                    fm.addServletName("*");
-                } else {
-                    for (String servletName : filterMap.getServletNames()) {
-                        fm.addServletName(servletName);
-                    }
-                }
-            }
-            if (!fm.getMatchAllUrlPatterns()) {
-                if (filterMap.getMatchAllUrlPatterns()) {
-                    fm.addURLPattern("*");
-                } else {
-                    for (String urlPattern : filterMap.getURLPatterns()) {
-                        fm.addURLPattern(urlPattern);
-                    }
-                }
-            }
-        }
+        filterMaps.add(filterMap);
+        filterMappingNames.add(filterMap.getFilterName());
     }
-    public Map<String,FilterMap> getFilterMappings() { return filterMaps; }
+    public Set<FilterMap> getFilterMappings() { return filterMaps; }
     
     // listener
     // TODO: description (multiple with language) is ignored
@@ -651,7 +627,7 @@ public class WebXml {
         }
         sb.append('\n');
 
-        for (FilterMap filterMap : filterMaps.values()) {
+        for (FilterMap filterMap : filterMaps) {
             sb.append("  <filter-mapping>\n");
             appendElement(sb, INDENT4, "filter-name",
                     filterMap.getFilterName());
@@ -1200,7 +1176,7 @@ public class WebXml {
             }
             context.addFilterDef(filter);
         }
-        for (FilterMap filterMap : filterMaps.values()) {
+        for (FilterMap filterMap : filterMaps) {
             context.addFilterMap(filterMap);
         }
         for (JspPropertyGroup jspPropertyGroup : jspPropertyGroups) {
@@ -1442,16 +1418,17 @@ public class WebXml {
         // main web.xml override those in fragments and those in fragments
         // override mappings in annotations
         for (WebXml fragment : fragments) {
-            Iterator<String> iterFilterMaps =
-                fragment.getFilterMappings().keySet().iterator();
+            Iterator<FilterMap> iterFilterMaps =
+                fragment.getFilterMappings().iterator();
             while (iterFilterMaps.hasNext()) {
-                if (filterMaps.containsKey(iterFilterMaps.next())) {
+                FilterMap filterMap = iterFilterMaps.next();
+                if (filterMappingNames.contains(filterMap.getFilterName())) {
                     iterFilterMaps.remove();
                 }
             }
         }
         for (WebXml fragment : fragments) {
-            for (FilterMap filterMap : fragment.getFilterMappings().values()) {
+            for (FilterMap filterMap : fragment.getFilterMappings()) {
                 // Additive
                 addFilterMapping(filterMap);
             }

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=1022068&r1=1022067&r2=1022068&view=diff
==============================================================================
--- tomcat/trunk/java/org/apache/catalina/startup/ContextConfig.java (original)
+++ tomcat/trunk/java/org/apache/catalina/startup/ContextConfig.java Wed Oct 13 11:10:30 2010
@@ -33,7 +33,6 @@ import java.net.URISyntaxException;
 import java.net.URL;
 import java.net.URLConnection;
 import java.util.ArrayList;
-import java.util.Collection;
 import java.util.Enumeration;
 import java.util.HashMap;
 import java.util.HashSet;
@@ -2210,7 +2209,7 @@ public class ContextConfig
             fragment.addFilterMapping(filterMap);
         }
         if (urlPatternsSet || dispatchTypesSet) {
-            Collection<FilterMap> fmap = fragment.getFilterMappings().values();
+            Set<FilterMap> fmap = fragment.getFilterMappings();
             FilterMap descMap = null;
             for (FilterMap map : fmap) {
                 if (filterName.equals(map.getFilterName())) {

Modified: tomcat/trunk/test/org/apache/catalina/core/TestStandardContext.java
URL: http://svn.apache.org/viewvc/tomcat/trunk/test/org/apache/catalina/core/TestStandardContext.java?rev=1022068&r1=1022067&r2=1022068&view=diff
==============================================================================
--- tomcat/trunk/test/org/apache/catalina/core/TestStandardContext.java (original)
+++ tomcat/trunk/test/org/apache/catalina/core/TestStandardContext.java Wed Oct 13 11:10:30
2010
@@ -126,23 +126,43 @@ public class TestStandardContext extends
         tomcat.addWebapp("", root.getAbsolutePath());
         
         tomcat.start();
+        ByteChunk result;
 
-        // Check path mapping works
-        ByteChunk result = getUrl("http://localhost:" + getPort() +
-        "/bug49922/foo");
-        // Filter should only have been called once
-        assertEquals("Filter", result.toString());
+        // Check filter and servlet aren't called
+        result = getUrl("http://localhost:" + getPort() +
+                "/bug49922/foo");
+        assertNull(result.toString());
 
         // Check extension mapping works
+        result = getUrl("http://localhost:" + getPort() + "/foo.do");
+        assertEquals("FilterServlet", result.toString());
+
+        // Check path mapping works
+        result = getUrl("http://localhost:" + getPort() + "/bug49922/servlet");
+        assertEquals("FilterServlet", result.toString());
+
+        // Check servlet name mapping works
+        result = getUrl("http://localhost:" + getPort() + "/foo.od");
+        assertEquals("FilterServlet", result.toString());
+
+        // Check filter is only called once
+        result = getUrl("http://localhost:" + getPort() +
+                "/bug49922/servlet/foo.do");
+        assertEquals("FilterServlet", result.toString());
         result = getUrl("http://localhost:" + getPort() +
-        "/foo.do");
-        // Filter should only have been called once
-        assertEquals("Filter", result.toString());
+                "/bug49922/servlet/foo.od");
+        assertEquals("FilterServlet", result.toString());
 
+        // Check dispatcher mapping
+        result = getUrl("http://localhost:" + getPort() +
+                "/bug49922/target");
+        assertEquals("Target", result.toString());
         result = getUrl("http://localhost:" + getPort() +
-                "/bug49922/index.do");
-        // Filter should only have been called once
-        assertEquals("Filter", result.toString());
+                "/bug49922/forward");
+        assertEquals("FilterTarget", result.toString());
+        result = getUrl("http://localhost:" + getPort() +
+                "/bug49922/include");
+        assertEquals("IncludeFilterTarget", result.toString());
     }
 
     
@@ -167,6 +187,45 @@ public class TestStandardContext extends
         }
     }
     
+    public static final class Bug49922ForwardServlet extends HttpServlet {
+
+        private static final long serialVersionUID = 1L;
+
+        @Override
+        protected void doGet(HttpServletRequest req, HttpServletResponse resp)
+                throws ServletException, IOException {
+            req.getRequestDispatcher("/bug49922/target").forward(req, resp);
+        }
+        
+    }
+
+    public static final class Bug49922IncludeServlet extends HttpServlet {
+
+        private static final long serialVersionUID = 1L;
+
+        @Override
+        protected void doGet(HttpServletRequest req, HttpServletResponse resp)
+                throws ServletException, IOException {
+            resp.setContentType("text/plain");
+            resp.getWriter().print("Include");
+            req.getRequestDispatcher("/bug49922/target").include(req, resp);
+        }
+        
+    }
+
+    public static final class Bug49922TargetServlet extends HttpServlet {
+
+        private static final long serialVersionUID = 1L;
+
+        @Override
+        protected void doGet(HttpServletRequest req, HttpServletResponse resp)
+                throws ServletException, IOException {
+            resp.setContentType("text/plain");
+            resp.getWriter().print("Target");
+        }
+        
+    }
+
     public static final class Bug49922Servlet extends HttpServlet {
 
         private static final long serialVersionUID = 1L;
@@ -174,7 +233,8 @@ public class TestStandardContext extends
         @Override
         protected void doGet(HttpServletRequest req, HttpServletResponse resp)
                 throws ServletException, IOException {
-            // NOOP
+            resp.setContentType("text/plain");
+            resp.getWriter().print("Servlet");
         }
         
     }

Modified: tomcat/trunk/test/org/apache/catalina/startup/TestContextConfigAnnotation.java
URL: http://svn.apache.org/viewvc/tomcat/trunk/test/org/apache/catalina/startup/TestContextConfigAnnotation.java?rev=1022068&r1=1022067&r2=1022068&view=diff
==============================================================================
--- tomcat/trunk/test/org/apache/catalina/startup/TestContextConfigAnnotation.java (original)
+++ tomcat/trunk/test/org/apache/catalina/startup/TestContextConfigAnnotation.java Wed Oct
13 11:10:30 2010
@@ -18,7 +18,7 @@ package org.apache.catalina.startup;
 
 import java.io.File;
 import java.net.URL;
-import java.util.Collection;
+import java.util.Set;
 
 import javax.servlet.DispatcherType;
 
@@ -201,8 +201,7 @@ public class TestContextConfigAnnotation
         assertNotNull(fdef);
         assertEquals(filterDef,fdef);
         assertEquals("tomcat",fdef.getParameterMap().get("message"));
-        Collection<FilterMap> filterMappings =
-            webxml.getFilterMappings().values();
+        Set<FilterMap> filterMappings = webxml.getFilterMappings();
         assertTrue(filterMappings.contains(filterMap));
         // annotation mapping not added s. Servlet Spec 3.0 (Nov 2009)
         // 8.2.3.3.vi page 81

Modified: tomcat/trunk/test/webapp-3.0/WEB-INF/web.xml
URL: http://svn.apache.org/viewvc/tomcat/trunk/test/webapp-3.0/WEB-INF/web.xml?rev=1022068&r1=1022067&r2=1022068&view=diff
==============================================================================
--- tomcat/trunk/test/webapp-3.0/WEB-INF/web.xml (original)
+++ tomcat/trunk/test/webapp-3.0/WEB-INF/web.xml Wed Oct 13 11:10:30 2010
@@ -37,13 +37,47 @@
   </filter>
   <filter-mapping>
     <filter-name>Bug49922</filter-name>
-    <url-pattern>/bug49922/*</url-pattern>
+    <url-pattern>/bug49922/servlet/*</url-pattern>
+    <url-pattern>*.do</url-pattern>
+    <servlet-name>Bug49922</servlet-name>
   </filter-mapping>
   <filter-mapping>
     <filter-name>Bug49922</filter-name>
-    <url-pattern>*.do</url-pattern>
+    <dispatcher>FORWARD</dispatcher>
+    <dispatcher>INCLUDE</dispatcher>
+    <servlet-name>Bug49922Target</servlet-name>
   </filter-mapping>
   <servlet>
+    <servlet-name>Bug49922Forward</servlet-name>
+    <servlet-class>
+      org.apache.catalina.core.TestStandardContext$Bug49922ForwardServlet
+    </servlet-class>
+  </servlet>
+  <servlet-mapping>
+    <servlet-name>Bug49922Forward</servlet-name>
+    <url-pattern>/bug49922/forward</url-pattern>
+  </servlet-mapping>
+  <servlet>
+    <servlet-name>Bug49922Include</servlet-name>
+    <servlet-class>
+      org.apache.catalina.core.TestStandardContext$Bug49922IncludeServlet
+    </servlet-class>
+  </servlet>
+  <servlet-mapping>
+    <servlet-name>Bug49922Include</servlet-name>
+    <url-pattern>/bug49922/include</url-pattern>
+  </servlet-mapping>
+  <servlet>
+    <servlet-name>Bug49922Target</servlet-name>
+    <servlet-class>
+      org.apache.catalina.core.TestStandardContext$Bug49922TargetServlet
+    </servlet-class>
+  </servlet>
+  <servlet-mapping>
+    <servlet-name>Bug49922Target</servlet-name>
+    <url-pattern>/bug49922/target</url-pattern>
+  </servlet-mapping>
+  <servlet>
     <servlet-name>Bug49922</servlet-name>
     <servlet-class>
       org.apache.catalina.core.TestStandardContext$Bug49922Servlet
@@ -51,12 +85,16 @@
   </servlet>
   <servlet-mapping>
     <servlet-name>Bug49922</servlet-name>
-    <url-pattern>/bug49922/*</url-pattern>
+    <url-pattern>/bug49922/servlet</url-pattern>
   </servlet-mapping>
   <servlet-mapping>
     <servlet-name>Bug49922</servlet-name>
     <url-pattern>*.do</url-pattern>
   </servlet-mapping>
+  <servlet-mapping>
+    <servlet-name>Bug49922</servlet-name>
+    <url-pattern>*.od</url-pattern>
+  </servlet-mapping>
   
   <jsp-config>
     <jsp-property-group>

Modified: tomcat/trunk/webapps/docs/changelog.xml
URL: http://svn.apache.org/viewvc/tomcat/trunk/webapps/docs/changelog.xml?rev=1022068&r1=1022067&r2=1022068&view=diff
==============================================================================
--- tomcat/trunk/webapps/docs/changelog.xml (original)
+++ tomcat/trunk/webapps/docs/changelog.xml Wed Oct 13 11:10:30 2010
@@ -45,7 +45,8 @@
       </fix>
       <fix>
         <bug>49922</bug>: Don&apos;t add filter twice to filter chain if
the
-        filter matches more than one URL pattern and/or Servlet name. (markt)
+        filter matches more than one URL pattern and/or Servlet name. Patch
+        provided by heyoulin. (markt)
       </fix>
       <fix>
         <bug>49937</bug>: Use an InstanceManager when creating an AsyncListener



---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org


Mime
View raw message