RFR: JAXP in JDK8/7u40 : 8021148 Regression in SAXParserImpl in 7u40 b34 (NPE)

[Chris Hegarty]

Joe,

I can see in SAXParserImpl constructor, setFeature0 could throw, leaving 
the fSecurityPropertyMgr uninitialized. There my be other code paths too.

I agree with this change, and Daniel's comments to make both 
fSecurityPropertyMgr fields final. Consider it reviewed ( at least on my 
side ).

-Chris.

On 24/07/2013 07:59, huizhe wang wrote:
>
> On 7/23/2013 11:00 PM, Daniel Fuchs wrote:
>> Hi Joe,
>>
>> This looks reasonable.
>> Out of curiosity - could it be that it was fSAXParser that was null,
>> and not fSecurityPropertyMgr?
>> JAXPSAXParser has a no arg public constructor that could have lead to
>> that...
>
> That was my suspicion as well. I thought NetBeans was referencing the
> internal class directly since the new JAXPSAXParser(this) inside
> SAXParserImpl was the only call in the entire jaxp code. I was therefore
> thinking it really should have been a private class. Of course, once
> NetBeans bugzilla became accessible (it was down at the time), I was
> able to get the error stacktrace.
>
> There is still something strange since XMLReaderManager.getXMLReader
> calls XMLReaderFactory which should have returned SAXParser since it's
> hardcoded default. In a manual test, I could only get a JAXPSAXParser if
> I intentionally set "org.xml.sax.driver" to a "bogus parser". I'm asking
> the NetBeans reporter and haven't heard from him yet.
>
>>
>> I have only one remark:
>>
>> It looks as if fSecurityPropertyMgr could be declared final in both
>> classes - and I think it
>> would be better if it were: it would make it clear that it's never
>> replaced in fSAXParser
>> and that therefore your new code is strictly equivalent to the old in
>> that respect.
>
> Make sense.
>
> Thanks,
> Joe
>
>>
>> best regards,
>>
>> -- daniel
>>
>> On 7/24/13 4:01 AM, huizhe wang wrote:
>>> Hi Lance, Chris,
>>>
>>> Looking at the affected class [1], and the error stack trace [2] , it
>>> appeared that in SAXParserImpl$JAXPSAXParser , line 545,
>>> fSAXParser.fSecurityPropertyMgr is null when setProperty is called.
>>> fSecurityPropertyMgr was instantiated in SAXParserImpl's constructor
>>> after JAXPSAXParser was. I can see a chance where the NetBeans got a
>>> copy of JAXPSAXParser instance with fSecurityPropertyMgr not
>>> initialized. The fix is to remove the reference of
>>> fSecurityPropertyMgr in JAXPSAXParser, and pass it in when
>>> JAXPSAXParser is created.
>>>
>>> Here is the webrev:
>>> http://cr.openjdk.java.net/~joehw/7u40/8021148/webrev/
>>>
>>> [1]
>>> http://cr.openjdk.java.net/~joehw/7u40/8021148/webrev/src/com/sun/org/apache/xerces/internal/jaxp/SAXParserImpl.java.html
>>>
>>> [2]
>>>
>>> Caused by: java.lang.NullPointerException
>>> at
>>> com.sun.org.apache.xerces.internal.jaxp.SAXParserImpl.access$300(SAXParserImpl.java:69)
>>>
>>> at
>>> com.sun.org.apache.xerces.internal.jaxp.SAXParserImpl$JAXPSAXParser.setProperty(SAXParserImpl.java:545)
>>>
>>> at
>>> com.sun.org.apache.xml.internal.utils.XMLReaderManager.getXMLReader(XMLReaderManager.java:161)
>>>
>>> at
>>> com.sun.org.apache.xml.internal.dtm.ref.DTMManagerDefault.getXMLReader(DTMManagerDefault.java:613)
>>>
>>> at
>>> com.sun.org.apache.xalan.internal.xsltc.dom.XSLTCDTMManager.getDTM(XSLTCDTMManager.java:401)
>>>
>>> at
>>> com.sun.org.apache.xalan.internal.xsltc.dom.XSLTCDTMManager.getDTM(XSLTCDTMManager.java:252)
>>>
>>> at
>>> com.sun.org.apache.xalan.internal.xsltc.trax.TransformerImpl.getDOM(TransformerImpl.java:559)
>>>
>>> ... 43 more
>>> ---------
>>> javax.xml.transform.TransformerException: java.lang.NullPointerException
>>> at
>>> com.sun.org.apache.xalan.internal.xsltc.trax.TransformerImpl.getDOM(TransformerImpl.java:581)
>>>
>>> at
>>> com.sun.org.apache.xalan.internal.xsltc.trax.TransformerImpl.transform(TransformerImpl.java:742)
>>>
>>> at
>>> com.sun.org.apache.xalan.internal.xsltc.trax.TransformerImpl.transform(TransformerImpl.java:353)
>>>
>>> at
>>> org.netbeans.spi.project.support.ant.GeneratedFilesHelper$1$1.run(GeneratedFilesHelper.java:332)
>>>
>>>
>>> Thanks,
>>> Joe
>>>
>>
>