From: markt <markt@13f79535-47bb-0310-9956-ffa450edef68>
Date: Sun, 3 Oct 2010 17:27:42 +0000 (+0000)
Subject: Fix https://issues.apache.org/bugzilla/show_bug.cgi?id=49922
X-Git-Url: https://git.internetallee.de/?a=commitdiff_plain;h=52e6e42fe62b2c673d2faa2eadc87b2ccceb5afd;p=tomcat7.0

Fix https://issues.apache.org/bugzilla/show_bug.cgi?id=49922
Don't map filter twice if filter matches multiple mapping.

git-svn-id: https://svn.apache.org/repos/asf/tomcat/trunk@1004006 13f79535-47bb-0310-9956-ffa450edef68
---

diff --git a/java/org/apache/catalina/deploy/FilterMap.java b/java/org/apache/catalina/deploy/FilterMap.java
index ad9898774..48755ace6 100644
--- a/java/org/apache/catalina/deploy/FilterMap.java
+++ b/java/org/apache/catalina/deploy/FilterMap.java
@@ -80,7 +80,11 @@ public class FilterMap implements Serializable {
     private String[] servletNames = new String[0];
 
     public String[] getServletNames() {
-        return (this.servletNames);
+        if (matchAllServletNames) {
+            return new String[] {};
+        } else {
+            return (this.servletNames);
+        }
     }
 
     public void addServletName(String servletName) {
@@ -121,7 +125,11 @@ public class FilterMap implements Serializable {
     private String[] urlPatterns = new String[0];
 
     public String[] getURLPatterns() {
-        return (this.urlPatterns);
+        if (matchAllUrlPatterns) {
+            return new String[] {};
+        } else {
+            return (this.urlPatterns);
+        }
     }
 
     public void addURLPattern(String urlPattern) {
diff --git a/java/org/apache/catalina/deploy/WebXml.java b/java/org/apache/catalina/deploy/WebXml.java
index 230e9fc80..a0b79fa1c 100644
--- a/java/org/apache/catalina/deploy/WebXml.java
+++ b/java/org/apache/catalina/deploy/WebXml.java
@@ -278,13 +278,37 @@ public class WebXml {
     public Map<String,FilterDef> getFilters() { return filters; }
     
     // filter-mapping
-    private Set<FilterMap> filterMaps = new LinkedHashSet<FilterMap>();
-    private Set<String> filterMappingNames = new HashSet<String>();
+    private Map<String,FilterMap> filterMaps =
+        new LinkedHashMap<String,FilterMap>();
     public void addFilterMapping(FilterMap filterMap) {
-        filterMaps.add(filterMap);
-        filterMappingNames.add(filterMap.getFilterName());
+        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);
+                    }
+                }
+            }
+        }
     }
-    public Set<FilterMap> getFilterMappings() { return filterMaps; }
+    public Map<String,FilterMap> getFilterMappings() { return filterMaps; }
     
     // listener
     // TODO: description (multiple with language) is ignored
@@ -627,7 +651,7 @@ public class WebXml {
         }
         sb.append('\n');
 
