Author: nbubna
Date: Mon Oct 6 11:15:18 2008
New Revision: 702218
URL: http://svn.apache.org/viewvc?rev=702218&view=rev
Log:
VELOCITY-618 add support for a 'strict reference mode' for more rigorous
template authoring (patch and idea by Byron Foster)
Added:
velocity/engine/trunk/src/test/org/apache/velocity/test/StrictReferenceTestCase.java
(with props)
Modified:
velocity/engine/trunk/src/java/org/apache/velocity/context/AbstractContext.java
velocity/engine/trunk/src/java/org/apache/velocity/context/ProxyVMContext.java
velocity/engine/trunk/src/java/org/apache/velocity/runtime/RuntimeConstants.java
velocity/engine/trunk/src/java/org/apache/velocity/runtime/parser/node/ASTIdentifier.java
velocity/engine/trunk/src/java/org/apache/velocity/runtime/parser/node/ASTMethod.java
velocity/engine/trunk/src/java/org/apache/velocity/runtime/parser/node/ASTReference.java
velocity/engine/trunk/src/java/org/apache/velocity/runtime/parser/node/ASTSetDirective.java
Modified:
velocity/engine/trunk/src/java/org/apache/velocity/context/AbstractContext.java
URL:
http://svn.apache.org/viewvc/velocity/engine/trunk/src/java/org/apache/velocity/context/AbstractContext.java?rev=702218&r1=702217&r2=702218&view=diff
==============================================================================
---
velocity/engine/trunk/src/java/org/apache/velocity/context/AbstractContext.java
(original)
+++
velocity/engine/trunk/src/java/org/apache/velocity/context/AbstractContext.java
Mon Oct 6 11:15:18 2008
@@ -154,18 +154,13 @@
public Object put(String key, Object value)
{
/*
- * don't even continue if key or value is null
+ * don't even continue if key is null
*/
-
if (key == null)
{
return null;
}
- else if (value == null)
- {
- return null;
- }
-
+
return internalPut(key, value);
}
@@ -219,7 +214,13 @@
return false;
}
- return internalContainsKey(key);
+ boolean exists = internalContainsKey(key);
+ if (!exists && innerContext != null)
+ {
+ exists = innerContext.containsKey(key);
+ }
+
+ return exists;
}
/**
Modified:
velocity/engine/trunk/src/java/org/apache/velocity/context/ProxyVMContext.java
URL:
http://svn.apache.org/viewvc/velocity/engine/trunk/src/java/org/apache/velocity/context/ProxyVMContext.java?rev=702218&r1=702217&r2=702218&view=diff
==============================================================================
---
velocity/engine/trunk/src/java/org/apache/velocity/context/ProxyVMContext.java
(original)
+++
velocity/engine/trunk/src/java/org/apache/velocity/context/ProxyVMContext.java
Mon Oct 6 11:15:18 2008
@@ -237,7 +237,17 @@
}
else
{
- return wrappedContext.get(ref.getRootString());
+ Object obj = wrappedContext.get(ref.getRootString());
+ if (obj == null && ref.strictRef)
+ {
+ if (!wrappedContext.containsKey(ref.getRootString()))
+ {
+ throw new MethodInvocationException("Parameter '"
+ ref.getRootString()
+ + "' not defined", null, key,
ref.getTemplateName(),
+ ref.getLine(), ref.getColumn());
+ }
+ }
+ return obj;
}
}
else if (type == ParserTreeConstants.JJTTEXT)
@@ -285,7 +295,9 @@
*/
public boolean containsKey(Object key)
{
- return false;
+ return vmproxyhash.containsKey(key)
+ || localcontext.containsKey(key)
+ || super.containsKey(key);
}
/**
Modified:
velocity/engine/trunk/src/java/org/apache/velocity/runtime/RuntimeConstants.java
URL:
http://svn.apache.org/viewvc/velocity/engine/trunk/src/java/org/apache/velocity/runtime/RuntimeConstants.java?rev=702218&r1=702217&r2=702218&view=diff
==============================================================================
---
velocity/engine/trunk/src/java/org/apache/velocity/runtime/RuntimeConstants.java
(original)
+++
velocity/engine/trunk/src/java/org/apache/velocity/runtime/RuntimeConstants.java
Mon Oct 6 11:15:18 2008
@@ -54,6 +54,11 @@
String RUNTIME_LOG_LOGSYSTEM_CLASS = "runtime.log.logsystem.class";
/**
+ * Properties referenced in the template are required to exist the object
+ */
+ String RUNTIME_REFERENCES_STRICT = "runtime.references.strict";
+
+ /**
* @deprecated This appears to have always been meaningless.
*/
String RUNTIME_LOG_ERROR_STACKTRACE = "runtime.log.error.stacktrace";
Modified:
velocity/engine/trunk/src/java/org/apache/velocity/runtime/parser/node/ASTIdentifier.java
URL:
http://svn.apache.org/viewvc/velocity/engine/trunk/src/java/org/apache/velocity/runtime/parser/node/ASTIdentifier.java?rev=702218&r1=702217&r2=702218&view=diff
==============================================================================
---
velocity/engine/trunk/src/java/org/apache/velocity/runtime/parser/node/ASTIdentifier.java
(original)
+++
velocity/engine/trunk/src/java/org/apache/velocity/runtime/parser/node/ASTIdentifier.java
Mon Oct 6 11:15:18 2008
@@ -26,6 +26,7 @@
import org.apache.velocity.exception.MethodInvocationException;
import org.apache.velocity.exception.TemplateInitException;
import org.apache.velocity.exception.VelocityException;
+import org.apache.velocity.runtime.RuntimeConstants;
import org.apache.velocity.runtime.parser.Parser;
import org.apache.velocity.util.introspection.Info;
import org.apache.velocity.util.introspection.IntrospectionCacheData;
@@ -55,6 +56,11 @@
* This is really immutable after the init, so keep one for this node
*/
protected Info uberInfo;
+
+ /**
+ * Indicates if we are running in strict reference mode.
+ */
+ protected boolean strictRef = false;
/**
* @param id
@@ -100,6 +106,8 @@
uberInfo = new Info(context.getCurrentTemplateName(),
getLine(), getColumn());
+ strictRef =
rsvc.getBoolean(RuntimeConstants.RUNTIME_REFERENCES_STRICT, false);
+
return data;
}
@@ -170,7 +178,16 @@
if (vg == null)
{
- return null;
+ if (strictRef)
+ {
+ throw new MethodInvocationException("Object '" +
o.getClass().getName() +
+ "' does not contain property '" + identifier + "'", null,
identifier,
+ uberInfo.getTemplateName(), uberInfo.getLine(),
uberInfo.getColumn());
+ }
+ else
+ {
+ return null;
+ }
}
/*
Modified:
velocity/engine/trunk/src/java/org/apache/velocity/runtime/parser/node/ASTMethod.java
URL:
http://svn.apache.org/viewvc/velocity/engine/trunk/src/java/org/apache/velocity/runtime/parser/node/ASTMethod.java?rev=702218&r1=702217&r2=702218&view=diff
==============================================================================
---
velocity/engine/trunk/src/java/org/apache/velocity/runtime/parser/node/ASTMethod.java
(original)
+++
velocity/engine/trunk/src/java/org/apache/velocity/runtime/parser/node/ASTMethod.java
Mon Oct 6 11:15:18 2008
@@ -28,6 +28,7 @@
import org.apache.velocity.exception.MethodInvocationException;
import org.apache.velocity.exception.TemplateInitException;
import org.apache.velocity.exception.VelocityException;
+import org.apache.velocity.runtime.RuntimeConstants;
import org.apache.velocity.runtime.parser.Parser;
import org.apache.velocity.util.introspection.Info;
import org.apache.velocity.util.introspection.IntrospectionCacheData;
@@ -57,6 +58,11 @@
protected Info uberInfo;
/**
+ * Indicates if we are running in strict reference mode.
+ */
+ protected boolean strictRef = false;
+
+ /**
* @param id
*/
public ASTMethod(int id)
@@ -107,6 +113,8 @@
methodName = getFirstToken().image;
paramCount = jjtGetNumChildren() - 1;
+ strictRef =
rsvc.getBoolean(RuntimeConstants.RUNTIME_REFERENCES_STRICT, false);
+
return data;
}
@@ -201,7 +209,24 @@
if (method == null)
{
- return null;
+ if (strictRef)
+ {
+ // Create a parameter list for the exception error message
+ StringBuffer plist = new StringBuffer();
+ for (int i=0; i<params.length; i++)
+ {
+ Class param = paramClasses[i];
+ plist.append(param == null ? "null" : param.getName());
+ if (i < params.length -1) plist.append(", ");
+ }
+ throw new MethodInvocationException("Object '" +
o.getClass().getName() +
+ "' does not contain method " + methodName + "(" + plist
+ ")",
+ null, methodName, uberInfo.getTemplateName(),
uberInfo.getLine(), uberInfo.getColumn());
+ }
+ else
+ {
+ return null;
+ }
}
}
catch( MethodInvocationException mie )
Modified:
velocity/engine/trunk/src/java/org/apache/velocity/runtime/parser/node/ASTReference.java
URL:
http://svn.apache.org/viewvc/velocity/engine/trunk/src/java/org/apache/velocity/runtime/parser/node/ASTReference.java?rev=702218&r1=702217&r2=702218&view=diff
==============================================================================
---
velocity/engine/trunk/src/java/org/apache/velocity/runtime/parser/node/ASTReference.java
(original)
+++
velocity/engine/trunk/src/java/org/apache/velocity/runtime/parser/node/ASTReference.java
Mon Oct 6 11:15:18 2008
@@ -69,6 +69,11 @@
private String literal = null;
+ /**
+ * Indicates if we are running in strict reference mode.
+ */
+ public boolean strictRef = false;
+
private int numChildren = 0;
protected Info uberInfo;
@@ -142,10 +147,53 @@
logOnNull =
rsvc.getBoolean(RuntimeConstants.RUNTIME_LOG_REFERENCE_LOG_INVALID, true);
+ strictRef =
rsvc.getBoolean(RuntimeConstants.RUNTIME_REFERENCES_STRICT, false);
+
+ /**
+ * In the case we are referencing a variable with #if($foo) or
+ * #if( ! $foo) then we allow variables to be undefined and we
+ * set strictRef to false so that if the variable is undefined
+ * an exception is not thrown.
+ */
+ if (strictRef && numChildren == 0)
+ {
+ logOnNull = false; // Strict mode allows nulls
+
+ Node node = this.jjtGetParent();
+ if (node instanceof ASTNotNode // #if( ! $foo)
+ || node instanceof ASTExpression // #if( $foo )
+ || node instanceof ASTOrNode // #if( $foo || ...
+ || node instanceof ASTAndNode) // #if( $foo && ...
+ {
+ // Now scan up tree to see if we are in an If statement
+ while (node != null)
+ {
+ if (node instanceof ASTIfStatement)
+ {
+ strictRef = false;
+ break;
+ }
+ node = node.jjtGetParent();
+ }
+ }
+ }
+
return data;
}
/**
+ * Returns the template name, once init() has been called.
+ */
+ public String getTemplateName()
+ {
+ if (uberInfo == null)
+ {
+ return null;
+ }
+ return uberInfo.getTemplateName();
+ }
+
+ /**
* Returns the 'root string', the reference key
* @return the root string.
*/
@@ -175,7 +223,7 @@
Object result = getVariableValue(context, rootString);
- if (result == null)
+ if (result == null && !strictRef)
{
return EventHandlerUtil.invalidGetMethod(rsvc, context,
"$" + rootString, null, null, uberInfo);
@@ -200,9 +248,22 @@
int failedChild = -1;
for (int i = 0; i < numChildren; i++)
{
+ if (strictRef && result == null)
+ {
+ /**
+ * At this point we know that an attempt is about to be
made
+ * to call a method or property on a null value.
+ */
+ String name = jjtGetChild(i).getFirstToken().image;
+ throw new MethodInvocationException("Attempted to access
'"
+ + name + "' on a null value", null, name,
uberInfo.getTemplateName(),
+ jjtGetChild(i).getLine(), jjtGetChild(i).getColumn());
+
+ }
previousResult = result;
result = jjtGetChild(i).execute(result,context);
- if (result == null)
+ if (result == null && !strictRef) // If strict and null then
well catch this
+ // next time through the
loop
{
failedChild = i;
break;
@@ -500,6 +561,14 @@
if (result == null)
{
+ if (strictRef)
+ {
+ String name = jjtGetChild(i+1).getFirstToken().image;
+ throw new MethodInvocationException("Attempted to access
'"
+ + name + "' on a null value", null, name,
uberInfo.getTemplateName(),
+ jjtGetChild(i+1).getLine(),
jjtGetChild(i+1).getColumn());
+ }
+
String msg = "reference set : template = "
+ context.getCurrentTemplateName() +
" [line " + getLine() + ",column " +
@@ -525,7 +594,18 @@
value, uberInfo);
if (vs == null)
- return false;
+ {
+ if (strictRef)
+ {
+ throw new MethodInvocationException("Object '" +
result.getClass().getName() +
+ "' does not contain property '" + identifier + "'",
null, identifier,
+ uberInfo.getTemplateName(), uberInfo.getLine(),
uberInfo.getColumn());
+ }
+ else
+ {
+ return false;
+ }
+ }
vs.invoke(result, value);
}
@@ -770,7 +850,17 @@
*/
public Object getVariableValue(Context context, String variable) throws
MethodInvocationException
{
- return context.get(variable);
+ Object obj = context.get(variable);
+ if (strictRef && obj == null)
+ {
+ if (!context.containsKey(variable))
+ {
+ throw new MethodInvocationException("Variable '" + variable +
+ "' has not been set", null, identifier,
+ uberInfo.getTemplateName(), uberInfo.getLine(),
uberInfo.getColumn());
+ }
+ }
+ return obj;
}
Modified:
velocity/engine/trunk/src/java/org/apache/velocity/runtime/parser/node/ASTSetDirective.java
URL:
http://svn.apache.org/viewvc/velocity/engine/trunk/src/java/org/apache/velocity/runtime/parser/node/ASTSetDirective.java?rev=702218&r1=702217&r2=702218&view=diff
==============================================================================
---
velocity/engine/trunk/src/java/org/apache/velocity/runtime/parser/node/ASTSetDirective.java
(original)
+++
velocity/engine/trunk/src/java/org/apache/velocity/runtime/parser/node/ASTSetDirective.java
Mon Oct 6 11:15:18 2008
@@ -52,6 +52,11 @@
protected Info uberInfo;
/**
+ * Indicates if we are running in strict reference mode.
+ */
+ protected boolean strictRef = false;
+
+ /**
* @param id
*/
public ASTSetDirective(int id)
@@ -104,7 +109,9 @@
logOnNull =
rsvc.getBoolean(RuntimeConstants.RUNTIME_LOG_REFERENCE_LOG_INVALID, true);
allowNull = rsvc.getBoolean(RuntimeConstants.SET_NULL_ALLOWED,
false);
-
+ strictRef =
rsvc.getBoolean(RuntimeConstants.RUNTIME_REFERENCES_STRICT, false);
+ if (strictRef) allowNull = true; // strictRef implies allowNull
+
/*
* grab this now. No need to redo each time
*/
@@ -168,7 +175,7 @@
}
}
- if ( value == null )
+ if ( value == null && !strictRef)
{
String rightReference = null;
if (right instanceof ASTExpression)
Added:
velocity/engine/trunk/src/test/org/apache/velocity/test/StrictReferenceTestCase.java
URL:
http://svn.apache.org/viewvc/velocity/engine/trunk/src/test/org/apache/velocity/test/StrictReferenceTestCase.java?rev=702218&view=auto
==============================================================================
---
velocity/engine/trunk/src/test/org/apache/velocity/test/StrictReferenceTestCase.java
(added)
+++
velocity/engine/trunk/src/test/org/apache/velocity/test/StrictReferenceTestCase.java
Mon Oct 6 11:15:18 2008
@@ -0,0 +1,175 @@
+package org.apache.velocity.test;
+
+import org.apache.velocity.exception.MethodInvocationException;
+import org.apache.velocity.runtime.RuntimeConstants;
+
+/**
+ * Test strict reference mode turned on by the velocity property
+ * runtime.references.strict
+ */
+public class StrictReferenceTestCase extends BaseEvalTestCase
+{
+ public StrictReferenceTestCase(String name)
+ {
+ super(name);
+ }
+
+ public void setUp() throws Exception
+ {
+ super.setUp();
+ engine.setProperty(RuntimeConstants.RUNTIME_REFERENCES_STRICT, true);
+ context.put("NULL", null);
+ context.put("bar", null);
+ context.put("TRUE", Boolean.TRUE);
+ }
+
+
+ /**
+ * Test the modified behavior of #if in strict mode. Mainly, that
+ * single variables references in #if statements use non strict rules
+ */
+ public void testIfStatement()
+ {
+ Fargo fargo = new Fargo();
+ fargo.next = new Fargo();
+ context.put("fargo", fargo);
+ assertEvalEquals("", "#if($bogus)xxx#end");
+ assertEvalEquals("xxx", "#if($fargo)xxx#end");
+ assertEvalEquals("", "#if( ! $fargo)xxx#end");
+ assertEvalEquals("xxx", "#if($bogus || $fargo)xxx#end");
+ assertEvalEquals("", "#if($bogus && $fargo)xxx#end");
+ assertEvalEquals("", "#if($fargo != $NULL && $bogus)xxx#end");
+ assertEvalEquals("xxx", "#if($fargo == $NULL || ! $bogus)xxx#end");
+ assertEvalEquals("xxx", "#if(! $bogus1 && ! $bogus2)xxx#end");
+ assertEvalEquals("xxx", "#if($fargo.prop == \"propiness\" && ! $bogus
&& $bar == $NULL)xxx#end");
+ assertEvalEquals("", "#if($bogus && $bogus.foo)xxx#end");
+
+ assertMethodEx("#if($bogus.foo)#end");
+ assertMethodEx("#if(!$bogus.foo)#end");
+ }
+
+
+ /**
+ * We make sure that variables can actuall hold null
+ * values.
+ */
+ public void testAllowNullValues()
+ throws Exception
+ {
+ evaluate("$bar");
+ assertEvalEquals("true", "#if($bar == $NULL)true#end");
+ assertEvalEquals("false", "#set($foobar =
$NULL)#if(!$foobar)false#end");
+ assertEvalEquals("13", "#set($list = [1, $NULL, 3])#foreach($item in
$list)#if($item)$item#end#end");
+ }
+
+ /**
+ * Test that variables references that have not been defined throw
exceptions
+ */
+ public void testStrictVariableRef()
+ throws Exception
+ {
+ // We expect a Method exception on the following
+ assertMethodEx("$bogus");
+ assertMethodEx("#macro(test)$bogus#end #test()");
+
+ assertMethodEx("#set($bar = $bogus)");
+
+ assertMethodEx("#if($bogus == \"bar\") #end");
+ assertMethodEx("#if($bogus != \"bar\") #end");
+ assertMethodEx("#if(\"bar\" == $bogus) #end");
+ assertMethodEx("#if($bogus > 1) #end");
+ assertMethodEx("#foreach($item in $bogus)#end");
+
+ // make sure no exceptions are thrown here
+ evaluate("#set($foo = \"bar\") $foo");
+ evaluate("#macro(test1 $foo1) $foo1 #end #test1(\"junk\")");
+ evaluate("#macro(test2) #set($foo2 = \"bar\") $foo2 #end #test2()");
+ }
+
+ /**
+ * Test that exceptions are thrown when methods are called on
+ * references that contains objects that do not contains those
+ * methods.
+ */
+ public void testStrictMethodRef()
+ {
+ Fargo fargo = new Fargo();
+ fargo.next = new Fargo();
+ context.put("fargo", fargo);
+
+ // Mainly want to make sure no exceptions are thrown here
+ assertEvalEquals("propiness", "$fargo.prop");
+ assertEvalEquals("$fargo.nullVal", "$fargo.nullVal");
+ assertEvalEquals("", "$!fargo.nullVal");
+ assertEvalEquals("propiness", "$fargo.next.prop");
+
+ assertMethodEx("$fargo.foobar");
+ assertMethodEx("$fargo.next.foobar");
+ assertMethodEx("$fargo.foobar()");
+ assertMethodEx("#set($fargo.next.prop = $TRUE)");
+ assertMethodEx("$fargo.next.setProp($TRUE)");
+ }
+
+ /**
+ * Make sure exceptions are thrown when when we attempt to call
+ * methods on null values.
+ */
+ public void testStrictMethodOnNull()
+ {
+ Fargo fargo = new Fargo();
+ fargo.next = new Fargo();
+ context.put("fargo", fargo);
+
+ assertMethodEx("$NULL.bogus");
+ assertMethodEx("$fargo.nullVal.bogus");
+ assertMethodEx("$fargo.next.nullVal.bogus");
+ assertMethodEx("#if (\"junk\" == $fargo.nullVal.bogus)#end");
+ assertMethodEx("#if ($fargo.nullVal.bogus > 2)#end");
+ assertMethodEx("#set($fargo.next.nullVal.bogus = \"junk\")");
+ assertMethodEx("#set($foo = $NULL.bogus)");
+ assertMethodEx("#foreach($item in $fargo.next.nullVal.bogus)#end");
+
+ evaluate("$fargo.prop.toString()");
+ assertMethodEx("#set($fargo.prop = $NULL)$fargo.prop.next");
+
+ // make sure no exceptions are thrown here
+ evaluate("$fargo.next.next");
+ evaluate("$fargo.next.nullVal");
+ evaluate("#foreach($item in $fargo.nullVal)#end");
+ }
+
+ /**
+ * Assert that we get a MethodInvocationException when calling evaluate
+ */
+ public void assertMethodEx(String template)
+ {
+ assertEvalException(template, MethodInvocationException.class);
+ }
+
+
+ public static class Fargo
+ {
+ String prop = "propiness";
+ Fargo next = null;
+
+ public String getProp()
+ {
+ return prop;
+ }
+
+ public void setProp(String val)
+ {
+ this.prop = prop;
+ }
+
+ public String getNullVal()
+ {
+ return null;
+ }
+
+ public Fargo getNext()
+ {
+ return next;
+ }
+ }
+}
Propchange:
velocity/engine/trunk/src/test/org/apache/velocity/test/StrictReferenceTestCase.java
------------------------------------------------------------------------------
svn:eol-style = native
Propchange:
velocity/engine/trunk/src/test/org/apache/velocity/test/StrictReferenceTestCase.java
------------------------------------------------------------------------------
svn:keywords = Revision
Propchange:
velocity/engine/trunk/src/test/org/apache/velocity/test/StrictReferenceTestCase.java
------------------------------------------------------------------------------
svn:mime-type = text/plain
