JDK 9 RFR of JDK-8149915: enabling validate-annotations feature for xsd schema with annotation causes NPE

[Langer, Christoph]

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.wang at oracle.com] 
Sent: Mittwoch, 17. Februar 2016 00:22
To: Langer, Christoph <christoph.langer at sap.com>
Cc: core-libs-dev at 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.wang at oracle.com]
> Sent: Dienstag, 16. Februar 2016 21:03
> To: Langer, Christoph <christoph.langer at sap.com>
> Cc: core-libs-dev at 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
>>