looks fine Joe
On Jul 25, 2013, at 2:30 AM, huizhe wang wrote:

> 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.ander...@oracle.com
>> 
> 


Lance Andersen| Principal Member of Technical Staff | +1.781.442.2037
Oracle Java Engineering 
1 Network Drive 
Burlington, MA 01803
lance.ander...@oracle.com

Reply via email to