JDK-8149915 -> Re: JDK 9 RFR of JDK-8149915: enabling validate-annotations feature for xsd schema with annotation causes NPE
huizhe wang
huizhe.wang at oracle.com
Wed Feb 24 00:06:58 UTC 2016
The change looks good.
Thanks Aleksej, again, for catching the issue. For future changes, we'll
need to make sure we run the JCK tests.
Best,
Joe
On 2/23/2016 3:51 PM, Langer, Christoph wrote:
>
> Sorry, this one is the right webrev:
>
> http://cr.openjdk.java.net/~clanger/webrevs/8150470.1/
> <http://cr.openjdk.java.net/%7Eclanger/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/
> <http://cr.openjdk.java.net/%7Eclanger/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
> <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 inhttp://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