You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@velocity.apache.org by cb...@apache.org on 2016/07/13 21:58:37 UTC
svn commit: r1752548 - in /velocity/engine/trunk/velocity-engine-core/src:
main/java/org/apache/velocity/runtime/parser/node/
test/java/org/apache/velocity/test/
Author: cbrisson
Date: Wed Jul 13 21:58:37 2016
New Revision: 1752548
URL: http://svn.apache.org/viewvc?rev=1752548&view=rev
Log:
invalid reference event should not be triggered by null values, or when testing a value inside #if/#elseif (fixes VELOCITY-553)
Modified:
velocity/engine/trunk/velocity-engine-core/src/main/java/org/apache/velocity/runtime/parser/node/ASTMethod.java
velocity/engine/trunk/velocity-engine-core/src/main/java/org/apache/velocity/runtime/parser/node/ASTReference.java
velocity/engine/trunk/velocity-engine-core/src/test/java/org/apache/velocity/test/InvalidEventHandlerTestCase.java
Modified: velocity/engine/trunk/velocity-engine-core/src/main/java/org/apache/velocity/runtime/parser/node/ASTMethod.java
URL: http://svn.apache.org/viewvc/velocity/engine/trunk/velocity-engine-core/src/main/java/org/apache/velocity/runtime/parser/node/ASTMethod.java?rev=1752548&r1=1752547&r2=1752548&view=diff
==============================================================================
--- velocity/engine/trunk/velocity-engine-core/src/main/java/org/apache/velocity/runtime/parser/node/ASTMethod.java (original)
+++ velocity/engine/trunk/velocity-engine-core/src/main/java/org/apache/velocity/runtime/parser/node/ASTMethod.java Wed Jul 13 21:58:37 2016
@@ -32,6 +32,7 @@ import org.apache.velocity.runtime.direc
import org.apache.velocity.runtime.parser.Parser;
import org.apache.velocity.util.ClassUtils;
import org.apache.velocity.util.introspection.Info;
+import org.apache.velocity.util.introspection.IntrospectionCacheData;
import org.apache.velocity.util.introspection.VelMethod;
/**
@@ -161,7 +162,25 @@ public class ASTMethod extends SimpleNod
VelMethod method = ClassUtils.getMethod(methodName, params, paramClasses,
o, context, this, strictRef);
- if (method == null) return null;
+
+ /*
+ * The parent class (typically ASTReference) uses the icache entry
+ * under 'this' key to distinguish a valid null result from a non-existent method.
+ * So update this dummy cache value if necessary.
+ */
+ IntrospectionCacheData prevICD = context.icacheGet(this);
+ if (method == null)
+ {
+ if (prevICD != null)
+ {
+ context.icachePut(this, null);
+ }
+ return null;
+ }
+ else if (prevICD == null)
+ {
+ context.icachePut(this, new IntrospectionCacheData()); // no need to fill in its members
+ }
try
{
Modified: velocity/engine/trunk/velocity-engine-core/src/main/java/org/apache/velocity/runtime/parser/node/ASTReference.java
URL: http://svn.apache.org/viewvc/velocity/engine/trunk/velocity-engine-core/src/main/java/org/apache/velocity/runtime/parser/node/ASTReference.java?rev=1752548&r1=1752547&r2=1752548&view=diff
==============================================================================
--- velocity/engine/trunk/velocity-engine-core/src/main/java/org/apache/velocity/runtime/parser/node/ASTReference.java (original)
+++ velocity/engine/trunk/velocity-engine-core/src/main/java/org/apache/velocity/runtime/parser/node/ASTReference.java Wed Jul 13 21:58:37 2016
@@ -213,7 +213,7 @@ public class ASTReference extends Simple
/**
* gets an Object that 'is' the value of the reference
*
- * @param o unused Object parameter
+ * @param o Object parameter, unused per se, but non-null by convention inside an #if/#elseif evaluation
* @param context context used to generate value
* @return The execution result.
* @throws MethodInvocationException
@@ -221,6 +221,14 @@ public class ASTReference extends Simple
public Object execute(Object o, InternalContextAdapter context)
throws MethodInvocationException
{
+ /*
+ * The only case where 'o' is not null is when this method is called by evaluate().
+ * Its value is not used, but it is a convention meant to allow statements like
+ * #if($invalidReference) *not* to trigger an invalid reference event.
+ * Statements like #if($invalidReference.prop) should *still* trigger an invalid reference event.
+ * Statements like #if($validReference.invalidProp) should not.
+ */
+ boolean onlyTestingReference = (o != null);
if (referenceType == RUNT)
return null;
@@ -233,8 +241,15 @@ public class ASTReference extends Simple
if (result == null && !strictRef)
{
- return EventHandlerUtil.invalidGetMethod(rsvc, context,
- "$" + rootString, null, null, uberInfo);
+ /*
+ * do not trigger an invalid reference if the reference is present, but with a null value
+ * don't either inside an #if/#elseif evaluation context
+ */
+ if (!context.containsKey(rootString) && !onlyTestingReference)
+ {
+ return EventHandlerUtil.invalidGetMethod(rsvc, context,
+ "$" + rootString, null, null, uberInfo);
+ }
}
/*
@@ -273,8 +288,15 @@ public class ASTReference extends Simple
if (result == null && !strictRef) // If strict and null then well catch this
// next time through the loop
{
- failedChild = i;
- break;
+ // do not call bad reference handler if the getter is present
+ // (it means the getter has been called and returned null)
+ // do not either if the *last* child failed while testing the reference
+ Object getter = context.icacheGet(jjtGetChild(i));
+ if (getter == null && (!onlyTestingReference || i < jjtGetNumChildren() - 1))
+ {
+ failedChild = i;
+ break;
+ }
}
}
@@ -282,8 +304,15 @@ public class ASTReference extends Simple
{
if (failedChild == -1)
{
- result = EventHandlerUtil.invalidGetMethod(rsvc, context,
- "$" + rootString, previousResult, null, uberInfo);
+ /*
+ * do not trigger an invalid reference if the reference is present, but with a null value
+ * don't either inside an #if/#elseif evaluation context when there's no child
+ */
+ if (!context.containsKey(rootString) && (!onlyTestingReference || jjtGetNumChildren() > 0))
+ {
+ result = EventHandlerUtil.invalidGetMethod(rsvc, context,
+ "$" + rootString, previousResult, null, uberInfo);
+ }
}
else
{
@@ -304,7 +333,7 @@ public class ASTReference extends Simple
if (jjtGetChild(failedChild) instanceof ASTMethod)
{
String methodName = ((ASTMethod) jjtGetChild(failedChild)).getMethodName();
- result = EventHandlerUtil.invalidMethod(rsvc, context,
+ result = EventHandlerUtil.invalidMethod(rsvc, context,
name.toString(), previousResult, methodName, uberInfo);
}
else
@@ -526,7 +555,7 @@ public class ASTReference extends Simple
public boolean evaluate(InternalContextAdapter context)
throws MethodInvocationException
{
- Object value = execute(null, context);
+ Object value = execute(this, context); // non-null object as first parameter by convention for 'evaluate'
if (value == null)
{
return false;
Modified: velocity/engine/trunk/velocity-engine-core/src/test/java/org/apache/velocity/test/InvalidEventHandlerTestCase.java
URL: http://svn.apache.org/viewvc/velocity/engine/trunk/velocity-engine-core/src/test/java/org/apache/velocity/test/InvalidEventHandlerTestCase.java?rev=1752548&r1=1752547&r2=1752548&view=diff
==============================================================================
--- velocity/engine/trunk/velocity-engine-core/src/test/java/org/apache/velocity/test/InvalidEventHandlerTestCase.java (original)
+++ velocity/engine/trunk/velocity-engine-core/src/test/java/org/apache/velocity/test/InvalidEventHandlerTestCase.java Wed Jul 13 21:58:37 2016
@@ -46,6 +46,42 @@ import org.apache.velocity.util.introspe
public class InvalidEventHandlerTestCase
extends TestCase
{
+ // @@ VELOCITY-553
+ public class TestObject {
+ private String nullValueAttribute = null;
+
+ public String getNullValueAttribute() {
+ return nullValueAttribute;
+ }
+
+ public String getRealString() {
+ return new String("helloFooRealStr");
+ }
+
+ public String getString() {
+ return new String("helloFoo");
+ }
+
+ public String getNullString() {
+ return null;
+ }
+
+ public java.util.Date getNullDate() {
+ return null;
+ }
+
+ @Override
+ public String toString() {
+ StringBuilder builder = new StringBuilder();
+ builder.append("TestObject [nullValueAttribute=");
+ builder.append(nullValueAttribute);
+ builder.append("]");
+ return builder.toString();
+ }
+ }
+ // @@ VELOCITY-553
+
+
/**
* Default constructor.
*/
@@ -86,6 +122,7 @@ extends TestCase
ec.addEventHandler(te);
ec.attachToContext( inner );
+ doTestInvalidReferenceEventHandler0(ve, inner);
doTestInvalidReferenceEventHandler1(ve, inner);
doTestInvalidReferenceEventHandler2(ve, inner);
doTestInvalidReferenceEventHandler3(ve, inner);
@@ -103,6 +140,7 @@ extends TestCase
ve.setProperty(RuntimeConstants.EVENTHANDLER_INVALIDREFERENCES, TestEventCartridge.class.getName());
ve.init();
+ doTestInvalidReferenceEventHandler0(ve, null);
doTestInvalidReferenceEventHandler1(ve, null);
doTestInvalidReferenceEventHandler2(ve, null);
doTestInvalidReferenceEventHandler3(ve, null);
@@ -133,11 +171,11 @@ extends TestCase
// show work fine
s = "$tree.Field $tree.field $tree.child.Field";
w = new StringWriter();
- ve.evaluate( context, w, "mystring", s );
+ ve.evaluate(context, w, "mystring", s);
s = "$tree.x $tree.field.x $tree.child.y $tree.child.Field.y";
w = new StringWriter();
- ve.evaluate( context, w, "mystring", s );
+ ve.evaluate(context, w, "mystring", s);
}
@@ -151,8 +189,8 @@ extends TestCase
throws Exception
{
VelocityContext context = new VelocityContext(vc);
- context.put("a1",new Integer(5));
- context.put("a4",new Integer(5));
+ context.put("a1", new Integer(5));
+ context.put("a4", new Integer(5));
context.put("b1","abc");
String s;
@@ -235,7 +273,7 @@ extends TestCase
// normal - should be no calls to handler
String s = "$a1 $a1.intValue() $b1 $b1.length() #set($c1 = '5')";
Writer w = new StringWriter();
- ve.evaluate( context, w, "mystring", s );
+ ve.evaluate(context, w, "mystring", s);
// good object, bad property
s = "$a1.foobar";
@@ -244,7 +282,14 @@ extends TestCase
ve.evaluate( context, w, "mystring", s );
fail("Expected exception.");
} catch (RuntimeException e) {}
-
+
+ // same one inside an #if statement should not fail
+ s = "#if($a1.foobar)yes#{else}no#end";
+ w = new StringWriter();
+ ve.evaluate( context, w, "mystring", s );
+ assertEquals("no",w.toString());
+
+
// bad object, bad property
s = "$a2.foobar";
w = new StringWriter();
@@ -252,7 +297,15 @@ extends TestCase
ve.evaluate( context, w, "mystring", s );
fail("Expected exception.");
} catch (RuntimeException e) {}
-
+
+ // same one inside an #if statement should still fail
+ s = "#if($a2.foobar)yes#{else}no#end";
+ w = new StringWriter();
+ try {
+ ve.evaluate( context, w, "mystring", s );
+ fail("Expected exception.");
+ } catch (RuntimeException e) {}
+
// bad object, no property
s = "$a3";
w = new StringWriter();
@@ -260,7 +313,14 @@ extends TestCase
ve.evaluate( context, w, "mystring", s );
fail("Expected exception.");
} catch (RuntimeException e) {}
-
+
+ // bad object, no property as #if condition should not fail
+ s = "#if($a3)yes#{else}no#end";
+ w = new StringWriter();
+ ve.evaluate( context, w, "mystring", s );
+ result = w.toString();
+ assertEquals("no", result);
+
// good object, bad property; change the value
s = "$a4.foobar";
w = new StringWriter();
@@ -269,8 +329,95 @@ extends TestCase
assertEquals("zzz", result);
}
-
-
+
+ /**
+ * Test invalidGetMethod
+ *
+ * Test behaviour (which should be the same) of
+ * $objRef.myAttribute and $objRef.getMyAttribute()
+ *
+ * @param ve
+ * @param vc
+ * @throws Exception
+ */
+ private void doTestInvalidReferenceEventHandler0(VelocityEngine ve, VelocityContext vc)
+ throws Exception
+ {
+ String result;
+ Writer w;
+ String s;
+ boolean rc;
+
+ VelocityContext context = new VelocityContext(vc);
+ context.put("propertyAccess", new String("lorem ipsum"));
+ context.put("objRef", new TestObject());
+ java.util.ArrayList arrayList = new java.util.ArrayList();
+ arrayList.add("firstOne");
+ arrayList.add(null);
+ java.util.HashMap hashMap = new java.util.HashMap();
+ hashMap.put(41, "41 is not 42");
+
+ context.put("objRefArrayList", arrayList);
+ context.put("objRefHashMap", hashMap);
+
+ // good object, good property (returns non null value)
+ s = "#set($resultVar = $propertyAccess.bytes)"; // -> getBytes()
+ w = new StringWriter();
+ rc = ve.evaluate( context, w, "mystring", s );
+
+ // good object, good property accessor method (returns non null value)
+ s = "#set($resultVar = $propertyAccess.getBytes())"; // -> getBytes()
+ w = new StringWriter();
+ ve.evaluate( context, w, "mystring", s );
+
+ // good object, good property (returns non null value)
+ s = "$objRef.getRealString()";
+ w = new StringWriter();
+ ve.evaluate( context, w, "mystring", s );
+
+ // good object, good property accessor method (returns null value)
+ // No exception shall be thrown, as returning null should be valid
+ s = "$objRef.getNullValueAttribute()";
+ w = new StringWriter();
+ ve.evaluate( context, w, "mystring", s );
+
+ // good object, good property (returns null value)
+ // No exception shall be thrown, as returning null should be valid
+ s = "$objRef.nullValueAttribute"; // -> getNullValueAttribute()
+ w = new StringWriter();
+ ve.evaluate( context, w, "mystring", s );
+
+ // good object, good accessor method which returns a non-null object reference
+ // Test removing a hashmap element which exists
+ s = "$objRefHashMap.remove(41)";
+ w = new StringWriter();
+ ve.evaluate( context, w, "mystring", s );
+
+
+ // good object, good accessor method which returns null
+ // Test removing a hashmap element which DOES NOT exist
+ // Expected behaviour: Returning null as a value should be
+ // OK and not result in an exception
+ s = "$objRefHashMap.remove(42)"; // will return null, as the key does not exist
+ w = new StringWriter();
+ ve.evaluate( context, w, "mystring", s );
+
+ // good object, good method invocation (returns non-null object reference)
+ s = "$objRefArrayList.get(0)"; // element 0 is NOT NULL
+ w = new StringWriter();
+ ve.evaluate( context, w, "mystring", s );
+
+
+ // good object, good method invocation (returns null value)
+ // Expected behaviour: Returning null as a value should be
+ // OK and not result in an exception
+ s = "$objRefArrayList.get(1)"; // element 1 is null
+ w = new StringWriter();
+ ve.evaluate( context, w, "mystring", s );
+
+ }
+
+
/**
* Test assigning the event handlers via properties