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 2018/02/06 12:08:26 UTC

svn commit: r1823310 - in /tomcat/trunk: java/org/apache/catalina/ java/org/apache/catalina/authenticator/ java/org/apache/catalina/core/ java/org/apache/catalina/startup/ webapps/docs/

Author: markt
Date: Tue Feb  6 12:08:26 2018
New Revision: 1823310

URL: http://svn.apache.org/viewvc?rev=1823310&view=rev
Log:
Process all ServletSecurity annotations at web application start rather
than at servlet load time to ensure constraints are applied
consistently.

Modified:
    tomcat/trunk/java/org/apache/catalina/Wrapper.java
    tomcat/trunk/java/org/apache/catalina/authenticator/AuthenticatorBase.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/java/org/apache/catalina/core/StandardWrapper.java
    tomcat/trunk/java/org/apache/catalina/startup/ContextConfig.java
    tomcat/trunk/java/org/apache/catalina/startup/Tomcat.java
    tomcat/trunk/java/org/apache/catalina/startup/WebAnnotationSet.java
    tomcat/trunk/webapps/docs/changelog.xml

Modified: tomcat/trunk/java/org/apache/catalina/Wrapper.java
URL: http://svn.apache.org/viewvc/tomcat/trunk/java/org/apache/catalina/Wrapper.java?rev=1823310&r1=1823309&r2=1823310&view=diff
==============================================================================
--- tomcat/trunk/java/org/apache/catalina/Wrapper.java (original)
+++ tomcat/trunk/java/org/apache/catalina/Wrapper.java Tue Feb  6 12:08:26 2018
@@ -368,21 +368,23 @@ public interface Wrapper extends Contain
     public void setEnabled(boolean enabled);
 
     /**
-     * Set the flag that indicates
-     * {@link javax.servlet.annotation.ServletSecurity} annotations must be
-     * scanned when the Servlet is first used.
+     * This method is no longer used. All implementations should be NO-OPs.
      *
-     * @param b The new value of the flag
+     * @param b Unused.
+     *
+     * @deprecated This will be removed in Tomcat 9.
      */
+    @Deprecated
     public void setServletSecurityAnnotationScanRequired(boolean b);
 
     /**
-     * Scan for (if necessary) and process (if found) the
-     * {@link javax.servlet.annotation.ServletSecurity} annotations for the
-     * Servlet associated with this wrapper.
+     * This method is no longer used. All implementations should be NO-OPs.
+     *
+     * @throws ServletException Never thrown
      *
-     * @throws ServletException if an annotation scanning error occurs
+     * @deprecated This will be removed in Tomcat 9.
      */
