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>""</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-unsubscr...@tomcat.apache.org For additional commands, e-mail: dev-h...@tomcat.apache.org