Mailing-List: contact issues-help@struts.apache.org; run by ezmlm
Precedence: bulk
Reply-To: dev@struts.apache.org
Date: Fri, 12 Jul 2013 14:17:48 +0000 (UTC)
From: "David Greene (JIRA)" <jira@apache.org>
To: issues@struts.apache.org
Message-ID: <JIRA.12657286.1373561069793.39912.1373638668760@arcas>
In-Reply-To: <JIRA.12657286.1373561069793@arcas>
References: <JIRA.12657286.1373561069793@arcas>
Subject: [jira] [Comment Edited] (WW-4138) OgnlTextParser contains a NPE
 when the expression is passed in as null
MIME-Version: 1.0
Content-Type: text/plain; charset=utf-8
Content-Transfer-Encoding: 7bit


    [ https://issues.apache.org/jira/browse/WW-4138?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13706967#comment-13706967 ] 

David Greene edited comment on WW-4138 at 7/12/13 2:17 PM:
-----------------------------------------------------------

The line number is a little bit off of the 2.3.15 compiled class because we have a built-in code formatter.  Here's our autoformatted class (which should be identical sans whitespace to the one in the 2.3.15 release:

{code}
package com.opensymphony.xwork2.util;

import org.apache.commons.lang3.StringUtils;


/**
 * OGNL implementation of {@link TextParser}
 */
public class OgnlTextParser
    implements TextParser
{
    //~ Methods ----------------------------------------------------------------

    public Object evaluate( char                               openChars[],
                            String                             expression,
                            TextParseUtil.ParsedValueEvaluator evaluator,
                            int                                maxLoopCount )
    {
        // deal with the "pure" expressions first!
        //expression = expression.trim();
        Object result = expression;    // = ( expression == null ) ? "" : expression;
        int    pos = 0;

        for ( char open : openChars )
        {
            int loopCount = 1;

            //this creates an implicit StringBuffer and shouldn't be used in the inner loop
            final String lookupChars = open + "{";

            while ( true )
            {
                int start = expression.indexOf( lookupChars, pos );
                if ( start == -1 )
                {
                    loopCount++;
                    start = expression.indexOf( lookupChars );
                }

                if ( loopCount > maxLoopCount )
                {
                    // translateVariables prevent infinite loop / expression recursive evaluation
                    break;
                }

                int  length = expression.length(  );
                int  x = start + 2;
                int  end;
                char c;
                int  count = 1;
                while ( ( start != -1 ) && ( x < length ) && ( count != 0 ) )
                {
                    c = expression.charAt( x++ );
                    if ( c == '{' )
                    {
                        count++;
                    }
                    else if ( c == '}' )
                    {
                        count--;
                    }
                }

                end = x - 1;

                if ( ( start != -1 ) && ( end != -1 ) && ( count == 0 ) )
                {
                    String var = expression.substring( start + 2, end );

                    Object o = evaluator.evaluate( var );

                    String left = expression.substring( 0, start );
                    String right = expression.substring( end + 1 );
                    String middle = null;
                    if ( o != null )
                    {
                        middle = o.toString(  );
                        if ( StringUtils.isEmpty( left ) )
                        {
                            result = o;
                        }
                        else
                        {
                            result = left.concat( middle );
                        }

                        if ( StringUtils.isNotEmpty( right ) )
                        {
                            result = result.toString(  ).concat( right );
                        }

                        expression = left.concat( middle ).concat( right );
                    }
                    else
                    {
                        // the variable doesn't exist, so don't display anything
                        expression = left.concat( right );
                        result     = expression;
                    }

                    pos = ( ( ( left != null ) && ( left.length(  ) > 0 ) )
                            ? ( left.length(  ) - 1 )
                            : 0 ) + ( ( ( middle != null ) && ( middle.length(  ) > 0 ) )
                                      ? ( middle.length(  ) - 1 )
                                      : 0 ) + 1;
                    pos = Math.max( pos, 1 );
                }
                else
                {
                    break;
                }
            }
        }

        return result;
    }
}
{code}
                
      was (Author: trumpetx):
    The line number is a little bit off of the 2.3.15 compiled class because we have a built-in code formatter.  Here's our autoformatted class (which should be identical to the one in the 2.3.15 release:

{code}
package com.opensymphony.xwork2.util;

import org.apache.commons.lang3.StringUtils;


/**
 * OGNL implementation of {@link TextParser}
 */
public class OgnlTextParser
    implements TextParser
{
    //~ Methods ----------------------------------------------------------------

    public Object evaluate( char                               openChars[],
                            String                             expression,
                            TextParseUtil.ParsedValueEvaluator evaluator,
                            int                                maxLoopCount )
    {
        // deal with the "pure" expressions first!
        //expression = expression.trim();
        Object result = expression;    // = ( expression == null ) ? "" : expression;
        int    pos = 0;

        for ( char open : openChars )
        {
            int loopCount = 1;

            //this creates an implicit StringBuffer and shouldn't be used in the inner loop
            final String lookupChars = open + "{";

            while ( true )
            {
                int start = expression.indexOf( lookupChars, pos );
                if ( start == -1 )
                {
                    loopCount++;
                    start = expression.indexOf( lookupChars );
                }

                if ( loopCount > maxLoopCount )
                {
                    // translateVariables prevent infinite loop / expression recursive evaluation
                    break;
                }

                int  length = expression.length(  );
                int  x = start + 2;
                int  end;
                char c;
                int  count = 1;
                while ( ( start != -1 ) && ( x < length ) && ( count != 0 ) )
                {
                    c = expression.charAt( x++ );
                    if ( c == '{' )
                    {
                        count++;
                    }
                    else if ( c == '}' )
                    {
                        count--;
                    }
                }

                end = x - 1;

                if ( ( start != -1 ) && ( end != -1 ) && ( count == 0 ) )
                {
                    String var = expression.substring( start + 2, end );

                    Object o = evaluator.evaluate( var );

                    String left = expression.substring( 0, start );
                    String right = expression.substring( end + 1 );
                    String middle = null;
                    if ( o != null )
                    {
                        middle = o.toString(  );
                        if ( StringUtils.isEmpty( left ) )
                        {
                            result = o;
                        }
                        else
                        {
                            result = left.concat( middle );
                        }

                        if ( StringUtils.isNotEmpty( right ) )
                        {
                            result = result.toString(  ).concat( right );
                        }

                        expression = left.concat( middle ).concat( right );
                    }
                    else
                    {
                        // the variable doesn't exist, so don't display anything
                        expression = left.concat( right );
                        result     = expression;
                    }

                    pos = ( ( ( left != null ) && ( left.length(  ) > 0 ) )
                            ? ( left.length(  ) - 1 )
                            : 0 ) + ( ( ( middle != null ) && ( middle.length(  ) > 0 ) )
                                      ? ( middle.length(  ) - 1 )
                                      : 0 ) + 1;
                    pos = Math.max( pos, 1 );
                }
                else
                {
                    break;
                }
            }
        }

        return result;
    }
}
{code}
                  
> OgnlTextParser contains a NPE when the expression is passed in as null
> ----------------------------------------------------------------------
>
>                 Key: WW-4138
>                 URL: https://issues.apache.org/jira/browse/WW-4138
>             Project: Struts 2
>          Issue Type: Bug
>    Affects Versions: 2.3.15
>            Reporter: David Greene
>             Fix For: 2.3.16
>
>
> Unfortunately, I haven't figured out the exact cause of the issue; however, the fix provided brings our system back to how it used to work.
> When expression is passed in a null, a NPE is thrown @:
> int start = expression.indexOf(lookupChars, pos);
> {code}
>         Object result = expression;
>         int pos = 0;
>         for (char open : openChars) {
>             int loopCount = 1;
>             //this creates an implicit StringBuffer and shouldn't be used in the inner loop
>             final String lookupChars = open + "{";
>             while (true) {
>                 int start = expression.indexOf(lookupChars, pos);
>                 if (start == -1) {
>                     loopCount++;
>                     start = expression.indexOf(lookupChars);
>                 }
> {code}
> Here is the fix I'm using in a locally built class file:
> {code}
>         Object result = expression = (expression == null) ? "" : expression;
>         int pos = 0;
>         for (char open : openChars) {
>             int loopCount = 1;
>             //this creates an implicit StringBuffer and shouldn't be used in the inner loop
>             final String lookupChars = open + "{";
>             while (true) {
>                 int start = expression.indexOf(lookupChars, pos);
>                 if (start == -1) {
>                     loopCount++;
>                     start = expression.indexOf(lookupChars);
>                 }
> {code}
> I can see about 10 different ways to 'fix' the issue, but this keeps the change to a single line since I don't entirely understand the workflow of what's going on.
> FYI, it seems to have something to do with validate="true" on the form and when there are fields to validate which have nested getters, aka:
> <@s.hidden key="myFakePatternHolder.pattern.id"/>

--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators
For more information on JIRA, see: http://www.atlassian.com/software/jira