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/08 15:22:04 UTC

svn commit: r1005811 - in /tomcat/trunk: java/org/apache/catalina/ java/org/apache/catalina/core/ webapps/docs/

Author: markt
Date: Fri Oct  8 13:22:04 2010
New Revision: 1005811

URL: http://svn.apache.org/viewvc?rev=1005811&view=rev
Log:
Fix https://issues.apache.org/bugzilla/show_bug.cgi?id=50015
Re-factor dynamic servlet security implementation to make extensions, such as JACC implementations, simpler. Patch provided by David Jencks.

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/ApplicationServletRegistration.java
    tomcat/trunk/java/org/apache/catalina/core/StandardContext.java
    tomcat/trunk/webapps/docs/changelog.xml

Modified: tomcat/trunk/java/org/apache/catalina/Context.java
URL: http://svn.apache.org/viewvc/tomcat/trunk/java/org/apache/catalina/Context.java?rev=1005811&r1=1005810&r2=1005811&view=diff
==============================================================================
--- tomcat/trunk/java/org/apache/catalina/Context.java (original)
+++ tomcat/trunk/java/org/apache/catalina/Context.java Fri Oct  8 13:22:04 2010
@@ -24,8 +24,10 @@ import java.util.Set;
 
 import javax.servlet.ServletContainerInitializer;
 import javax.servlet.ServletContext;
+import javax.servlet.ServletSecurityElement;
 import javax.servlet.descriptor.JspConfigDescriptor;
 
+import org.apache.catalina.core.ApplicationServletRegistration;
 import org.apache.catalina.deploy.ApplicationParameter;
 import org.apache.catalina.deploy.ErrorPage;
 import org.apache.catalina.deploy.FilterDef;
@@ -1222,5 +1224,16 @@ public interface Context extends Contain
      * Is this context using version 2.2 of the Servlet spec?
      */
     boolean isServlet22();
+
+    /**
+     * Notification that servlet security has been dynamically set in a
+     * {@Link ServletRegistration.Dynamic}
+     * @param registration servlet security was modified for
+     * @param servletSecurityElement new security constraints for this servlet
+     * @return urls currently mapped to this registration that are already
+     *         present in web.xml
+     */
+    Set<String> addServletSecurity(ApplicationServletRegistration registration,
+            ServletSecurityElement servletSecurityElement);
 }
 

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=1005811&r1=1005810&r2=1005811&view=diff
==============================================================================
--- tomcat/trunk/java/org/apache/catalina/core/ApplicationContext.java (original)
+++ tomcat/trunk/java/org/apache/catalina/core/ApplicationContext.java Fri Oct  8 13:22:04 2010
@@ -1082,10 +1082,10 @@ public class ApplicationContext
         } else {
             wrapper.setServletClass(servlet.getClass().getName());
             wrapper.setServlet(servlet);
-       }
-        
-        return new ApplicationServletRegistration(wrapper, context);
-    } 
+        }
+
+        return context.dynamicServletAdded(wrapper);
+    }
 
 
     public <T extends Servlet> T createServlet(Class<T> c)
@@ -1093,6 +1093,7 @@ public class ApplicationContext
         try {
             @SuppressWarnings("unchecked")
             T servlet = (T) context.getInstanceManager().newInstance(c.getName());
+            context.dynamicServletCreated(servlet);
             return servlet;
         } catch (IllegalAccessException e) {
             throw new ServletException(e);

Modified: tomcat/trunk/java/org/apache/catalina/core/ApplicationServletRegistration.java
URL: http://svn.apache.org/viewvc/tomcat/trunk/java/org/apache/catalina/core/ApplicationServletRegistration.java?rev=1005811&r1=1005810&r2=1005811&view=diff
==============================================================================
--- tomcat/trunk/java/org/apache/catalina/core/ApplicationServletRegistration.java (original)
+++ tomcat/trunk/java/org/apache/catalina/core/ApplicationServletRegistration.java Fri Oct  8 13:22:04 2010
@@ -117,10 +117,12 @@ public class ApplicationServletRegistrat
 
         // Have to add in a separate loop since spec requires no updates at all
         // if there is an issue
-        for (Map.Entry<String, String> entry : initParameters.entrySet()) {
-            setInitParameter(entry.getKey(), entry.getValue());
+        if (conflicts.isEmpty()) {
+            for (Map.Entry<String, String> entry : initParameters.entrySet()) {
+                setInitParameter(entry.getKey(), entry.getValue());
+            }
         }
-        
+
         return conflicts;
     }
 
@@ -158,53 +160,7 @@ public class ApplicationServletRegistrat
                     getName(), context.getPath()));
         }
 
