Blob Blame History Raw
commit 2aac69f694d42d9219eb27018b3da0ae1bdd73ab
Author: Mark Thomas <markt@apache.org>
Date:   Tue Feb 6 12:49:49 2018 +0000

    Process all ServletSecurity annotations at web application start rather
    than at servlet load time to ensure constraints are applied
    consistently.
    
    git-svn-id: https://svn.apache.org/repos/asf/tomcat/tc7.0.x/trunk@1823322 13f79535-47bb-0310-9956-ffa450edef68

diff -up java/org/apache/catalina/authenticator/AuthenticatorBase.java.orig java/org/apache/catalina/authenticator/AuthenticatorBase.java
--- java/org/apache/catalina/authenticator/AuthenticatorBase.java.orig	2019-02-28 15:41:52.561346049 -0500
+++ java/org/apache/catalina/authenticator/AuthenticatorBase.java	2019-02-28 15:44:09.129930251 -0500
@@ -40,7 +40,6 @@ import org.apache.catalina.Manager;
 import org.apache.catalina.Realm;
 import org.apache.catalina.Session;
 import org.apache.catalina.Valve;
-import org.apache.catalina.Wrapper;
 import org.apache.catalina.connector.Request;
 import org.apache.catalina.connector.Response;
 import org.apache.catalina.deploy.LoginConfig;
@@ -488,13 +487,6 @@ public abstract class AuthenticatorBase
             }
         }
 
-        // The Servlet may specify security constraints through annotations.
-        // Ensure that they have been processed before constraints are checked
-        Wrapper wrapper = (Wrapper) request.getMappingData().wrapper;
-        if (wrapper != null) {
-            wrapper.servletSecurityAnnotationScan();
-        }
-
         Realm realm = this.context.getRealm();
         // Is this request URI subject to a security constraint?
         SecurityConstraint [] constraints
diff -up java/org/apache/catalina/core/ApplicationContext.java.orig java/org/apache/catalina/core/ApplicationContext.java
--- java/org/apache/catalina/core/ApplicationContext.java.orig	2017-03-09 08:51:39.000000000 -0500
+++ java/org/apache/catalina/core/ApplicationContext.java	2019-02-28 15:44:09.130930248 -0500
@@ -53,8 +53,10 @@ import javax.servlet.ServletException;
 import javax.servlet.ServletRegistration;
 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.HttpSessionAttributeListener;
 import javax.servlet.http.HttpSessionListener;
@@ -69,6 +71,7 @@ import org.apache.catalina.Wrapper;
 import org.apache.catalina.connector.Connector;
 import org.apache.catalina.deploy.FilterDef;
 import org.apache.catalina.util.ResourceSet;
+import org.apache.catalina.util.Introspection;
 import org.apache.catalina.util.ServerInfo;
 import org.apache.catalina.util.URLEncoder;
 import org.apache.naming.resources.DirContextURLStreamHandler;
@@ -1186,14 +1189,27 @@ 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);
+            }
         }
 
-        return context.dynamicServletAdded(wrapper);
+        ServletRegistration.Dynamic registration =
+                new ApplicationServletRegistration(wrapper, context);
+        if (annotation != null) {
+            registration.setServletSecurity(new ServletSecurityElement(annotation));
+        }
+        return registration;
     }
 
 
diff -up java/org/apache/catalina/core/ApplicationServletRegistration.java.orig java/org/apache/catalina/core/ApplicationServletRegistration.java
--- java/org/apache/catalina/core/ApplicationServletRegistration.java.orig	2019-02-28 15:41:52.568346028 -0500
+++ java/org/apache/catalina/core/ApplicationServletRegistration.java	2019-02-28 15:44:09.130930248 -0500
@@ -5,9 +5,9 @@
  * The ASF licenses this file to You under the Apache License, Version 2.0
  * (the "License"); you may not use this file except in compliance with
  * the License.  You may obtain a copy of the License at
- * 
+ *
  *      http://www.apache.org/licenses/LICENSE-2.0
- * 
+ *
  * Unless required by applicable law or agreed to in writing, software
  * distributed under the License is distributed on an "AS IS" BASIS,
  * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
