Bug 54370 - NPE mapping method in EL
Summary: NPE mapping method in EL
Status: RESOLVED FIXED
Alias: None
Product: Tomcat 7
Classification: Unclassified
Component: Jasper (show other bugs)
Version: trunk
Hardware: PC Linux
: P2 normal (vote)
Target Milestone: ---
Assignee: Tomcat Developers Mailing List
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2013-01-03 14:20 UTC by Remy Maucherat
Modified: 2013-01-03 17:22 UTC (History)
0 users



Attachments
Patch (735 bytes, text/plain)
2013-01-03 14:20 UTC, Remy Maucherat
Details

Note You need to log in before you can comment on or make changes to this bug.
Description Remy Maucherat 2013-01-03 14:20:38 UTC
Created attachment 29808 [details]
Patch

NPE when using null parameters in some cases: https://issues.jboss.org/browse/AS7-3177

I did put a workaround for it, but I don't know how legitimate it is. It looks a bit similar to the NPE with a null toString that got reverted, where a null object is used as an argument.
Comment 1 Mark Thomas 2013-01-03 14:39:47 UTC
I think you have identified a problem but I'm not sure the proposed solution is correct. Treating a null as an exact match for a parameter seems wrong. I think it needs to be treated as a possible match. I'll work on some test cases.
Comment 2 Remy Maucherat 2013-01-03 14:55:00 UTC
(In reply to comment #1)
> I think you have identified a problem but I'm not sure the proposed solution
> is correct. Treating a null as an exact match for a parameter seems wrong. I
> think it needs to be treated as a possible match. I'll work on some test
> cases.

I think it is suspicious too [like many one line patches], but even if it goes through ELSupport.coerceToType, I think null looks like it will always end up matching the expected type (unless I missed something).
Comment 3 Mark Thomas 2013-01-03 15:39:59 UTC
It is the exact match part that is the problem. As soon as an exact match is find, Tomcat stops looking. With a null parameter type it needs to keep looking as it is only a possible match. Tomcat needs to ensure it is the only possible match and that there aren't ambiguous matches.

I fixed this in trunk and 7.0.x and it will be included in 7.0.35 onwards.
Comment 4 Remy Maucherat 2013-01-03 17:22:59 UTC
Thanks, very nice improvement.