tomcat-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Mark Thomas <ma...@apache.org>
Subject Re: svn commit: r1676393 - in /tomcat/trunk: java/org/apache/el/parser/AstValue.java test/org/apache/el/TestMethodExpressionImpl.java
Date Tue, 28 Apr 2015 21:26:31 GMT
On 28/04/2015 22:21, Christopher Schultz wrote:
> Mark,
> 
> On 4/27/15 7:01 PM, markt@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-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org


Mime
View raw message