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

[huizhe wang]

I received test from the NetBeans team. The NetBeans did reference the 
internal JAXPSAXParser directly. This is not a usage supported by the 
spec. I would suggest them remove it and use the default parser instead. 
I'm asking the NetBeans team what are the features they rely on that the 
default parser couldn't provide. In the meantime, we're making this fix 
so that existing NetBeans continue to work properly when 7u40 is used.

Here's an updated webrev (test included):
http://cr.openjdk.java.net/~joehw/7u40/8021148/webrev/

Thanks,
Joe

On 7/24/2013 3:41 AM, Lance Andersen - Oracle wrote:
> Agree with the change and making fSecurityPropertyMgr final
>
> Best
> Lance
> On Jul 24, 2013, at 5:04 AM, chris Hegarty wrote:
>
>> 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
>>>>>
>
>
> Lance Andersen| Principal Member of Technical Staff | +1.781.442.2037
> Oracle Java Engineering
> 1 Network Drive
> Burlington, MA 01803
> Lance.Andersen at oracle.com
>