Hi Joe, here's my next version, trying to implement your suggestions: http://cr.openjdk.java.net/~clanger/webrevs/8149915.1/
I also took the chance to do some cleanups and type check fixes. Hope that's ok? I'm also wondering why some files have a copyright header like this, e.g. src/java.xml/share/classes/com/sun/org/apache/xerces/internal/impl/XML11DTDScannerImpl.java: /* * reserved comment block * DO NOT REMOVE OR ALTER! */ Shouldn't there be the standard copyright header or is there some reason behind it? Thanks in advance and best regards Christoph -----Original Message----- From: huizhe wang [mailto:huizhe.w...@oracle.com] Sent: Mittwoch, 17. Februar 2016 00:22 To: Langer, Christoph <christoph.lan...@sap.com> Cc: core-libs-dev@openjdk.java.net Subject: Re: JDK 9 RFR of JDK-8149915: enabling validate-annotations feature for xsd schema with annotation causes NPE On 2/16/2016 2:13 PM, Langer, Christoph wrote: > Hi Joe, > > thanks for your great suggestions. I'll come up with a new webrev tomorrow. > > However, I'm still wondering if the change for the default value of > XML_SECURITY_PROPERTY_MANAGER in XMLDocumentFragmentScannerImpl should be > done anyway as I guess an XMLObject is more expected rather than a String > from other places in the code that could make use of such a default...? Is > that true? You're right, the default value was incorrectly set to the default value of a property manager. "null" would be good in this case since there's no need to create a new instance here. Best, Joe > > Best > Christoph > > -----Original Message----- > From: huizhe wang [mailto:huizhe.w...@oracle.com] > Sent: Dienstag, 16. Februar 2016 21:03 > To: Langer, Christoph <christoph.lan...@sap.com> > Cc: core-libs-dev@openjdk.java.net > Subject: Re: JDK 9 RFR of JDK-8149915: enabling validate-annotations feature > for xsd schema with annotation causes NPE > > Hi Christoph, > > At initialization of XSDHandler (in reset), note there are two local > variables for the security / property manager, change them to instance > variables, then when you need to create annotation validator, set both > on the validator, e.g.: > > fAnnotationValidator.setProperty(SECURITY_MANAGER, > fSecurityManager); > fAnnotationValidator.setProperty(XML_SECURITY_PROPERTY_MANAGER, > fSecurityPropertyMgr); > > That should fix the issue. > > There are a bunch of rawtypes/unchecked warnings in this class, bonus > but not required :-) > > For the test, in the past (jaxp standalone), we've created tests for > each issue, nowadays we'd prefer to consolidate them. We don't have > "SchemaTest" yet, so you may name the test "SchemaTest", then for the > @summary, make it general, e.g. "Test Schema creation", and move the > specific comment to the test case and make it specific to the issue: > > /* > * @bug 8149915 > * Verifies that the annotation validator is initialized with the > security manager for schema > * creation with > http://apache.org/xml/features/validate-annotations=true. > */ > > @Test > public void testValidation() throws Exception { > > > The test is new, the copyright year would be "2016," instead of "2014, > 2016,". > > Best, > Joe > > On 2/16/2016 4:49 AM, Langer, Christoph wrote: >> Hi, >> >> can you please review the following change for JDK-8149915: >> >> http://cr.openjdk.java.net/~clanger/webrevs/8149915.0/ >> >> I'm not sure if it is the right thing to set the default for >> XML_SECURITY_PROPERTY_MANAGER to a new XMLSecurityPropertyManager() object >> instead of the String "all". But it must not be a String, I think. >> >> And also I don't know if my approach to add a field "fSecurityManager" to >> XML11Configuration. >> >> Please advise and let me know how I can do it better. >> >> Thanks in advance and best regards >> Christoph >>
