Author: markt
Date: Wed Nov 18 23:33:57 2009
New Revision: 882002
URL: http://svn.apache.org/viewvc?rev=882002&view=rev
Log:
Fix https://issues.apache.org/bugzilla/show_bug.cgi?id=48172
Potential threading issues, make fields final where possible
Modified:
tomcat/trunk/java/org/apache/jasper/compiler/JspRuntimeContext.java
Modified: tomcat/trunk/java/org/apache/jasper/compiler/JspRuntimeContext.java
URL:
http://svn.apache.org/viewvc/tomcat/trunk/java/org/apache/jasper/compiler/JspRuntimeContext.java?rev=882002&r1=882001&r2=882002&view=diff
==============================================================================
--- tomcat/trunk/java/org/apache/jasper/compiler/JspRuntimeContext.java
(original)
+++ tomcat/trunk/java/org/apache/jasper/compiler/JspRuntimeContext.java Wed Nov
18 23:33:57 2009
@@ -29,6 +29,7 @@
import java.util.Iterator;
import java.util.Map;
import java.util.concurrent.ConcurrentHashMap;
+import java.util.concurrent.atomic.AtomicInteger;
import javax.servlet.ServletContext;
import javax.servlet.jsp.JspFactory;
@@ -63,7 +64,7 @@
/*
* Counts how many times the webapp's JSPs have been reloaded.
*/
- private int jspReloadCount;
+ private AtomicInteger jspReloadCount;
/**
* Preload classes required at runtime by a JSP servlet so that
@@ -110,29 +111,37 @@
this.options = options;
// Get the parent class loader
- parentClassLoader = Thread.currentThread().getContextClassLoader();
- if (parentClassLoader == null) {
- parentClassLoader = this.getClass().getClassLoader();
+ ClassLoader loader = Thread.currentThread().getContextClassLoader();
+ if (loader == null) {
+ loader = this.getClass().getClassLoader();
}
if (log.isDebugEnabled()) {
- if (parentClassLoader != null) {
+ if (loader != null) {
log.debug(Localizer.getMessage("jsp.message.parent_class_loader_is",
- parentClassLoader.toString()));
+ loader.toString()));
} else {
log.debug(Localizer.getMessage("jsp.message.parent_class_loader_is",
"<none>"));
}
}
- initClassPath();
+ parentClassLoader = loader;
+ classpath = initClassPath();
if (context instanceof org.apache.jasper.servlet.JspCServletContext) {
+ codeSource = null;
+ permissionCollection = null;
return;
}
if (Constants.IS_SECURITY_ENABLED) {
- initSecurity();
+ SecurityHolder holder = initSecurity();
+ codeSource = holder.cs;
+ permissionCollection = holder.pc;
+ } else {
+ codeSource = null;
+ permissionCollection = null;
}
// If this web application context is running from a
@@ -150,13 +159,13 @@
/**
* This web applications ServletContext
*/
- private ServletContext context;
- private Options options;
- private ClassLoader parentClassLoader;
- private PermissionCollection permissionCollection;
- private CodeSource codeSource;
- private String classpath;
- private long lastCheck = -1L;
+ private final ServletContext context;
+ private final Options options;
+ private final ClassLoader parentClassLoader;
+ private final PermissionCollection permissionCollection;
+ private final CodeSource codeSource;
+ private final String classpath;
+ private volatile long lastCheck = -1L;
/**
* Maps JSP pages to their JspServletWrapper's
@@ -247,8 +256,8 @@
/**
* Increments the JSP reload counter.
*/
- public synchronized void incrementJspReloadCount() {
- jspReloadCount++;
+ public void incrementJspReloadCount() {
+ jspReloadCount.incrementAndGet();
}
/**
@@ -256,8 +265,8 @@
*
* @param count Value to which to reset the JSP reload counter
*/
- public synchronized void setJspReloadCount(int count) {
- this.jspReloadCount = count;
+ public void setJspReloadCount(int count) {
+ jspReloadCount.set(count);
}
/**
@@ -266,7 +275,7 @@
* @return The current value of the JSP reload counter
*/
public int getJspReloadCount() {
- return jspReloadCount;
+ return jspReloadCount.intValue();
}
@@ -321,7 +330,7 @@
/**
* Method used to initialize classpath for compiles.
*/
- private void initClassPath() {
+ private String initClassPath() {
StringBuilder cpath = new StringBuilder();
String sep = System.getProperty("path.separator");
@@ -348,23 +357,35 @@
cp = options.getClassPath();
}
- classpath = cpath.toString() + cp;
+ String path = cpath.toString() + cp;
if(log.isDebugEnabled()) {
- log.debug("Compilation classpath initialized: " + getClassPath());
+ log.debug("Compilation classpath initialized: " + path);
}
+ return path;
}
+ // Helper class to allow initSecurity() to return two items
+ private static class SecurityHolder{
+ private final CodeSource cs;
+ private final PermissionCollection pc;
+ private SecurityHolder(CodeSource cs, PermissionCollection pc){
+ this.cs = cs;
+ this.pc = pc;
+ }
+ }
/**
* Method used to initialize SecurityManager data.
*/
- private void initSecurity() {
+ private SecurityHolder initSecurity() {
// Setup the PermissionCollection for this web app context
// based on the permissions configured for the root of the
// web app context directory, then add a file read permission
// for that directory.
Policy policy = Policy.getPolicy();
+ CodeSource source = null;
+ PermissionCollection permissions = null;
if( policy != null ) {
try {
// Get the permissions for the web app context
@@ -378,21 +399,21 @@
}
File contextDir = new File(codeBase);
URL url = contextDir.getCanonicalFile().toURI().toURL();
- codeSource = new CodeSource(url,(Certificate[])null);
- permissionCollection = policy.getPermissions(codeSource);
+ source = new CodeSource(url,(Certificate[])null);
+ permissions = policy.getPermissions(source);
// Create a file read permission for web app context directory
if (!docBase.endsWith(File.separator)){
- permissionCollection.add
+ permissions.add
(new FilePermission(docBase,"read"));
docBase = docBase + File.separator;
} else {
- permissionCollection.add
+ permissions.add
(new FilePermission
(docBase.substring(0,docBase.length() -
1),"read"));
}
docBase = docBase + "-";
- permissionCollection.add(new FilePermission(docBase,"read"));
+ permissions.add(new FilePermission(docBase,"read"));
// Spec says apps should have read/write for their temp
// directory. This is fine, as no security sensitive files, at
@@ -400,16 +421,16 @@
// will be written here.
String workDir = options.getScratchDir().toString();
if (!workDir.endsWith(File.separator)){
- permissionCollection.add
+ permissions.add
(new FilePermission(workDir,"read,write"));
workDir = workDir + File.separator;
}
workDir = workDir + "-";
- permissionCollection.add(new FilePermission(
+ permissions.add(new FilePermission(
workDir,"read,write,delete"));
// Allow the JSP to access
org.apache.jasper.runtime.HttpJspBase
- permissionCollection.add( new RuntimePermission(
+ permissions.add( new RuntimePermission(
"accessClassInPackage.org.apache.jasper.runtime") );
if (parentClassLoader instanceof URLClassLoader) {
@@ -431,19 +452,20 @@
}
}
if (jarUrl != null) {
- permissionCollection.add(
+ permissions.add(
new FilePermission(jarUrl,"read"));
- permissionCollection.add(
+ permissions.add(
new
FilePermission(jarUrl.substring(4),"read"));
}
if (jndiUrl != null)
- permissionCollection.add(
+ permissions.add(
new FilePermission(jndiUrl,"read") );
}
} catch(Exception e) {
context.log("Security Init for context failed",e);
}
}
+ return new SecurityHolder(source, permissions);
}
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org
For additional commands, e-mail: dev-h...@tomcat.apache.org
