Author: markt
Date: Fri Mar 4 15:17:22 2011
New Revision: 1077995
URL: http://svn.apache.org/viewvc?rev=1077995&view=rev
Log:
@ServletSecurity
Servlets added via addServlet() should not be processed unless created via
craeteServlet. Need to delay scanning until urlPatterns are known
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/StandardContext.java
tomcat/trunk/java/org/apache/catalina/core/StandardWrapper.java
tomcat/trunk/test/org/apache/catalina/core/TestStandardWrapper.java
Modified: tomcat/trunk/java/org/apache/catalina/Wrapper.java
URL:
http://svn.apache.org/viewvc/tomcat/trunk/java/org/apache/catalina/Wrapper.java?rev=1077995&r1=1077994&r2=1077995&view=diff
==============================================================================
--- tomcat/trunk/java/org/apache/catalina/Wrapper.java (original)
+++ tomcat/trunk/java/org/apache/catalina/Wrapper.java Fri Mar 4 15:17:22 2011
@@ -371,4 +371,19 @@ 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.
+ *
+ * @param b The new value of the flag
+ */
+ 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.
+ */
+ 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=1077995&r1=1077994&r2=1077995&view=diff
==============================================================================
--- tomcat/trunk/java/org/apache/catalina/authenticator/AuthenticatorBase.java
(original)
+++ tomcat/trunk/java/org/apache/catalina/authenticator/AuthenticatorBase.java
Fri Mar 4 15:17:22 2011
@@ -482,9 +482,7 @@ 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.getServlet() == null) {
- wrapper.load();
- }
+ wrapper.servletSecurityAnnotationScan();
Realm realm = this.context.getRealm();
// Is this request URI subject to a security 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=1077995&r1=1077994&r2=1077995&view=diff
==============================================================================
--- tomcat/trunk/java/org/apache/catalina/core/StandardContext.java (original)
+++ tomcat/trunk/java/org/apache/catalina/core/StandardContext.java Fri Mar 4
15:17:22 2011
@@ -837,6 +837,12 @@ public class StandardContext extends Con
private boolean fireRequestListenersOnForwards = false;
+ /**
+ * Servlets created via {@link ApplicationContext#createServlet(Class)} for
+ * tracking purposes.
+ */
+ private Set<Servlet> createdServlets = new HashSet<Servlet>();
+
// ----------------------------------------------------- Context Properties
@@ -4364,6 +4370,11 @@ public class StandardContext extends Con
* @param wrapper The wrapper for the Servlet that was added
*/
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);
}
@@ -4372,7 +4383,7 @@ public class StandardContext extends Con
* @param servlet
*/
public void dynamicServletCreated(Servlet servlet) {
- // NOOP - Hook for JACC implementations
+ createdServlets.add(servlet);
}
@@ -5530,6 +5541,8 @@ public class StandardContext extends Con
initializers.clear();
+ createdServlets.clear();
+
if(log.isDebugEnabled())
log.debug("resetContext " + getObjectName());
}
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=1077995&r1=1077994&r2=1077995&view=diff
==============================================================================
--- tomcat/trunk/java/org/apache/catalina/core/StandardWrapper.java (original)
+++ tomcat/trunk/java/org/apache/catalina/core/StandardWrapper.java Fri Mar 4
15:17:22 2011
@@ -273,6 +273,8 @@ public class StandardWrapper extends Con
*/
protected boolean enabled = true;
+ protected volatile boolean servletSecurityAnnotationScanRequired = false;
+
/**
* Static class array used when the SecurityManager is turned on and
* <code>Servlet.init</code> is invoked.
@@ -643,6 +645,15 @@ public class StandardWrapper extends Con
instance = servlet;
}
+
+ /**
+ * {@inheritDoc}
+ */
+ @Override
+ public void setServletSecurityAnnotationScanRequired(boolean b) {
+ this.servletSecurityAnnotationScanRequired = b;
+ }
+
// --------------------------------------------------------- Public Methods
@@ -1077,20 +1088,12 @@ public class StandardWrapper extends Con
}
}
- ServletSecurity secAnnotation =
- servlet.getClass().getAnnotation(ServletSecurity.class);
- Context ctxt = (Context) getParent();
- if (secAnnotation != null) {
- ctxt.addServletSecurity(
- new ApplicationServletRegistration(this, ctxt),
- new ServletSecurityElement(secAnnotation));
- }
-
+ processServletSecurityAnnotation(servlet);
// Special handling for ContainerServlet instances
if ((servlet instanceof ContainerServlet) &&
- (isContainerProvidedServlet(servletClass) ||
- ctxt.getPrivileged() )) {
+ (isContainerProvidedServlet(servletClass) ||
+ ((Context) getParent()).getPrivileged() )) {
((ContainerServlet) servlet).setWrapper(this);
}
@@ -1123,6 +1126,34 @@ public class StandardWrapper extends Con
}
+ /**
+ * {@inheritDoc}
+ */
+ @Override
+ public void servletSecurityAnnotationScan() throws ServletException {
+ if (instance == null) {
+ load();
+ } else {
+ if (servletSecurityAnnotationScanRequired) {
+ processServletSecurityAnnotation(instance);
+ }
+ }
+ }
+
+ private void processServletSecurityAnnotation(Servlet servlet) {
+ // Calling this twice isn't harmful so no syncs
+ servletSecurityAnnotationScanRequired = false;
+
+ ServletSecurity secAnnotation =
+ servlet.getClass().getAnnotation(ServletSecurity.class);
+ Context ctxt = (Context) getParent();
+ if (secAnnotation != null) {
+ ctxt.addServletSecurity(
+ new ApplicationServletRegistration(this, ctxt),
+ new ServletSecurityElement(secAnnotation));
+ }
+ }
+
private synchronized void initServlet(Servlet servlet)
throws ServletException {
Modified: tomcat/trunk/test/org/apache/catalina/core/TestStandardWrapper.java
URL:
http://svn.apache.org/viewvc/tomcat/trunk/test/org/apache/catalina/core/TestStandardWrapper.java?rev=1077995&r1=1077994&r2=1077995&view=diff
==============================================================================
--- tomcat/trunk/test/org/apache/catalina/core/TestStandardWrapper.java
(original)
+++ tomcat/trunk/test/org/apache/catalina/core/TestStandardWrapper.java Fri Mar
4 15:17:22 2011
@@ -23,8 +23,13 @@ import java.util.ArrayList;
import java.util.HashMap;
import java.util.List;
import java.util.Map;
+import java.util.Set;
+import javax.servlet.Servlet;
+import javax.servlet.ServletContainerInitializer;
+import javax.servlet.ServletContext;
import javax.servlet.ServletException;
+import javax.servlet.ServletRegistration;
import javax.servlet.annotation.HttpConstraint;
import javax.servlet.annotation.HttpMethodConstraint;
import javax.servlet.annotation.ServletSecurity;
@@ -112,6 +117,43 @@ public class TestStandardWrapper extends
assertEquals(200, rc);
}
+ public void testSecurityAnnotationsAddServlet1() throws Exception {
+ doTestSecurityAnnotationsAddServlet(false);
+ }
+
+ public void testSecurityAnnotationsAddServlet2() throws Exception {
+ doTestSecurityAnnotationsAddServlet(true);
+ }
+
+ private void doTestSecurityAnnotationsAddServlet(boolean useCreateServlet)
+ throws Exception {
+
+ // Setup Tomcat instance
+ Tomcat tomcat = getTomcatInstance();
+
+ // Must have a real docBase - just use temp
+ Context ctx =
+ tomcat.addContext("", System.getProperty("java.io.tmpdir"));
+
+ Servlet s = new DenyAllServlet();
+ ServletContainerInitializer sci = new SCI(s, useCreateServlet);
+ ctx.addServletContainerInitializer(sci, null);
+
+ tomcat.start();
+
+ ByteChunk bc = new ByteChunk();
+ int rc;
+ rc = getUrl("http://localhost:"; + getPort() + "/", bc, null, null);
+
+ if (useCreateServlet) {
+ assertNull(bc.toString());
+ assertEquals(403, rc);
+ } else {
+ assertEquals("OK", bc.toString());
+ assertEquals(200, rc);
+ }
+ }
+
private void doTest(String servletClassName, boolean usePost,
boolean useRole, boolean expect200) throws Exception {
@@ -217,4 +259,29 @@ public class TestStandardWrapper extends
public static class RoleDenyServlet extends TestServlet {
private static final long serialVersionUID = 1L;
}
+
+ public static class SCI implements ServletContainerInitializer {
+
+ private Servlet servlet;
+ private boolean createServlet;
+
+ public SCI(Servlet servlet, boolean createServlet) {
+ this.servlet = servlet;
+ this.createServlet = createServlet;
+ }
+
+ @Override
+ public void onStartup(Set<Class<?>> c, ServletContext ctx)
+ throws ServletException {
+ Servlet s;
+
+ if (createServlet) {
+ s = ctx.createServlet(servlet.getClass());
+ } else {
+ s = servlet;
+ }
+ ServletRegistration.Dynamic r = ctx.addServlet("servlet", s);
+ r.addMapping("/");
+ }
+ }
}
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org
For additional commands, e-mail: dev-h...@tomcat.apache.org