+    @Deprecated
     public void servletSecurityAnnotationScan() throws ServletException;
 
     /**

Modified: tomcat/trunk/java/org/apache/catalina/authenticator/AuthenticatorBase.java
URL: http://svn.apache.org/viewvc/tomcat/trunk/java/org/apache/catalina/authenticator/AuthenticatorBase.java?rev=1823310&r1=1823309&r2=1823310&view=diff
==============================================================================
--- tomcat/trunk/java/org/apache/catalina/authenticator/AuthenticatorBase.java (original)
+++ tomcat/trunk/java/org/apache/catalina/authenticator/AuthenticatorBase.java Tue Feb  6 12:08:26 2018
@@ -52,7 +52,6 @@ import org.apache.catalina.Realm;
 import org.apache.catalina.Session;
 import org.apache.catalina.TomcatPrincipal;
 import org.apache.catalina.Valve;
-import org.apache.catalina.Wrapper;
 import org.apache.catalina.authenticator.jaspic.CallbackHandlerImpl;
 import org.apache.catalina.authenticator.jaspic.MessageInfoImpl;
 import org.apache.catalina.connector.Request;
@@ -479,13 +478,6 @@ public abstract class AuthenticatorBase
 
         boolean authRequired = isContinuationRequired(request);
 
-        // The Servlet may specify security constraints through annotations.
-        // Ensure that they have been processed before constraints are checked
-        Wrapper wrapper = request.getWrapper();
-        if (wrapper != null) {
-            wrapper.servletSecurityAnnotationScan();
-        }
-
         Realm realm = this.context.getRealm();
         // Is this request URI subject to a security constraint?
         SecurityConstraint[] constraints = realm.findSecurityConstraints(request, this.context);

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=1823310&r1=1823309&r2=1823310&view=diff
==============================================================================
--- tomcat/trunk/java/org/apache/catalina/core/ApplicationContext.java (original)
+++ tomcat/trunk/java/org/apache/catalina/core/ApplicationContext.java Tue Feb  6 12:08:26 2018
@@ -49,8 +49,10 @@ import javax.servlet.ServletRegistration
 import javax.servlet.ServletRegistration.Dynamic;
 import javax.servlet.ServletRequestAttributeListener;
 import javax.servlet.ServletRequestListener;
+import javax.servlet.ServletSecurityElement;
 import javax.servlet.SessionCookieConfig;
 import javax.servlet.SessionTrackingMode;
+import javax.servlet.annotation.ServletSecurity;
 import javax.servlet.descriptor.JspConfigDescriptor;
 import javax.servlet.http.HttpServletMapping;
 import javax.servlet.http.HttpSessionAttributeListener;
@@ -67,6 +69,7 @@ import org.apache.catalina.WebResourceRo
 import org.apache.catalina.Wrapper;
 import org.apache.catalina.connector.Connector;
 import org.apache.catalina.mapper.MappingData;
+import org.apache.catalina.util.Introspection;
 import org.apache.catalina.util.ServerInfo;
 import org.apache.catalina.util.URLEncoder;
 import org.apache.tomcat.util.ExceptionUtils;
@@ -931,11 +934,19 @@ public class ApplicationContext implemen
             }
         }
 
+        ServletSecurity annotation = null;
         if (servlet == null) {
             wrapper.setServletClass(servletClass);
+            Class<?> clazz = Introspection.loadClass(context, servletClass);
+            if (clazz != null) {
+                annotation = clazz.getAnnotation(ServletSecurity.class);
+            }
         } else {
             wrapper.setServletClass(servlet.getClass().getName());
             wrapper.setServlet(servlet);
+            if (context.wasCreatedDynamicServlet(servlet)) {
+                annotation = servlet.getClass().getAnnotation(ServletSecurity.class);
+            }
         }
 
         if (initParams != null) {
@@ -944,7 +955,12 @@ public class ApplicationContext implemen
             }
         }
 
-        return context.dynamicServletAdded(wrapper);
+        ServletRegistration.Dynamic registration =
+                new ApplicationServletRegistration(wrapper, context);
+        if (annotation != null) {
+            registration.setServletSecurity(new ServletSecurityElement(annotation));
+        }
+        return registration;
     }
 
 

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=1823310&r1=1823309&r2=1823310&view=diff
==============================================================================
--- tomcat/trunk/java/org/apache/catalina/core/ApplicationServletRegistration.java (original)
+++ tomcat/trunk/java/org/apache/catalina/core/ApplicationServletRegistration.java Tue Feb  6 12:08:26 2018
@@ -46,6 +46,7 @@ public class ApplicationServletRegistrat
 
     private final Wrapper wrapper;
     private final Context context;
+    private ServletSecurityElement constraint;
 
     public ApplicationServletRegistration(Wrapper wrapper,
             Context context) {
@@ -160,6 +161,7 @@ public class ApplicationServletRegistrat
                     getName(), context.getName()));
         }
 
+        this.constraint = constraint;
         return context.addServletSecurity(this, constraint);
     }
 
@@ -194,6 +196,11 @@ public class ApplicationServletRegistrat
             context.addServletMappingDecoded(
                     UDecoder.URLDecode(urlPattern, StandardCharsets.UTF_8), wrapper.getName());
         }
+
+        if (constraint != null) {
+            context.addServletSecurity(this, constraint);
+        }
+
         return Collections.emptySet();
     }
 

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=1823310&r1=1823309&r2=1823310&view=diff
==============================================================================
--- tomcat/trunk/java/org/apache/catalina/core/StandardContext.java (original)
+++ tomcat/trunk/java/org/apache/catalina/core/StandardContext.java Tue Feb  6 12:08:26 2018
@@ -4343,28 +4343,36 @@ public class StandardContext extends Con
     }
 
     /**
-     * Hook to register that we need to scan for security annotations.
-     * @param wrapper   The wrapper for the Servlet that was added
-     * @return the associated registration
+     * Create a servlet registration.
+     *
+     * @param wrapper The wrapper for which the registration should be created.
+     *
+     * @return An appropriate registration
+     *
+     * @deprecated This will be removed in Tomcat 9. The registration should be
+     *             created directly.
      */
+    @Deprecated
     public ServletRegistration.Dynamic dynamicServletAdded(Wrapper wrapper) {
-        Servlet s = wrapper.getServlet();
-        if (s != null && createdServlets.contains(s)) {
-            // Mark the wrapper to indicate annotations need to be scanned
-            wrapper.setServletSecurityAnnotationScanRequired(true);
-        }
         return new ApplicationServletRegistration(wrapper, this);
     }
 
     /**
-     * Hook to track which registrations need annotation scanning
-     * @param servlet the Servlet to add
+     * Hook to track which Servlets were created via
+     * {@link ServletContext#createServlet(Class)}.
+     *
+     * @param servlet the created Servlet
      */
     public void dynamicServletCreated(Servlet servlet) {
         createdServlets.add(servlet);
     }
 
 
