Bug 54241 - NPE in BodyContentImpl
Summary: NPE in BodyContentImpl
Alias: None
Product: Tomcat 7
Classification: Unclassified
Component: Jasper (show other bugs)
Version: trunk
Hardware: PC All
: P2 major (vote)
Target Milestone: ---
Assignee: Tomcat Developers Mailing List
Depends on:
Reported: 2012-12-04 07:53 UTC by Sheldon Shao
Modified: 2013-01-02 15:59 UTC (History)
0 users

Patch for BodyContentImpl.java (542 bytes, patch)
2012-12-04 07:53 UTC, Sheldon Shao
Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Sheldon Shao 2012-12-04 07:53:25 UTC
Created attachment 29688 [details]
Patch for BodyContentImpl.java

Similar as BUG 35410, there is a NPE when calling BodyContentImpl#write(String) if the String is NULL.
Comment 1 Mark Thomas 2012-12-04 08:25:16 UTC
I have a number of concerns with this proposed change:

1. Writers generally do throw NPEs when called with null.

2. I'm not convinced 35410 should have been fixed since toString() should never return null (else all sorts of standard code patterns break).

3. The patch should ideally include a test case.

4. The proposed patch only addresses one possible NPE of many.
Comment 2 Konstantin Kolinko 2012-12-04 12:38:36 UTC
What is your stacktrace and in what case this issue is triggered?

1. I agree with Mark that Writer.write((String)null) should throw an NPE

2. Regarding bug 35410 I think that the fix is wrong but in another way

Expressions are defined in two places in JSP 2.2,
a) JSP.1.12.3 "Expressions"
b) JSP.9.4.3 "Expressions"

b) says that <%= expr %> is out.print(expr)
and JspWriter.print(String) is defined to print null value as "null", not "".

So using the test from bug 35410 I expect "null null null". The actual result in 7.0.33 is "null null".

The generated java code is
      out.print( test );

I think it should generate
      out.print( String.valueOf(test) );

The behaviour of JspWriterImpl.print(Object) could be "improved" to handle obj.toString() returning null,  but I do not think that such change is needed, because its current behaviour is consistent with PrintWriter.print(Object) which results in an NPE.

My reading is that the spec says that the value of the expression should be coerced to String *before* printing it.
Comment 3 Mark Thomas 2013-01-02 11:58:36 UTC
+1 to Konstantin's analysis. I'll work on a test case and patch based on bug 35410. We could re-open that one but since it is for 5.5.x which is now unsupported I'd rather just deal with it here.
Comment 4 Mark Thomas 2013-01-02 15:59:35 UTC
The proposed patch was not applied and the previous fix for bug 35410 has been reverted. The changes will be in Tomcat 7.0.35 onwards.