-        Set<String> conflicts = new HashSet<String>();
-
-        Collection<String> urlPatterns = getMappings();
-        for (String urlPattern : urlPatterns) {
-            boolean foundConflict = false;
-            
-            SecurityConstraint[] securityConstraints =
-                context.findConstraints();
-            for (SecurityConstraint securityConstraint : securityConstraints) {
-                
-                SecurityCollection[] collections =
-                    securityConstraint.findCollections();
-                for (SecurityCollection collection : collections) {
-                    if (collection.findPattern(urlPattern)) {
-                        // First pattern found will indicate if there is a
-                        // conflict since for any given pattern all matching
-                        // constraints will be from either the descriptor or
-                        // not. It is not permitted to have a mixture
-                        if (collection.isFromDescriptor()) {
-                            // Skip this pattern
-                            foundConflict = true;
-                        } else {
-                            // Need to overwrite constraint for this pattern
-                            // so remove every pattern found
-                            context.removeConstraint(securityConstraint);
-                        }
-                    }
-                    if (foundConflict) {
-                        break;
-                    }
-                }
-                if (foundConflict) {
-                    break;
-                }
-            }
-            if (!foundConflict) {
-                SecurityConstraint[] newSecurityConstraints =
-                        SecurityConstraint.createConstraints(constraint,
-                                urlPattern);
-                for (SecurityConstraint securityConstraint :
-                        newSecurityConstraints) {
-                    context.addConstraint(securityConstraint);
-                }
-            }
-        }
-        
-        return conflicts;
+        return context.addServletSecurity(this, constraint);
     }
 
 

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=1005811&r1=1005810&r2=1005811&view=diff
==============================================================================
--- tomcat/trunk/java/org/apache/catalina/core/StandardContext.java (original)
+++ tomcat/trunk/java/org/apache/catalina/core/StandardContext.java Fri Oct  8 13:22:04 2010
@@ -26,7 +26,9 @@ import java.io.InputStreamReader;
 import java.net.URL;
 import java.util.ArrayList;
 import java.util.Arrays;
+import java.util.Collection;
 import java.util.HashMap;
+import java.util.HashSet;
 import java.util.Hashtable;
 import java.util.Iterator;
 import java.util.LinkedHashMap;
@@ -47,14 +49,17 @@ import javax.management.ObjectName;
 import javax.naming.NamingException;
 import javax.naming.directory.DirContext;
 import javax.servlet.FilterConfig;
+import javax.servlet.Servlet;
 import javax.servlet.ServletContainerInitializer;
 import javax.servlet.ServletContext;
 import javax.servlet.ServletContextAttributeListener;
 import javax.servlet.ServletContextEvent;
 import javax.servlet.ServletContextListener;
 import javax.servlet.ServletException;
+import javax.servlet.ServletRegistration;
 import javax.servlet.ServletRequestAttributeListener;
 import javax.servlet.ServletRequestListener;
+import javax.servlet.ServletSecurityElement;
 import javax.servlet.descriptor.JspConfigDescriptor;
 import javax.servlet.http.HttpSessionAttributeListener;
 import javax.servlet.http.HttpSessionListener;