+    public boolean wasCreatedDynamicServlet(Servlet servlet) {
+        return createdServlets.contains(servlet);
+    }
+
+
     /**
      * A helper class to manage the filter mappings in a Context.
      */
@@ -5639,8 +5647,6 @@ public class StandardContext extends Con
                         newSecurityConstraints) {
                     addConstraint(securityConstraint);
                 }
-
-                checkConstraintsForUncoveredMethods(newSecurityConstraints);
             }
         }
 

Modified: tomcat/trunk/java/org/apache/catalina/core/StandardWrapper.java
URL: http://svn.apache.org/viewvc/tomcat/trunk/java/org/apache/catalina/core/StandardWrapper.java?rev=1823310&r1=1823309&r2=1823310&view=diff
==============================================================================
--- tomcat/trunk/java/org/apache/catalina/core/StandardWrapper.java (original)
+++ tomcat/trunk/java/org/apache/catalina/core/StandardWrapper.java Tue Feb  6 12:08:26 2018
@@ -14,8 +14,6 @@
  * See the License for the specific language governing permissions and
  * limitations under the License.
  */
-
-
 package org.apache.catalina.core;
 
 import java.io.PrintStream;
@@ -43,11 +41,9 @@ import javax.servlet.Servlet;
 import javax.servlet.ServletConfig;
 import javax.servlet.ServletContext;
 import javax.servlet.ServletException;
-import javax.servlet.ServletSecurityElement;
 import javax.servlet.SingleThreadModel;
 import javax.servlet.UnavailableException;
 import javax.servlet.annotation.MultipartConfig;
-import javax.servlet.annotation.ServletSecurity;
 
 import org.apache.catalina.Container;
 import org.apache.catalina.ContainerServlet;
@@ -257,8 +253,6 @@ public class StandardWrapper extends Con
      */
     protected boolean enabled = true;
 
-    protected volatile boolean servletSecurityAnnotationScanRequired = false;
-
     private boolean overridable = false;
 
     /**
@@ -616,7 +610,7 @@ public class StandardWrapper extends Con
      */
     @Override
     public void setServletSecurityAnnotationScanRequired(boolean b) {
-        this.servletSecurityAnnotationScanRequired = b;
+        // NO-OP
     }
 
     // --------------------------------------------------------- Public Methods
@@ -1075,8 +1069,6 @@ public class StandardWrapper extends Con
                 }
             }
 
-            processServletSecurityAnnotation(servlet.getClass());
-
             // Special handling for ContainerServlet instances
             // Note: The InstanceManager checks if the application is permitted
             //       to load ContainerServlets
@@ -1119,40 +1111,9 @@ public class StandardWrapper extends Con
      */
     @Override
     public void servletSecurityAnnotationScan() throws ServletException {
-        if (getServlet() == null) {
-            Class<?> clazz = null;
-            try {
-                clazz = ((Context) getParent()).getLoader().getClassLoader().loadClass(
-                        getServletClass());
-                processServletSecurityAnnotation(clazz);
-            } catch (ClassNotFoundException e) {
-                // Safe to ignore. No class means no annotations to process
-            }
-        } else {
-            if (servletSecurityAnnotationScanRequired) {
-                processServletSecurityAnnotation(getServlet().getClass());
-            }
-        }
+        // NO-OP
     }
 