@@ -41,15 +41,16 @@ public class ApplicationServletRegistrat
      */
     private static final StringManager sm =
       StringManager.getManager(Constants.Package);
-    
-    private Wrapper wrapper;
-    private Context context;
-    
+
+    private final Wrapper wrapper;
+    private final Context context;
+    private ServletSecurityElement constraint;
+
     public ApplicationServletRegistration(Wrapper wrapper,
             Context context) {
         this.wrapper = wrapper;
         this.context = context;
-        
+
     }
 
     @Override
@@ -65,9 +66,9 @@ public class ApplicationServletRegistrat
     @Override
     public Map<String, String> getInitParameters() {
         ParameterMap<String,String> result = new ParameterMap<String,String>();
-        
+
         String[] parameterNames = wrapper.findInitParameters();
-        
+
         for (String parameterName : parameterNames) {
             result.put(parameterName, wrapper.findInitParameter(parameterName));
         }
@@ -91,7 +92,7 @@ public class ApplicationServletRegistrat
         if (getInitParameter(name) != null) {
             return false;
         }
-        
+
         wrapper.addInitParameter(name, value);
 
         return true;
@@ -99,9 +100,9 @@ public class ApplicationServletRegistrat
 
     @Override
     public Set<String> setInitParameters(Map<String, String> initParameters) {
-        
+
         Set<String> conflicts = new HashSet<String>();
-        
+
         for (Map.Entry<String, String> entry : initParameters.entrySet()) {
             if (entry.getKey() == null || entry.getValue() == null) {
                 throw new IllegalArgumentException(sm.getString(
@@ -151,13 +152,14 @@ public class ApplicationServletRegistrat
                     "applicationServletRegistration.setServletSecurity.iae",
                     getName(), context.getName()));
         }
-        
+
         if (!context.getState().equals(LifecycleState.STARTING_PREP)) {
             throw new IllegalStateException(sm.getString(
                     "applicationServletRegistration.setServletSecurity.ise",
                     getName(), context.getName()));
         }
 
+        this.constraint = constraint;
         return context.addServletSecurity(this, constraint);
     }
 
@@ -167,9 +169,9 @@ public class ApplicationServletRegistrat
         if (urlPatterns == null) {
             return Collections.emptySet();
         }
-        
+
         Set<String> conflicts = new HashSet<String>();
-        
+
         for (String urlPattern : urlPatterns) {
             String wrapperName = context.findServletMapping(urlPattern);
             if (wrapperName != null) {
@@ -187,10 +189,15 @@ public class ApplicationServletRegistrat
         if (!conflicts.isEmpty()) {
             return conflicts;
         }
-        
+
         for (String urlPattern : urlPatterns) {
             context.addServletMapping(urlPattern, wrapper.getName());
         }
+
+        if (constraint != null) {
+            context.addServletSecurity(this, constraint);
+        }
+
         return Collections.emptySet();
     }
 
@@ -199,7 +206,7 @@ public class ApplicationServletRegistrat
 
         Set<String> result = new HashSet<String>();
         String servletName = wrapper.getName();
-        
+
         String[] urlPatterns = context.findServletMappings();
         for (String urlPattern : urlPatterns) {
             String name = context.findServletMapping(urlPattern);
@@ -214,5 +221,5 @@ public class ApplicationServletRegistrat
     public String getRunAsRole() {
         return wrapper.getRunAs();
     }
-    
+
 }
diff -up java/org/apache/catalina/core/StandardContext.java.orig java/org/apache/catalina/core/StandardContext.java
--- java/org/apache/catalina/core/StandardContext.java.orig	2017-03-09 08:51:39.000000000 -0500
+++ java/org/apache/catalina/core/StandardContext.java	2019-02-28 15:44:09.132930242 -0500
@@ -4813,27 +4813,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
+     * 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
+     * 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.
      */
diff -up java/org/apache/catalina/core/StandardWrapper.java.orig java/org/apache/catalina/core/StandardWrapper.java
--- java/org/apache/catalina/core/StandardWrapper.java.orig	2019-02-28 15:41:52.576346004 -0500
+++ java/org/apache/catalina/core/StandardWrapper.java	2019-02-28 15:44:09.133930239 -0500
@@ -5,17 +5,15 @@
  * The ASF licenses this file to You under the Apache License, Version 2.0
  * (the "License"); you may not use this file except in compliance with
  * the License.  You may obtain a copy of the License at
- * 
+ *
  *      http://www.apache.org/licenses/LICENSE-2.0
- * 
+ *
  * Unless required by applicable law or agreed to in writing, software
  * distributed under the License is distributed on an "AS IS" BASIS,
  * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
  * See the License for the specific language governing permissions and
  * limitations under the License.
  */
-
-
 package org.apache.catalina.core;
 
 import java.io.PrintStream;
@@ -44,11 +42,9 @@ import javax.servlet.ServletContext;
 import javax.servlet.ServletException;
 import javax.servlet.ServletRequest;
 import javax.servlet.ServletResponse;
-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;
@@ -114,12 +110,12 @@ public class StandardWrapper extends Con
      * servlet is considered permanent.
      */
     protected long available = 0L;
-    
+
     /**
-     * The broadcaster that sends j2ee notifications. 
+     * The broadcaster that sends j2ee notifications.
      */
     protected NotificationBroadcasterSupport broadcaster = null;
-    
+
     /**
      * The count of allocations that are currently active (even if they
      * are for the same instance, as will be true on a non-STM servlet).
@@ -231,12 +227,12 @@ public class StandardWrapper extends Con
      */
     protected Stack<Servlet> instancePool = null;
 
-    
+
     /**
      * Wait time for servlet unload in ms.
      */
     protected long unloadDelay = 2000;
-    
+
 
     /**
      * True if this StandardWrapper is for the JspServlet
@@ -259,12 +255,12 @@ public class StandardWrapper extends Con
     protected StandardWrapperValve swValve;
     protected long loadTime=0;
     protected int classLoadTime=0;
-    
+
     /**
      * Multipart config
      */
     protected MultipartConfigElement multipartConfigElement = null;
-    
+
     /**
      * Async support
      */
@@ -275,26 +271,24 @@ public class StandardWrapper extends Con
      */
     protected boolean enabled = true;
 
-    protected volatile boolean servletSecurityAnnotationScanRequired = false;
-
     private boolean overridable = false;
-    
+
     /**
-     * Static class array used when the SecurityManager is turned on and 
+     * Static class array used when the SecurityManager is turned on and
      * <code>Servlet.init</code> is invoked.
      */
     protected static Class<?>[] classType = new Class[]{ServletConfig.class};
-    
-    
+
+
     /**
-     * Static class array used when the SecurityManager is turned on and 
+     * Static class array used when the SecurityManager is turned on and
      * <code>Servlet.service</code>  is invoked.
-     */                                                 
+     */
     @Deprecated
     protected static Class<?>[] classTypeUsedInService = new Class[]{
                                                          ServletRequest.class,
                                                          ServletResponse.class};
-    
+
 
     private final ReentrantReadWriteLock parametersLock =
             new ReentrantReadWriteLock();
@@ -625,7 +619,7 @@ public class StandardWrapper extends Con
     public String[] getServletMethods() throws ServletException {
 
         instance = loadServlet();
-        
+
         Class<? extends Servlet> servletClazz = instance.getClass();
         if (!javax.servlet.http.HttpServlet.class.isAssignableFrom(
                                                         servletClazz)) {
@@ -665,8 +659,8 @@ public class StandardWrapper extends Con
     public Servlet getServlet() {
         return instance;
     }
-    
-    
+
+
     /**
      * Set the associated servlet instance.
      */
@@ -675,13 +669,13 @@ public class StandardWrapper extends Con
         instance = servlet;
     }
 
-    
+
     /**
      * {@inheritDoc}
      */
     @Override
     public void setServletSecurityAnnotationScanRequired(boolean b) {
-        this.servletSecurityAnnotationScanRequired = b;
+        // NO-OP
     }
 
     // --------------------------------------------------------- Public Methods
@@ -695,19 +689,19 @@ public class StandardWrapper extends Con
     @Override
     public void backgroundProcess() {
         super.backgroundProcess();
-        
+
         if (!getState().isAvailable())
             return;
-        
+
         if (getServlet() != null && (getServlet() instanceof PeriodicEventListener)) {
             ((PeriodicEventListener) getServlet()).periodicEvent();
         }
     }
-    
-    
+
+
     /**
      * Extract the root cause from a servlet exception.
-     * 
+     *
      * @param e The servlet exception
      */
     public static Throwable getRootCause(ServletException e) {
@@ -836,7 +830,7 @@ public class StandardWrapper extends Con
         }
 
         boolean newInstance = false;
-        
+
         // If not SingleThreadedModel, return the same instance every time
         if (!singleThreadModel) {
             // Load and initialize our instance if necessary
@@ -1070,7 +1064,7 @@ public class StandardWrapper extends Con
     @Override
     public synchronized void load() throws ServletException {
         instance = loadServlet();
-        
+
         if (!instanceInitialized) {
             initServlet(instance);
         }
@@ -1078,12 +1072,12 @@ public class StandardWrapper extends Con
         if (isJspServlet) {
             StringBuilder oname =
                 new StringBuilder(MBeanUtils.getDomain(getParent()));
-            
+
             oname.append(":type=JspMonitor,name=");
             oname.append(getName());
-            
+
             oname.append(getWebModuleKeyProperties());
-            
+
             try {
                 jspMonitorON = new ObjectName(oname.toString());
                 Registry.getRegistry(null, null)
@@ -1161,8 +1155,6 @@ public class StandardWrapper extends Con
                 }
             }
 
-            processServletSecurityAnnotation(servlet.getClass());
-
             // Special handling for ContainerServlet instances
             if ((servlet instanceof ContainerServlet) &&
                     (isContainerProvidedServlet(servletClass) ||
@@ -1205,44 +1197,13 @@ public class StandardWrapper extends Con
      */
     @Override
     public void servletSecurityAnnotationScan() throws ServletException {
-        if (getServlet() == null) {
-            Class<?> clazz = null;
-            try {
-                clazz = 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 {
-        
+
         if (instanceInitialized && !singleThreadModel) return;
 
         // Call the initialization method of this servlet
@@ -1454,12 +1415,12 @@ public class StandardWrapper extends Con
             if (swallowOutput) {
                 SystemLogHandler.startCapture();
             }
-    
+
             // Call the servlet destroy() method
             try {
                 instanceSupport.fireInstanceEvent
                   (InstanceEvent.BEFORE_DESTROY_EVENT, instance);
-    
+
                 if( Globals.IS_SECURITY_ENABLED) {
                     try {
                         SecurityUtil.doAsPrivilege("destroy",
@@ -1470,7 +1431,7 @@ public class StandardWrapper extends Con
                 } else {
                     instance.destroy();
                 }
-                
+
                 instanceSupport.fireInstanceEvent
                   (InstanceEvent.AFTER_DESTROY_EVENT, instance);
 
@@ -1701,7 +1662,7 @@ public class StandardWrapper extends Con
     public boolean isAsyncSupported() {
         return asyncSupported;
     }
-    
+
     @Override
     public void setAsyncSupported(boolean asyncSupported) {
         this.asyncSupported = asyncSupported;
@@ -1711,7 +1672,7 @@ public class StandardWrapper extends Con
     public boolean isEnabled() {
         return enabled;
     }
-    
+
     @Override
     public void setEnabled(boolean enabled) {
         this.enabled = enabled;
@@ -1787,24 +1748,24 @@ public class StandardWrapper extends Con
      */
     @Override
     protected synchronized void startInternal() throws LifecycleException {
-    
-        // Send j2ee.state.starting notification 
+
+        // Send j2ee.state.starting notification
         if (this.getObjectName() != null) {
-            Notification notification = new Notification("j2ee.state.starting", 
-                                                        this.getObjectName(), 
+            Notification notification = new Notification("j2ee.state.starting",
+                                                        this.getObjectName(),
                                                         sequenceNumber++);
             broadcaster.sendNotification(notification);
         }
-        
+
         // Start up this component
         super.startInternal();
 
         setAvailable(0L);
 
-        // Send j2ee.state.running notification 
+        // Send j2ee.state.running notification
         if (this.getObjectName() != null) {
-            Notification notification = 
-                new Notification("j2ee.state.running", this.getObjectName(), 
+            Notification notification =
+                new Notification("j2ee.state.running", this.getObjectName(),
                                 sequenceNumber++);
             broadcaster.sendNotification(notification);
         }
@@ -1823,15 +1784,15 @@ public class StandardWrapper extends Con
     protected synchronized void stopInternal() throws LifecycleException {
 
         setAvailable(Long.MAX_VALUE);
-        
-        // Send j2ee.state.stopping notification 
+
+        // Send j2ee.state.stopping notification
         if (this.getObjectName() != null) {
-            Notification notification = 
-                new Notification("j2ee.state.stopping", this.getObjectName(), 
+            Notification notification =
+                new Notification("j2ee.state.stopping", this.getObjectName(),
                                 sequenceNumber++);
             broadcaster.sendNotification(notification);
         }
-        
+
         // Shut down our servlet instance (if it has been initialized)
         try {
             unload();
@@ -1843,43 +1804,43 @@ public class StandardWrapper extends Con
         // Shut down this component
         super.stopInternal();
 
-        // Send j2ee.state.stopped notification 
+        // Send j2ee.state.stopped notification
         if (this.getObjectName() != null) {
-            Notification notification = 
-                new Notification("j2ee.state.stopped", this.getObjectName(), 
+            Notification notification =
+                new Notification("j2ee.state.stopped", this.getObjectName(),
                                 sequenceNumber++);
             broadcaster.sendNotification(notification);
         }
-        
-        // Send j2ee.object.deleted notification 
-        Notification notification = 
-            new Notification("j2ee.object.deleted", this.getObjectName(), 
+
+        // Send j2ee.object.deleted notification
+        Notification notification =
+            new Notification("j2ee.object.deleted", this.getObjectName(),
                             sequenceNumber++);
         broadcaster.sendNotification(notification);
 
     }
 
-    
+
     @Override
     protected String getObjectNameKeyProperties() {
 
         StringBuilder keyProperties =
             new StringBuilder("j2eeType=Servlet,name=");
-        
+
         String name = getName();
         if (Util.objectNameValueNeedsQuote(name)) {
             name = ObjectName.quote(name);
         }
         keyProperties.append(name);
-        
+
         keyProperties.append(getWebModuleKeyProperties());
 
         return keyProperties.toString();
     }
-        
+
 
     private String getWebModuleKeyProperties() {
-        
+
         StringBuilder keyProperties = new StringBuilder(",WebModule=//");
         String hostName = getParent().getParent().getName();
         if (hostName == null) {
@@ -1887,7 +1848,7 @@ public class StandardWrapper extends Con
         } else {
             keyProperties.append(hostName);
         }
-        
+
         String contextName = ((Context) getParent()).getName();
         if (!contextName.startsWith("/")) {
             keyProperties.append('/');
@@ -1898,7 +1859,7 @@ public class StandardWrapper extends Con
         if (parent instanceof StandardContext) {
             ctx = (StandardContext) getParent();
         }
-        
+
         keyProperties.append(",J2EEApplication=");
         if (ctx == null) {
             keyProperties.append("none");
@@ -1911,10 +1872,10 @@ public class StandardWrapper extends Con
         } else {
             keyProperties.append(ctx.getJ2EEServer());
         }
-        
+
         return keyProperties.toString();
     }
-    
+
 
     /**
      * JSR 77. Always return false.
@@ -1922,19 +1883,19 @@ public class StandardWrapper extends Con
     public boolean isStateManageable() {
         return false;
     }
-    
 
-    /* Remove a JMX notificationListener 
+
+    /* Remove a JMX notificationListener
      * @see javax.management.NotificationEmitter#removeNotificationListener(javax.management.NotificationListener, javax.management.NotificationFilter, java.lang.Object)
      */
     @Override
-    public void removeNotificationListener(NotificationListener listener, 
+    public void removeNotificationListener(NotificationListener listener,
             NotificationFilter filter, Object object) throws ListenerNotFoundException {
         broadcaster.removeNotificationListener(listener,filter,object);
     }
-    
+
     protected MBeanNotificationInfo[] notificationInfo;
-    
+
     /* Get JMX Broadcaster Info
      * @TODO use StringManager for international support!
      * @TODO This two events we not send j2ee.state.failed and j2ee.attribute.changed!
@@ -1949,7 +1910,7 @@ public class StandardWrapper extends Con
                     "j2ee.object.created"},
                     Notification.class.getName(),
                     "servlet is created"
-                    ), 
+                    ),
                     new MBeanNotificationInfo(new String[] {
                     "j2ee.state.starting"},
                     Notification.class.getName(),
@@ -1980,37 +1941,37 @@ public class StandardWrapper extends Con
 
         return notificationInfo;
     }
-    
-    
+
+
     /* Add a JMX-NotificationListener
      * @see javax.management.NotificationBroadcaster#addNotificationListener(javax.management.NotificationListener, javax.management.NotificationFilter, java.lang.Object)
      */
     @Override
-    public void addNotificationListener(NotificationListener listener, 
+    public void addNotificationListener(NotificationListener listener,
             NotificationFilter filter, Object object) throws IllegalArgumentException {
         broadcaster.addNotificationListener(listener,filter,object);
     }
-    
-    
+
+
     /**
-     * Remove a JMX-NotificationListener 
+     * Remove a JMX-NotificationListener
      * @see javax.management.NotificationBroadcaster#removeNotificationListener(javax.management.NotificationListener)
      */
     @Override
-    public void removeNotificationListener(NotificationListener listener) 
+    public void removeNotificationListener(NotificationListener listener)
         throws ListenerNotFoundException {
         broadcaster.removeNotificationListener(listener);
     }
-    
-    
+
+
      // ------------------------------------------------------------- Attributes
-        
-        
+
+
     @Deprecated
     public boolean isEventProvider() {
         return false;
     }
-    
+
     @Deprecated
     public boolean isStatisticsProvider() {
         return false;
diff -up java/org/apache/catalina/startup/ContextConfig.java.orig java/org/apache/catalina/startup/ContextConfig.java
--- java/org/apache/catalina/startup/ContextConfig.java.orig	2019-02-28 15:41:52.580345992 -0500
+++ java/org/apache/catalina/startup/ContextConfig.java	2019-02-28 15:44:09.134930236 -0500
@@ -433,15 +433,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);
             }
diff -up java/org/apache/catalina/startup/Tomcat.java.orig java/org/apache/catalina/startup/Tomcat.java
--- java/org/apache/catalina/startup/Tomcat.java.orig	2019-02-28 15:44:09.135930233 -0500
+++ java/org/apache/catalina/startup/Tomcat.java	2019-02-28 15:47:52.376250650 -0500
@@ -928,6 +928,9 @@ public class Tomcat {
                 Context context = (Context) event.getLifecycle();
                 if (event.getType().equals(Lifecycle.CONFIGURE_START_EVENT)) {
                     context.setConfigured(true);
+
+                    // Process annotations
+                    WebAnnotationSet.loadApplicationAnnotations(context);
                 }
                 // LoginConfig is required to process @ServletSecurity
                 // annotations
diff -up java/org/apache/catalina/startup/WebAnnotationSet.java.orig java/org/apache/catalina/startup/WebAnnotationSet.java
--- java/org/apache/catalina/startup/WebAnnotationSet.java.orig	2017-03-09 08:51:40.000000000 -0500
+++ java/org/apache/catalina/startup/WebAnnotationSet.java	2019-02-28 15:47:07.249388035 -0500
@@ -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.deploy.ContextEnvironment;
 import org.apache.catalina.deploy.ContextResource;
 import org.apache.catalina.deploy.ContextResourceEnvRef;
@@ -143,9 +146,17 @@ public class WebAnnotationSet {
                  * Ref JSR 250, equivalent to the run-as element in
                  * the deployment descriptor
                  */
-                RunAs annotation = classClass.getAnnotation(RunAs.class);
-                if (annotation != null) {
-                    wrapper.setRunAs(annotation.value());
+                RunAs runAs = classClass.getAnnotation(RunAs.class);
+                if (runAs != null) {
+                    wrapper.setRunAs(runAs.value());
+                }
+
+                // Process ServletSecurity annotation
+                ServletSecurity servletSecurity = classClass.getAnnotation(ServletSecurity.class);
+                if (servletSecurity != null) {
+                    context.addServletSecurity(
+                            new ApplicationServletRegistration(wrapper, context),
+                            new ServletSecurityElement(servletSecurity));
                 }
             }
         }
diff -up java/org/apache/catalina/Wrapper.java.orig java/org/apache/catalina/Wrapper.java
--- java/org/apache/catalina/Wrapper.java.orig	2019-02-28 15:41:52.554346071 -0500
+++ java/org/apache/catalina/Wrapper.java	2019-02-28 15:44:09.129930251 -0500
@@ -370,19 +370,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
+     *
+     * @deprecated This will be removed in Tomcat 9.
      */
+    @Deprecated
     public void servletSecurityAnnotationScan() throws ServletException;
 
     /**
diff -up webapps/docs/changelog.xml.orig webapps/docs/changelog.xml
--- webapps/docs/changelog.xml.orig	2019-02-28 15:41:52.591345958 -0500
+++ webapps/docs/changelog.xml	2019-02-28 15:44:09.142930212 -0500
@@ -68,6 +68,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>
 </section>
commit c63b96d72cd39287e17b2ba698f4eee0ba508073
Author: Mark Thomas <markt@apache.org>
Date:   Thu Feb 15 20:21:56 2018 +0000

    Fix https://bz.apache.org/bugzilla/show_bug.cgi?id=62104
    Fix programmatic login regression as the NonLoginAuthenticator has to be set for it to work (if no login method is specified).
    
    git-svn-id: https://svn.apache.org/repos/asf/tomcat/tc7.0.x/trunk@1824360 13f79535-47bb-0310-9956-ffa450edef68

diff --git a/java/org/apache/catalina/startup/ContextConfig.java b/java/org/apache/catalina/startup/ContextConfig.java
index d913b0e489..2d8aae0ff1 100644
--- java/org/apache/catalina/startup/ContextConfig.java
+++ java/org/apache/catalina/startup/ContextConfig.java
@@ -431,19 +431,10 @@ public class ContextConfig implements LifecycleListener {
     protected void authenticatorConfig() {
 
         LoginConfig loginConfig = context.getLoginConfig();
-
-        SecurityConstraint constraints[] = context.findConstraints();
-        if ((constraints == null || constraints.length ==0) &&
-                !context.getPreemptiveAuthentication())  {
-            // No need for an authenticator
-            return;
-        } else {
-            if (loginConfig == null) {
-                // Security constraints present. Need an authenticator to
-                // support them.
-                loginConfig = DUMMY_LOGIN_CONFIG;
-                context.setLoginConfig(loginConfig);
-            }
+        if (loginConfig == null) {
+            // Need an authenticator to support HttpServletRequest.login()
+            loginConfig = DUMMY_LOGIN_CONFIG;
+            context.setLoginConfig(loginConfig);
         }
 
         // Has an authenticator been configured already?
diff -up webapps/docs/changelog.xml.orig webapps/docs/changelog.xml
--- webapps/docs/changelog.xml.orig	2019-02-28 16:12:24.743968359 -0500
+++ webapps/docs/changelog.xml	2019-02-28 16:12:39.412925943 -0500
@@ -73,6 +73,11 @@
         start rather than at servlet load time to ensure constraints are applied
         consistently. (markt)
       </fix>
+      <fix>
+        <bug>62104</bug>: Fix programmatic login regression as the
+        NonLoginAuthenticator has to be set for it to work (if no login method
+        is specified). (remm)
+      </fix>
     </changelog>
   </subsection>
 </section>