-        for (FilterMap filterMap : filterMaps) {
+        for (FilterMap filterMap : filterMaps.values()) {
             sb.append("  <filter-mapping>\n");
             appendElement(sb, INDENT4, "filter-name",
                     filterMap.getFilterName());
@@ -1176,7 +1200,7 @@ public class WebXml {
             }
             context.addFilterDef(filter);
         }
-        for (FilterMap filterMap : filterMaps) {
+        for (FilterMap filterMap : filterMaps.values()) {
             context.addFilterMap(filterMap);
         }
         for (JspPropertyGroup jspPropertyGroup : jspPropertyGroups) {
@@ -1418,17 +1442,16 @@ public class WebXml {
         // main web.xml override those in fragments and those in fragments
         // override mappings in annotations
         for (WebXml fragment : fragments) {
-            Iterator<FilterMap> iterFilterMaps =
-                fragment.getFilterMappings().iterator();
+            Iterator<String> iterFilterMaps =
+                fragment.getFilterMappings().keySet().iterator();
             while (iterFilterMaps.hasNext()) {
-                FilterMap filterMap = iterFilterMaps.next();
-                if (filterMappingNames.contains(filterMap.getFilterName())) {
+                if (filterMaps.containsKey(iterFilterMaps.next())) {
                     iterFilterMaps.remove();
                 }
             }
         }
         for (WebXml fragment : fragments) {
-            for (FilterMap filterMap : fragment.getFilterMappings()) {
+            for (FilterMap filterMap : fragment.getFilterMappings().values()) {
                 // Additive
                 addFilterMapping(filterMap);
             }
diff --git a/java/org/apache/catalina/startup/ContextConfig.java b/java/org/apache/catalina/startup/ContextConfig.java
index dbf5efee6..f9ab9a4af 100644
--- a/java/org/apache/catalina/startup/ContextConfig.java
+++ b/java/org/apache/catalina/startup/ContextConfig.java
@@ -33,6 +33,7 @@ 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;
@@ -2192,7 +2193,7 @@ public class ContextConfig
             fragment.addFilterMapping(filterMap);
         }
         if (urlPatternsSet || dispatchTypesSet) {
-            Set<FilterMap> fmap = fragment.getFilterMappings();
+            Collection<FilterMap> fmap = fragment.getFilterMappings().values();
             FilterMap descMap = null;
             for (FilterMap map : fmap) {
                 if (filterName.equals(map.getFilterName())) {
diff --git a/test/org/apache/catalina/core/TestStandardContext.java b/test/org/apache/catalina/core/TestStandardContext.java
index eefe83ce3..6ea70a148 100644
--- a/test/org/apache/catalina/core/TestStandardContext.java
+++ b/test/org/apache/catalina/core/TestStandardContext.java
@@ -26,6 +26,9 @@ import javax.servlet.FilterConfig;
 import javax.servlet.ServletException;
 import javax.servlet.ServletRequest;
 import javax.servlet.ServletResponse;
+import javax.servlet.http.HttpServlet;
+import javax.servlet.http.HttpServletRequest;
+import javax.servlet.http.HttpServletResponse;
 
 import org.apache.catalina.Context;
 import org.apache.catalina.deploy.FilterDef;
@@ -33,6 +36,7 @@ import org.apache.catalina.deploy.FilterMap;
 import org.apache.catalina.startup.SimpleHttpClient;
 import org.apache.catalina.startup.Tomcat;
 import org.apache.catalina.startup.TomcatBaseTest;
+import org.apache.tomcat.util.buf.ByteChunk;
 
 public class TestStandardContext extends TomcatBaseTest {
 
@@ -109,4 +113,69 @@ public class TestStandardContext extends TomcatBaseTest {
         }
         
     }
+
+
+    public void testBug49922() throws Exception {
+        
+        // Set up a container
+        Tomcat tomcat = getTomcatInstance();
+        
+        // Must have a real docBase - just use temp
+        // Use the normal Tomcat ROOT context
+        File root = new File("test/webapp-3.0");
+        tomcat.addWebapp("", root.getAbsolutePath());
+        
+        tomcat.start();
+
+        // Check path mapping works
+        ByteChunk result = getUrl("http://localhost:" + getPort() +
+        "/bug49922/foo");
+        // Filter should only have been called once
+        assertEquals("Filter", result.toString());
+
+        // Check extension mapping works
+        result = getUrl("http://localhost:" + getPort() +
+        "/foo.do");
+        // Filter should only have been called once
+        assertEquals("Filter", result.toString());
+
+        result = getUrl("http://localhost:" + getPort() +
+                "/bug49922/index.do");
+        // Filter should only have been called once
+        assertEquals("Filter", result.toString());
+    }
+
+    
+    public static final class Bug49922Filter implements Filter {
+        
+        @Override
+        public void destroy() {
+            // NOOP
+        }
+
+        @Override
+        public void doFilter(ServletRequest request, ServletResponse response,
+                FilterChain chain) throws IOException, ServletException {
+            response.setContentType("text/plain");
+            response.getWriter().print("Filter");
+            chain.doFilter(request, response);
+        }
+
+        @Override
+        public void init(FilterConfig filterConfig) throws ServletException {
+            // NOOP
+        }
+    }
+    
+    public static final class Bug49922Servlet extends HttpServlet {
+
+        private static final long serialVersionUID = 1L;
+
+        @Override
+        protected void doGet(HttpServletRequest req, HttpServletResponse resp)
+                throws ServletException, IOException {
+            // NOOP
+        }
+        
+    }
 }
diff --git a/test/org/apache/catalina/startup/TestContextConfigAnnotation.java b/test/org/apache/catalina/startup/TestContextConfigAnnotation.java
index cabf5e77e..bfc53df64 100644
--- a/test/org/apache/catalina/startup/TestContextConfigAnnotation.java
+++ b/test/org/apache/catalina/startup/TestContextConfigAnnotation.java
@@ -18,7 +18,7 @@ package org.apache.catalina.startup;
 
 import java.io.File;
 import java.net.URL;
-import java.util.Set;
+import java.util.Collection;
 
 import javax.servlet.DispatcherType;
 
@@ -201,7 +201,8 @@ public class TestContextConfigAnnotation extends TestCase {
         assertNotNull(fdef);
         assertEquals(filterDef,fdef);
         assertEquals("tomcat",fdef.getParameterMap().get("message"));
-        Set<FilterMap> filterMappings = webxml.getFilterMappings();
+        Collection<FilterMap> filterMappings =
+            webxml.getFilterMappings().values();
         assertTrue(filterMappings.contains(filterMap));
         // annotation mapping not added s. Servlet Spec 3.0 (Nov 2009)
         // 8.2.3.3.vi page 81
diff --git a/test/webapp-3.0/WEB-INF/web.xml b/test/webapp-3.0/WEB-INF/web.xml
index ac2852fb1..9d975fd8b 100644
--- a/test/webapp-3.0/WEB-INF/web.xml
+++ b/test/webapp-3.0/WEB-INF/web.xml
@@ -27,6 +27,37 @@
      Used as part of the Tomcat unit tests when a full web application is
      required.
   </description>
+  
+  <!--  Bug 49922 -->
+  <filter>
+    <filter-name>Bug49922</filter-name>
+    <filter-class>
+      org.apache.catalina.core.TestStandardContext$Bug49922Filter
+    </filter-class>
+  </filter>
+  <filter-mapping>
+    <filter-name>Bug49922</filter-name>
+    <url-pattern>/bug49922/*</url-pattern>
+  </filter-mapping>
+  <filter-mapping>
+    <filter-name>Bug49922</filter-name>
+    <url-pattern>*.do</url-pattern>
+  </filter-mapping>
+  <servlet>
+    <servlet-name>Bug49922</servlet-name>
+    <servlet-class>
+      org.apache.catalina.core.TestStandardContext$Bug49922Servlet
+    </servlet-class>
+  </servlet>
+  <servlet-mapping>
+    <servlet-name>Bug49922</servlet-name>
+    <url-pattern>/bug49922/*</url-pattern>
+  </servlet-mapping>
+  <servlet-mapping>
+    <servlet-name>Bug49922</servlet-name>
+    <url-pattern>*.do</url-pattern>
+  </servlet-mapping>
+  
   <jsp-config>
     <jsp-property-group>
       <default-content-type>text/plain</default-content-type>