-    private void processServletSecurityAnnotation(Class<?> clazz) {
-        // Calling this twice isn't harmful so no syncs
-        servletSecurityAnnotationScanRequired = false;
-
-        Context ctxt = (Context) getParent();
-
-        if (ctxt.getIgnoreAnnotations()) {
-            return;
-        }
-
-        ServletSecurity secAnnotation =
-            clazz.getAnnotation(ServletSecurity.class);
-        if (secAnnotation != null) {
-            ctxt.addServletSecurity(
-                    new ApplicationServletRegistration(this, ctxt),
-                    new ServletSecurityElement(secAnnotation));
-        }
-    }
 
     private synchronized void initServlet(Servlet servlet)
             throws ServletException {

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=1823310&r1=1823309&r2=1823310&view=diff
==============================================================================
--- tomcat/trunk/java/org/apache/catalina/startup/ContextConfig.java (original)
+++ tomcat/trunk/java/org/apache/catalina/startup/ContextConfig.java Tue Feb  6 12:08:26 2018
@@ -344,15 +344,14 @@ public class ContextConfig implements Li
         LoginConfig loginConfig = context.getLoginConfig();
 
         SecurityConstraint constraints[] = context.findConstraints();
-        if (context.getIgnoreAnnotations() &&
-                (constraints == null || constraints.length ==0) &&
+        if ((constraints == null || constraints.length ==0) &&
                 !context.getPreemptiveAuthentication())  {
+            // No need for an authenticator
             return;
         } else {
             if (loginConfig == null) {
-                // Not metadata-complete or security constraints present, need
-                // an authenticator to support @ServletSecurity annotations
-                // and/or constraints
+                // Security constraints present. Need an authenticator to
+                // support them.
                 loginConfig = DUMMY_LOGIN_CONFIG;
                 context.setLoginConfig(loginConfig);
             }

Modified: tomcat/trunk/java/org/apache/catalina/startup/Tomcat.java
URL: http://svn.apache.org/viewvc/tomcat/trunk/java/org/apache/catalina/startup/Tomcat.java?rev=1823310&r1=1823309&r2=1823310&view=diff
==============================================================================
--- tomcat/trunk/java/org/apache/catalina/startup/Tomcat.java (original)
+++ tomcat/trunk/java/org/apache/catalina/startup/Tomcat.java Tue Feb  6 12:08:26 2018
@@ -967,6 +967,9 @@ public class Tomcat {
                 if (event.getType().equals(Lifecycle.CONFIGURE_START_EVENT)) {
                     context.setConfigured(true);
 
+                    // Process annotations
+                    WebAnnotationSet.loadApplicationAnnotations(context);
+
                     // LoginConfig is required to process @ServletSecurity
                     // annotations
                     if (context.getLoginConfig() == null) {

Modified: tomcat/trunk/java/org/apache/catalina/startup/WebAnnotationSet.java
URL: http://svn.apache.org/viewvc/tomcat/trunk/java/org/apache/catalina/startup/WebAnnotationSet.java?rev=1823310&r1=1823309&r2=1823310&view=diff
==============================================================================
--- tomcat/trunk/java/org/apache/catalina/startup/WebAnnotationSet.java (original)
+++ tomcat/trunk/java/org/apache/catalina/startup/WebAnnotationSet.java Tue Feb  6 12:08:26 2018
@@ -23,10 +23,13 @@ import javax.annotation.Resource;
 import javax.annotation.Resources;
 import javax.annotation.security.DeclareRoles;
 import javax.annotation.security.RunAs;
+import javax.servlet.ServletSecurityElement;
+import javax.servlet.annotation.ServletSecurity;
 
 import org.apache.catalina.Container;
 import org.apache.catalina.Context;
 import org.apache.catalina.Wrapper;
+import org.apache.catalina.core.ApplicationServletRegistration;
 import org.apache.catalina.util.Introspection;
 import org.apache.tomcat.util.descriptor.web.ContextEnvironment;
 import org.apache.tomcat.util.descriptor.web.ContextResource;
@@ -136,9 +139,17 @@ public class WebAnnotationSet {
                  * Ref JSR 250, equivalent to the run-as element in
                  * the deployment descriptor
                  */
-                RunAs annotation = clazz.getAnnotation(RunAs.class);
-                if (annotation != null) {
-                    wrapper.setRunAs(annotation.value());
+                RunAs runAs = clazz.getAnnotation(RunAs.class);
+                if (runAs != null) {
+                    wrapper.setRunAs(runAs.value());
+                }
+
+                // Process ServletSecurity annotation
+                ServletSecurity servletSecurity = clazz.getAnnotation(ServletSecurity.class);
+                if (servletSecurity != null) {
+                    context.addServletSecurity(
+                            new ApplicationServletRegistration(wrapper, context),
+                            new ServletSecurityElement(servletSecurity));
                 }
             }
         }

Modified: tomcat/trunk/webapps/docs/changelog.xml
URL: http://svn.apache.org/viewvc/tomcat/trunk/webapps/docs/changelog.xml?rev=1823310&r1=1823309&r2=1823310&view=diff
==============================================================================
--- tomcat/trunk/webapps/docs/changelog.xml (original)
+++ tomcat/trunk/webapps/docs/changelog.xml Tue Feb  6 12:08:26 2018
@@ -95,6 +95,11 @@
         <bug>62067</bug>: Correctly apply security constraints mapped to the
         context root using a URL pattern of <code>&quot;&quot;</code>. (markt)
       </fix>
+      <fix>
+        Process all <code>ServletSecurity</code> annotations at web application
+        start rather than at servlet load time to ensure constraints are applied
+        consistently. (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