@@ -4050,6 +4055,22 @@ public class StandardContext extends Con
         return null;
     }
 
+    /**
+     * hook to register that we need to scan for security annotations.
+     * @param registration
+     */
+    public ServletRegistration.Dynamic dynamicServletAdded(Wrapper wrapper) {
+        return new ApplicationServletRegistration(wrapper, this);
+    }
+
+    /**
+     * hook to track which registrations need annotation scanning
+     * @param servlet
+     */
+    public void dynamicServletCreated(Servlet servlet) {
+        // NOOP - Hook for JACC implementations
+    }
+
 
     /**
      * A helper class to manage the filter mappings in a Context.
@@ -5173,6 +5194,71 @@ public class StandardContext extends Con
 
     }
 
+    public Set<String> addServletSecurity(
+            ApplicationServletRegistration registration,
+            ServletSecurityElement servletSecurityElement) {
+
+        Set<String> conflicts = new HashSet<String>();
+
+        Collection<String> urlPatterns = registration.getMappings();
+        for (String urlPattern : urlPatterns) {
+            boolean foundConflict = false;
+
+            SecurityConstraint[] securityConstraints =
+                findConstraints();
+            for (SecurityConstraint securityConstraint : securityConstraints) {
+
+                SecurityCollection[] collections =
+                    securityConstraint.findCollections();
+                for (SecurityCollection collection : collections) {
+                    if (collection.findPattern(urlPattern)) {
+                        // First pattern found will indicate if there is a
+                        // conflict since for any given pattern all matching
+                        // constraints will be from either the descriptor or
+                        // not. It is not permitted to have a mixture
+                        if (collection.isFromDescriptor()) {
+                            // Skip this pattern
+                            foundConflict = true;
+                            conflicts.add(urlPattern);
+                        } else {
+                            // Need to overwrite constraint for this pattern
+                            // so remove every pattern found
+
+                            // TODO spec 13.4.2 appears to say only the
+                            // conflicting pattern is overwritten, not the
+                            // entire security constraint.
+                            removeConstraint(securityConstraint);
+                        }
+                    }
+                    if (foundConflict) {
+                        break;
+                    }
+                }
+                if (foundConflict) {
+                    break;
+                }
+            }
+            // TODO spec 13.4.2 appears to say that non-conflicting patterns are
+            // still used.
+            // TODO you can't calculate the eventual security constraint now,
+            // you have to wait until the context is started, since application
+            // code can add url patterns after calling setSecurity.
+            if (!foundConflict) {
+                SecurityConstraint[] newSecurityConstraints =
+                        SecurityConstraint.createConstraints(
+                                servletSecurityElement,
+                                urlPattern);
+                for (SecurityConstraint securityConstraint :
+                        newSecurityConstraints) {
+                    addConstraint(securityConstraint);
+                }
+            }
+        }
+
+        return conflicts;
+
+    }
+
 
     /**
      * Return a File object representing the base directory for the

Modified: tomcat/trunk/webapps/docs/changelog.xml
URL: http://svn.apache.org/viewvc/tomcat/trunk/webapps/docs/changelog.xml?rev=1005811&r1=1005810&r2=1005811&view=diff
==============================================================================
--- tomcat/trunk/webapps/docs/changelog.xml (original)
+++ tomcat/trunk/webapps/docs/changelog.xml Fri Oct  8 13:22:04 2010
@@ -89,6 +89,11 @@
         <bug>49994</bug>: As per the Java EE 6 specification, return a new
         object instance for each JNDI look up of a resource reference. (markt) 
       </fix>
+      <fix>
+        <bug>50015</bug>: Re-factor dynamic servlet security implementation to
+        make extensions, such as JACC implementations, simpler. Patch provided
+        by David Jencks. (markt) 
+      </fix>
     </changelog>
   </subsection>
   <subsection name="Coyote">



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