You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@tomcat.apache.org by bu...@apache.org on 2022/07/05 05:19:08 UTC

[Bug 66161] New: EL change broke String.concat with null

https://bz.apache.org/bugzilla/show_bug.cgi?id=66161

            Bug ID: 66161
           Summary: EL change broke String.concat with null
           Product: Tomcat 9
           Version: 9.0.64
          Hardware: PC
                OS: Linux
            Status: NEW
          Severity: normal
          Priority: P2
         Component: EL
          Assignee: dev@tomcat.apache.org
          Reporter: tza@silbergrau.com
  Target Milestone: -----

Hello, I use Tomcat 9 with JSF. Currently 9.0.59 and wanted to update to
9.0.64. 
Then a Nullpointer occured, so I tried to find out the commit which is related
to our problem. 

Since version 9.0.60 the problem occurs and is still a problem with 9.0.64.
Probably introduced with commit
https://github.com/apache/tomcat/commit/d4050b4cc979302c5f5cc9237609e1564fa367c4
?

The EL behaviour has changed for string.concat() with null values. Probably the
new behaviour is more correct than before, but we are not sure yet if the new
behaviour is expected. So could you please check and let us know, if this is a
bug or desired behaviour?


How to reproduce:
--- Our usecase was like xhtml has an <ui:include src="someother.xhtml> and
sometimes <ui:param name="foo" value="ABC" /> is passed, sometimes foo is
null/not added via ui:param. 
In someother.xhtml we had the following EL: <p:column
id="#{'columnId'.concat(foo)}"
---

But it can be tested with a simple xhtml and 
<h:outputText value="#{'sometext'.concat(variableNotExisting)}" />


With tomcat 9.0.59 passing null resulted in 'sometext'.concat('') [empty
string] -> 'sometext'.

With tomcat 9.0.60 passing null results in 'sometext'.concat(null) which leads
to an Nullpointer in java String.concat()

Using 'sometext' += foo would be nullsafe, but produces 'sometextnull'.

-- 
You are receiving this mail because:
You are the assignee for the bug.
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org


[Bug 66161] EL change broke String.concat with null

Posted by bu...@apache.org.
https://bz.apache.org/bugzilla/show_bug.cgi?id=66161

--- Comment #2 from tza@silbergrau.com ---
Just debugged into it.
The linked commit has changed the behaviour.

 context.convertToType(src, target); 

now triggers our EmptyNullStringResolver and the string gets null. 

Removing the EmptyNullStringResolver is no option for us, since we expect null
and not empty strings set in our beans / tests.

-- 
You are receiving this mail because:
You are the assignee for the bug.
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org


[Bug 66161] EL change broke String.concat with null

Posted by bu...@apache.org.
https://bz.apache.org/bugzilla/show_bug.cgi?id=66161

Mark Thomas <ma...@apache.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|NEW                         |RESOLVED
         Resolution|---                         |INVALID

--- Comment #3 from Mark Thomas <ma...@apache.org> ---
The short answer is that Tomcat 9.0.60 onwards is behaving correctly in this
case.

Unfortunately, the fix introduced in
https://github.com/apache/tomcat/commit/d4050b4cc979302c5f5cc9237609e1564fa367c4
exposed an issue with EmptyNullStringResolver.

For the app's use of beans, the app wants null values to remain null (contrary
to the EL spec).
For the app's use of String.conact(), the app wants null values converted to
empty string (consistent with EL spec).

Having behaviour consistent with EL spec in one case and contrary to the spec
in another is going to require some hoop jumping.

There are lots of ways you could work-around this. My suggestion is to handle
this in EmptyNullStringResolver. If it overrides invoke() with something like
this:

@Override
public Object invoke(ELContext context, Object base, Object method,
        Class<?>[] paramTypes, Object[] params) {
    if ((base instanceof String) && "concat".equals(method) &&
            params.length == 1 && "".equals(params[0])) {
        context.setPropertyResolved(true);
        return base;
    }
    return super.invoke(context, base, method, paramTypes, params);
}

It can treat concat as a special case any bypass the EL type conversion when
call with null. The code above is only lightly tested and intended to give you
an ideas of how you might address this issue.

I am resolving this as invalid since Tomcat's behaviour is spec compliant in
this case.

-- 
You are receiving this mail because:
You are the assignee for the bug.
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org


[Bug 66161] EL change broke String.concat with null

Posted by bu...@apache.org.
https://bz.apache.org/bugzilla/show_bug.cgi?id=66161

--- Comment #1 from tza@silbergrau.com ---
Some additions: maybe the current wrong behaviour (for us) resulted due to
updates and we missed something updating from EL2.x to EL3.0.

And the tomcat update and the resulting error was just a "by-product".


We also set 
        <context-param>
               
<param-name>javax.faces.INTERPRET_EMPTY_STRING_SUBMITTED_VALUES_AS_NULL</param-name>
                <param-value>true</param-value>
        </context-param>

and currently still provide a own EmptyNullStringResolver¹ (which might be the
problem here). Without the Resolver, an empty string is passed to concat. I
will take a look into removing the custom resolver.

¹ see eg. https://balusc.omnifaces.org/2015/10/the-empty-string-madness.html

So this bug simple might be invalid.

-- 
You are receiving this mail because:
You are the assignee for the bug.
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org


[Bug 66161] EL change broke String.concat with null

Posted by bu...@apache.org.
https://bz.apache.org/bugzilla/show_bug.cgi?id=66161

--- Comment #4 from tza@silbergrau.com ---
(In reply to Mark Thomas from comment #3)
> The short answer is that Tomcat 9.0.60 onwards is behaving correctly in this
> case.

Yes, that was my guess as well after debugging.

> There are lots of ways you could work-around this. My suggestion is to
> handle this in EmptyNullStringResolver. If it overrides invoke() with
> something like this:

Thanks for the hint, I'll have a look at that.

-- 
You are receiving this mail because:
You are the assignee for the bug.
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org