JDK 9 RFR of JDK-8149915: enabling validate-annotations feature for xsd schema with annotation causes NPE
Langer, Christoph
christoph.langer at sap.com
Tue Feb 23 23:51:40 UTC 2016
Sorry, this one is the right webrev:
http://cr.openjdk.java.net/~clanger/webrevs/8150470.1/
From: Langer, Christoph
Sent: Dienstag, 23. Februar 2016 21:13
To: 'Aleksej Efimov' <aleksej.efimov at oracle.com>; huizhe wang <huizhe.wang at oracle.com>
Subject: RE: JDK 9 RFR of JDK-8149915: enabling validate-annotations feature for xsd schema with annotation causes NPE
Hi Aleksej, Joe,
sorry, I already had a bad feeling with that line...
Here is a bug: https://bugs.openjdk.java.net/browse/JDK-8150470
And here is a webrev for fix: http://cr.openjdk.java.net/~clanger/webrevs/8150470.0/
Feel free to add more data to the bug and do more tests. I only had the jtreg tests at hand.
Shall I post this in the mailing list?
Best regards
Christoph
From: Aleksej Efimov [mailto:aleksej.efimov at oracle.com]
Sent: Dienstag, 23. Februar 2016 18:26
To: huizhe wang <huizhe.wang at oracle.com<mailto:huizhe.wang at oracle.com>>
Cc: Langer, Christoph <christoph.langer at sap.com<mailto:christoph.langer at sap.com>>
Subject: Re: JDK 9 RFR of JDK-8149915: enabling validate-annotations feature for xsd schema with annotation causes NPE
Sure!
Will do it tomorrow.
Best Regards,
Aleksej
On 02/23/2016 07:50 PM, huizhe wang wrote:
Hi Aleksej,
Thanks for catching the issue and identifying the fix! Once you come back from the holiday, could you submit a review of the fix?
Best,
Joe
On 2/23/2016 8:15 AM, Aleksej Efimov wrote:
Hi again,
Got an update on this issue:
The fix introduced back this one issue: https://bugs.openjdk.java.net/browse/JDK-8021366
The root cause is that Entity.DEFAULT_XMLDECL_BUFFER_SIZE and XMLEntityManager.DEFAULT_XMLDECL_BUFFER_SIZE have different values: 32 and 64. This change erased the JDK-8021366 change (64->32) and introduced back the issue:
- len = fCurrentEntity.DEFAULT_XMLDECL_BUFFER_SIZE;
+ len = DEFAULT_XMLDECL_BUFFER_SIZE;
Tested the reversion of the fix for only this line and JCK test passes now: - len = DEFAULT_XMLDECL_BUFFER_SIZE; + len = fCurrentEntity.DEFAULT_XMLDECL_BUFFER_SIZE; With Best Regards, Aleksej On 02/23/2016 06:46 PM, Aleksej Efimov wrote:
Hi Christoph, Joe, I think we may have a problem with this fix. I'm observing one new JCK test failure with the fix. If the fix is stripped from JDK9 repo - no failure observed. Failed test is api/xsl/conf/copy/copy19.html#copy19 The failure message is:
MultiJVM group agent ID: 1 java version "9-internal" OpenJDK Runtime Environment (build 9-internal+0-2016-02-23-012828.aefimov.9) OpenJDK 64-Bit Server VM (build 9-internal+0-2016-02-23-012828.aefimov.9, mixed mode) Failed to parse as XML. Cause: warning;org.xml.sax.SAXParseException; lineNumber: 2; columnNumber: 10; XML document structures must start and end within the same entity. Parsing as text. Failed to parse as XML. Cause: warning;org.xml.sax.SAXParseException; lineNumber: 1; columnNumber: 53; XML document structures must start and end within the same entity. Parsing as text. mismatch-value;TEXT()len=43;TEXT()len=62;lengths do not match mismatch-value-gold;TEXT(); <?xml version="1.0" encoding="ISO-8859-1"?> mismatch-value-text;TEXT(); <?xml version="1.0" encoding="ISO-8859-1"?><out>abcd\u00e8fgh</out> XHTFileCheckService results were not equal. Expected result: <?xml version="1.0" encoding="ISO-8859-1"?> <out>abcd\u00e8fgh</out> Actual result: <?xml version="1.0" encoding="ISO-8859-1"?><out>abcd\u00e8fgh</out> result: Failed. test cases: 1; all failed; first test case failure: testTransformation test result: Failed. test cases: 1; all failed; first test case failure: testTransformation
Today is a holiday in SPb. Tomorrow I can log a bug to track this failure (new line is missing in actual result output). Best Regards, Aleksej On 02/23/2016 02:27 PM, Langer, Christoph wrote:
Hi Aleksej,
that's great, please go ahead. Sooner or later I would have started this effort, too. But right now I'm working on another fix for the XALAN which I'll post soon :-)
Best regards
Christoph
PS: @Joe: I noticed that in the commit I was not the author of the change but only mentioned in the "Contributed-by" tag. Couldn't you push the change on my behalf but with maintaining me as the author?
-----Original Message-----
From: Aleksej Efimov [mailto:aleksej.efimov at oracle.com]
Sent: Dienstag, 23. Februar 2016 12:21
To: Langer, Christoph <christoph.langer at sap.com><mailto:christoph.langer at sap.com>
Cc: Joe Wang <huizhe.wang at oracle.com><mailto:huizhe.wang at oracle.com>
Subject: Re: JDK 9 RFR of JDK-8149915: enabling validate-annotations feature for xsd schema with annotation causes NPE
Hi Christoph,
I would like to backport your fix to JDK8.
Just wanted to get your confirmation that you're not working on that and
we are not doing the same task =)
With Best Regards,
Aleksej
On 02/18/2016 10:33 PM, Langer, Christoph wrote:
Hi Joe,
I fixed the copyright in http://cr.openjdk.java.net/~clanger/webrevs/8149915.3/<http://cr.openjdk.java.net/%7Eclanger/webrevs/8149915.3/>. So I think I'm done. Can you review and push the change then?
Thanks
Christoph
-----Original Message-----
From: huizhe wang [mailto:huizhe.wang at oracle.com]
Sent: Donnerstag, 18. Februar 2016 19:19
To: Langer, Christoph <christoph.langer at sap.com><mailto:christoph.langer at sap.com>
Cc: core-libs-dev at openjdk.java.net<mailto: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/18/2016 5:15 AM, Langer, Christoph wrote:
Hi Joe,
here is the next version:
http://cr.openjdk.java.net/~clanger/webrevs/8149915.2/<http://cr.openjdk.java.net/%7Eclanger/webrevs/8149915.2/>
Looks good! Thanks.
Copyright year in XSAttributeChecker.java is still incorrect. I meant to
say the year "2016" needs to have a comma, so in this case, it needs to
be "2015, 2016, " rather than "2015, 2016".
Best,
Joe
Hopefully the final one :-)
Yes, it's good to have some cleanups. For the generic initialization,
it's preferable to have no redundant type arguments, for example,
instead of:
protected Stack<Entity> fEntityStack = new Stack<Entity>();
do:
protected Stack<Entity> fEntityStack = new Stack<>();
Some of the "cleanup" in the webrev were reversing <> with added type
argument(s), that would be unnecessary.
I think I did that on all the places that I touched now.
For the obsolete Vector, it would be good to replace it with ArrayList.
There are so many Vector instances in this code that it's out of scope for me now to touch them all...
Copyright year "2016" in XSAttributeChecker needs to be "2016," or else
the script would complain.
For that one I thought that it would look like "2015,2016" if the initial copyright was done in 2015. But I changed it as you suggested.
In XSDHandler, note that there's another call to get SECURITY_MANAGER
towards the end of the reset method (at 3572 in your new version) that
may be removed. You may also consolidate most of those
setFeature/Property statements in one try-catch block if you want.
OK, I did some further refacturing here. However, I've left the try/catch blocks as I think it's the intention to silently catch some exceptions but then try to go on with the next property.
For the test, the @bug tag is still desirable in the general comment
section. For now, it's @bug 8149915. As we add more test to the class in
the future, we'd then append more bug ids, @bug 8149915, xxxxxxx.
Done. I also left the @bug in the comments for the test method.
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/XML11D
TDScannerImpl.java:
/*
* reserved comment block
* DO NOT REMOVE OR ALTER!
*/
Shouldn't there be the standard copyright header or is there some reason
behind it?
The block is used by the licensing script to swap with license
information required by licensees. For code that came from Xerces, we
should keep the block as is with the Apache License. But when we make
changes, we're required to put in the copyright header. Since the
changes you made are cleanup / removing unused fields, it's okay to keep
the block as is.
OK :)
Thanks again for reviewing.
All the Best
Christoph
More information about the core-libs-dev
mailing list