On 28/04/2015 22:21, Christopher Schultz wrote: > Mark, > > On 4/27/15 7:01 PM, ma...@apache.org wrote: >> Author: markt >> Date: Mon Apr 27 23:01:16 2015 >> New Revision: 1676393 >> >> URL: http://svn.apache.org/r1676393 >> Log: >> Add some comments to clarify behaviour. >> Review by schultz re object allocation >> >> Modified: >> tomcat/trunk/java/org/apache/el/parser/AstValue.java >> tomcat/trunk/test/org/apache/el/TestMethodExpressionImpl.java >> >> Modified: tomcat/trunk/java/org/apache/el/parser/AstValue.java >> URL: >> http://svn.apache.org/viewvc/tomcat/trunk/java/org/apache/el/parser/AstValue.java?rev=1676393&r1=1676392&r2=1676393&view=diff >> ============================================================================== >> --- tomcat/trunk/java/org/apache/el/parser/AstValue.java (original) >> +++ tomcat/trunk/java/org/apache/el/parser/AstValue.java Mon Apr 27 23:01:16 >> 2015 >> @@ -41,6 +41,9 @@ import org.apache.el.util.ReflectionUtil >> */ >> public final class AstValue extends SimpleNode { >> >> + private static final Object[] EMPTY_ARRAY = new Object[0]; >> + private static final Object[] ARRAY_OF_SINGLE_NULL = new Object[1]; >> + >> protected static class Target { >> protected Object base; >> >> @@ -263,7 +266,8 @@ public final class AstValue extends Simp >> private Object[] convertArgs(EvaluationContext ctx, Object[] src, >> Method m) { >> Class<?>[] types = m.getParameterTypes(); >> if (types.length == 0) { >> - return new Object[0]; >> + // Treated as if parameters have been provided so src is ignored >> + return EMPTY_ARRAY; >> } >> >> int paramCount = types.length; >> @@ -271,23 +275,24 @@ public final class AstValue extends Simp >> if (m.isVarArgs() && paramCount > 1 && (src == null || paramCount > >> src.length) || >> !m.isVarArgs() && (paramCount > 0 && src == null || >> src != null && src.length != paramCount)) { >> - String inputParamCount = null; >> + String srcCount = null; >> if (src != null) { >> - inputParamCount = Integer.toString(src.length); >> + srcCount = Integer.toString(src.length); >> } >> String msg; >> if (m.isVarArgs()) { >> msg = MessageFactory.get("error.invoke.tooFewParams", >> - m.getName(), inputParamCount, >> Integer.toString(paramCount)); >> + m.getName(), srcCount, >> Integer.toString(paramCount)); >> } else { >> msg = MessageFactory.get("error.invoke.wrongParams", >> - m.getName(), inputParamCount, >> Integer.toString(paramCount)); >> + m.getName(), srcCount, >> Integer.toString(paramCount)); >> } >> throw new IllegalArgumentException(msg); >> } >> >> if (src == null) { >> - return new Object[1]; >> + // Must be a varargs method with a single parameter. >> + return ARRAY_OF_SINGLE_NULL; > > Is this safe? I'm not sure of the scope of this array, but client code > could potentially mutate the contents. We fully expect > ARRAY_OF_SINGLE_NULL[0] to be == null, but some other code could change > it and it would probably cause a huge mess. > > I like the use of a flyweight here, but if that object ever gets out of > our control, it could be poisoned.
Untrusted apps are a minority use case but they do exist. I think you are right and I'll use a new array every time. Mark --------------------------------------------------------------------- To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org For additional commands, e-mail: dev-h...@tomcat.apache.org
