Hi Joe,
Looks good.
Small nit: you could keep fSecurityPropertyManager final by using:
if (spm != null) {
fSecurityPropertyManager = spm;
} else {
fSecurityPropertyManager = new ...
...
}
Don't feel obliged to do it though - your fix is good as it is.
best regards,
-- daniel
On 7/25/13 8: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
