You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@tomcat.apache.org by ma...@apache.org on 2010/10/13 13:10:30 UTC
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/
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'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