Skip to content

Commit

Permalink
Process all ServletSecurity annotations at web application start rather
Browse files Browse the repository at this point in the history
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
  • Loading branch information
markt-asf committed Feb 6, 2018
1 parent 15d7f0e commit 2aac69f
Show file tree
Hide file tree
Showing 10 changed files with 182 additions and 175 deletions.
18 changes: 11 additions & 7 deletions java/org/apache/catalina/Wrapper.java
Original file line number Diff line number Diff line change
Expand Up @@ -370,19 +370,23 @@ public void setMultipartConfigElement(
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;

/**
Expand Down
8 changes: 0 additions & 8 deletions java/org/apache/catalina/authenticator/AuthenticatorBase.java
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,6 @@
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;
Expand Down Expand Up @@ -488,13 +487,6 @@ public void invoke(Request request, Response response)
}
}

// 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
Expand Down
18 changes: 17 additions & 1 deletion java/org/apache/catalina/core/ApplicationContext.java
Original file line number Diff line number Diff line change
Expand Up @@ -53,8 +53,10 @@
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;
Expand All @@ -69,6 +71,7 @@
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;
Expand Down Expand Up @@ -1192,14 +1195,27 @@ private ServletRegistration.Dynamic addServlet(String servletName,
}
}

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;
}


Expand Down
43 changes: 25 additions & 18 deletions java/org/apache/catalina/core/ApplicationServletRegistration.java
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down Expand Up @@ -41,15 +41,16 @@ public class ApplicationServletRegistration
*/
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
Expand All @@ -65,9 +66,9 @@ public String getInitParameter(String name) {
@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));
}
Expand All @@ -91,17 +92,17 @@ public boolean setInitParameter(String name, String value) {
if (getInitParameter(name) != null) {
return false;
}

wrapper.addInitParameter(name, value);

return true;
}

@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(
Expand Down Expand Up @@ -151,13 +152,14 @@ public Set<String> setServletSecurity(ServletSecurityElement constraint) {
"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);
}

Expand All @@ -167,9 +169,9 @@ public Set<String> addMapping(String... urlPatterns) {
if (urlPatterns == null) {
return Collections.emptySet();
}

Set<String> conflicts = new HashSet<String>();

for (String urlPattern : urlPatterns) {
String wrapperName = context.findServletMapping(urlPattern);
if (wrapperName != null) {
Expand All @@ -187,10 +189,15 @@ public Set<String> addMapping(String... urlPatterns) {
if (!conflicts.isEmpty()) {
return conflicts;
}

for (String urlPattern : urlPatterns) {
context.addServletMapping(urlPattern, wrapper.getName());
}

if (constraint != null) {
context.addServletSecurity(this, constraint);
}

return Collections.emptySet();
}

Expand All @@ -199,7 +206,7 @@ public Collection<String> getMappings() {

Set<String> result = new HashSet<String>();
String servletName = wrapper.getName();

String[] urlPatterns = context.findServletMappings();
for (String urlPattern : urlPatterns) {
String name = context.findServletMapping(urlPattern);
Expand All @@ -214,5 +221,5 @@ public Collection<String> getMappings() {
public String getRunAsRole() {
return wrapper.getRunAs();
}

}
27 changes: 18 additions & 9 deletions java/org/apache/catalina/core/StandardContext.java
Original file line number Diff line number Diff line change
Expand Up @@ -4804,27 +4804,36 @@ public String getRealPath(String path) {
}

/**
* 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.
*/
Expand Down
Loading

0 comments on commit 2aac69f

Please sign in to comment.