[ 
https://issues.apache.org/jira/browse/OGNL-102?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
 ]

Lukasz Lenart updated OGNL-102:
-------------------------------
    Description: 
In the "for" loop, we did not check the result is null or not. if it's null 
(returned "result = children[i].getValue( context, result );

"), then next time when execute this "result = children[i].getValue( context, 
result );", it will throw one exception. if this method is used frequently, 
then one performance issue here.

suggestion to check the result here like this:

{code:java}
protected Object getValueBody(OgnlContext context, Object source)
        throws OgnlException
{
    Object result = source;

    for(int i = 0, ilast = _children.length - 1; i <= ilast; ++i)
    {

        if(result == null) {
            break;
        }

        boolean handled = false;

        if (i < ilast) {
            if (_children[i] instanceof ASTProperty) {
                ASTProperty propertyNode = (ASTProperty) _children[i];
                int indexType = propertyNode.getIndexedPropertyType(context, 
result);

                if ((indexType != OgnlRuntime.INDEXED_PROPERTY_NONE) && 
(_children[i + 1] instanceof ASTProperty)) {
                    ASTProperty indexNode = (ASTProperty) _children[i + 1];

                    if (indexNode.isIndexedAccess()) {
                        Object index = indexNode.getProperty(context, result);

                        if (index instanceof DynamicSubscript) {
                            if (indexType == OgnlRuntime.INDEXED_PROPERTY_INT) {
                                Object array = propertyNode.getValue(context, 
result);
                                int len = Array.getLength(array);

                                switch(((DynamicSubscript) index).getFlag()) {
                                    case DynamicSubscript.ALL:
                                        result = 
Array.newInstance(array.getClass().getComponentType(), len);
                                        System.arraycopy(array, 0, result, 0, 
len);
                                        handled = true;
                                        i++;
                                        break;
                                    case DynamicSubscript.FIRST:
                                        index = new Integer((len > 0) ? 0 : -1);
                                        break;
                                    case DynamicSubscript.MID:
                                        index = new Integer((len > 0) ? (len / 
2) : -1);
                                        break;
                                    case DynamicSubscript.LAST:
                                        index = new Integer((len > 0) ? (len - 
1) : -1);
                                        break;
                                }
                            } else {
                                if (indexType == 
OgnlRuntime.INDEXED_PROPERTY_OBJECT) { throw new OgnlException(
                                        "DynamicSubscript '" + indexNode
                                        + "' not allowed for object indexed 
property '" + propertyNode
                                        + "'"); }
                            }
                        }
                        if (!handled)
                        {
                            result = OgnlRuntime.getIndexedProperty(context, 
result,
                                                                    
propertyNode.getProperty(context, result).toString(),
                                                                    index);
                            handled = true;
                            i++;
                        }
                    }
                }
            }
        }
        if (!handled)
        {
            result = _children[i].getValue(context, result);
        }
    }
    return result;
}
{code}
 

  was:
In the "for" loop, we did not check the result is null or not. if it's null 
(returned "result = children[i].getValue( context, result );

"), then next time when execute this "result = children[i].getValue( context, 
result );", it will throw one exception. if this method is used frequently, 
then one performance issue here.

suggestion to check the result here like this:


{code:java}
    protected Object getValueBody(OgnlContext context, Object source)
            throws OgnlException
    {
        Object result = source;

        for(int i = 0, ilast = _children.length - 1; i <= ilast; ++i)
        {

            if(result == null) {
                break;
            }

            boolean handled = false;

            if (i < ilast) {
                if (_children[i] instanceof ASTProperty) {
                    ASTProperty propertyNode = (ASTProperty) _children[i];
                    int indexType = 
propertyNode.getIndexedPropertyType(context, result);

                    if ((indexType != OgnlRuntime.INDEXED_PROPERTY_NONE) && 
(_children[i + 1] instanceof ASTProperty)) {
                        ASTProperty indexNode = (ASTProperty) _children[i + 1];

                        if (indexNode.isIndexedAccess()) {
                            Object index = indexNode.getProperty(context, 
result);

                            if (index instanceof DynamicSubscript) {
                                if (indexType == 
OgnlRuntime.INDEXED_PROPERTY_INT) {
                                    Object array = 
propertyNode.getValue(context, result);
                                    int len = Array.getLength(array);

                                    switch(((DynamicSubscript) 
index).getFlag()) {
                                        case DynamicSubscript.ALL:
                                            result = 
Array.newInstance(array.getClass().getComponentType(), len);
                                            System.arraycopy(array, 0, result, 
0, len);
                                            handled = true;
                                            i++;
                                            break;
                                        case DynamicSubscript.FIRST:
                                            index = new Integer((len > 0) ? 0 : 
-1);
                                            break;
                                        case DynamicSubscript.MID:
                                            index = new Integer((len > 0) ? 
(len / 2) : -1);
                                            break;
                                        case DynamicSubscript.LAST:
                                            index = new Integer((len > 0) ? 
(len - 1) : -1);
                                            break;
                                    }
                                } else {
                                    if (indexType == 
OgnlRuntime.INDEXED_PROPERTY_OBJECT) { throw new OgnlException(
                                            "DynamicSubscript '" + indexNode
                                            + "' not allowed for object indexed 
property '" + propertyNode
                                            + "'"); }
                                }
                            }
                            if (!handled) 
                            {
                                result = 
OgnlRuntime.getIndexedProperty(context, result,
                                                                        
propertyNode.getProperty(context, result).toString(),
                                                                        index);
                                handled = true;
                                i++;
                            }
                        }
                    }
                }
            }
            if (!handled)
            {
                result = _children[i].getValue(context, result);
            }
        }
        return result;
    }

{code}
 


> performance for the ASTChain.getValueBody() method.
> ---------------------------------------------------
>
>                 Key: OGNL-102
>                 URL: https://issues.apache.org/jira/browse/OGNL-102
>             Project: Commons OGNL
>          Issue Type: Improvement
>    Affects Versions: 2.7
>            Reporter: Torr
>            Assignee: Jesse Kuhnert
>            Priority: Major
>
> In the "for" loop, we did not check the result is null or not. if it's null 
> (returned "result = children[i].getValue( context, result );
> "), then next time when execute this "result = children[i].getValue( context, 
> result );", it will throw one exception. if this method is used frequently, 
> then one performance issue here.
> suggestion to check the result here like this:
> {code:java}
> protected Object getValueBody(OgnlContext context, Object source)
>         throws OgnlException
> {
>     Object result = source;
>     for(int i = 0, ilast = _children.length - 1; i <= ilast; ++i)
>     {
>         if(result == null) {
>             break;
>         }
>         boolean handled = false;
>         if (i < ilast) {
>             if (_children[i] instanceof ASTProperty) {
>                 ASTProperty propertyNode = (ASTProperty) _children[i];
>                 int indexType = propertyNode.getIndexedPropertyType(context, 
> result);
>                 if ((indexType != OgnlRuntime.INDEXED_PROPERTY_NONE) && 
> (_children[i + 1] instanceof ASTProperty)) {
>                     ASTProperty indexNode = (ASTProperty) _children[i + 1];
>                     if (indexNode.isIndexedAccess()) {
>                         Object index = indexNode.getProperty(context, result);
>                         if (index instanceof DynamicSubscript) {
>                             if (indexType == 
> OgnlRuntime.INDEXED_PROPERTY_INT) {
>                                 Object array = propertyNode.getValue(context, 
> result);
>                                 int len = Array.getLength(array);
>                                 switch(((DynamicSubscript) index).getFlag()) {
>                                     case DynamicSubscript.ALL:
>                                         result = 
> Array.newInstance(array.getClass().getComponentType(), len);
>                                         System.arraycopy(array, 0, result, 0, 
> len);
>                                         handled = true;
>                                         i++;
>                                         break;
>                                     case DynamicSubscript.FIRST:
>                                         index = new Integer((len > 0) ? 0 : 
> -1);
>                                         break;
>                                     case DynamicSubscript.MID:
>                                         index = new Integer((len > 0) ? (len 
> / 2) : -1);
>                                         break;
>                                     case DynamicSubscript.LAST:
>                                         index = new Integer((len > 0) ? (len 
> - 1) : -1);
>                                         break;
>                                 }
>                             } else {
>                                 if (indexType == 
> OgnlRuntime.INDEXED_PROPERTY_OBJECT) { throw new OgnlException(
>                                         "DynamicSubscript '" + indexNode
>                                         + "' not allowed for object indexed 
> property '" + propertyNode
>                                         + "'"); }
>                             }
>                         }
>                         if (!handled)
>                         {
>                             result = OgnlRuntime.getIndexedProperty(context, 
> result,
>                                                                     
> propertyNode.getProperty(context, result).toString(),
>                                                                     index);
>                             handled = true;
>                             i++;
>                         }
>                     }
>                 }
>             }
>         }
>         if (!handled)
>         {
>             result = _children[i].getValue(context, result);
>         }
>     }
>     return result;
> }
> {code}
>  



--
This message was sent by Atlassian Jira
(v8.20.1#820001)

Reply via email to