Bug 54242 - NPE in tagPlugins:ForEach
NPE in tagPlugins:ForEach
Status: RESOLVED FIXED
Product: Tomcat 7
Classification: Unclassified
Component: Jasper
trunk
PC All
: P2 normal (vote)
: ---
Assigned To: Tomcat Developers Mailing List
:
Depends on:
Blocks: 54314
  Show dependency tree
 
Reported: 2012-12-04 08:12 UTC by Sheldon Shao
Modified: 2013-01-02 16:27 UTC (History)
0 users



Attachments
Patch for ForEach (809 bytes, patch)
2012-12-04 08:12 UTC, Sheldon Shao
Details | Diff
New patch, upgraded based on Konstantin's suggestion (813 bytes, patch)
2012-12-06 06:22 UTC, Sheldon Shao
Details | Diff
Test case for "ForEach" (2.96 KB, text/plain)
2012-12-06 06:23 UTC, Sheldon Shao
Details
Helper class for testing purpose (4.34 KB, text/plain)
2012-12-06 06:24 UTC, Sheldon Shao
Details
Helper class for testing purpose (5.28 KB, application/octet-stream)
2012-12-17 06:54 UTC, Sheldon Shao
Details

Note You need to log in before you can comment on or make changes to this bug.
Description Sheldon Shao 2012-12-04 08:12:05 UTC
Created attachment 29689 [details]
Patch for ForEach

There is not NULL pointer handling for iterator in ForEach.
If the iterator is NULL, there is a NPE.
Comment 1 Mark Thomas 2012-12-04 08:28:42 UTC
The JSTL spec does indeed require that null is treated like an empty collection.

The patch does not include a test case.
Comment 2 Konstantin Kolinko 2012-12-04 13:23:23 UTC
I think generating an "if(itemsV != null){ }" clause around the whole iteration loop will be more effective.  An iterator is a temporary object that is GC'ed.
Comment 3 Sheldon Shao 2012-12-06 06:22:12 UTC
Created attachment 29709 [details]
New patch, upgraded based on Konstantin's suggestion
Comment 4 Sheldon Shao 2012-12-06 06:23:05 UTC
Created attachment 29710 [details]
Test case for "ForEach"
Comment 5 Sheldon Shao 2012-12-06 06:24:02 UTC
Created attachment 29711 [details]
Helper class for testing purpose
Comment 6 Sheldon Shao 2012-12-06 06:24:42 UTC
Test case has been included.
Thanks

(In reply to comment #1)
> The JSTL spec does indeed require that null is treated like an empty
> collection.
> 
> The patch does not include a test case.
Comment 7 Sheldon Shao 2012-12-17 06:54:40 UTC
Created attachment 29772 [details]
Helper class for testing purpose
Comment 8 Mark Thomas 2013-01-02 16:27:24 UTC
Thanks for the report and the patches.

I have applied the updated patch to fix the issue to trunk and 7.0.x. It will be included in 7.0.35 onwards.

I used a different test case based on the existing test cases for the JSTL tag plug-ins. Those test cases only work on Tomcat 8 because they depend on the new resources implementation but my preference for test cases like these is - where possible - to use a real web application.