RFR: 8169772: [JAXP] XALAN: Transformation of DOM with null valued text node causes NPE

[Langer, Christoph]

Hi Joe,

thanks for the feedback.

I've created a new version of the webrev working in your suggestions, adding some further formatting cleanups in the files and I also moved a small refactoring in TransformerTest.java to this changeset.

http://cr.openjdk.java.net/~clanger/webrevs/8169772.1/

>From my end this one is ready for pushing - waiting for your final go.

Best regards
Christoph


From: Joe Wang [mailto:huizhe.wang at oracle.com]
Sent: Freitag, 18. November 2016 07:36
To: Langer, Christoph <christoph.langer at sap.com>
Cc: core-libs-dev at openjdk.java.net
Subject: Re: RFR: 8169772: [JAXP] XALAN: Transformation of DOM with null valued text node causes NPE

Looks fine.

License header: in general, don't change / add Year if there's no material change, removing the legacy $Id field in EmptySerializer.java for example, does not constitute a change to the code, so just keep the original year (see below).

     The initial years for the classes:
            EmptySerializer.java 2012
            SerializerBase.java 2012
            ToSAXHandler.java none (meaning if you make changes to this class, just add 2016)
            ToStream.java 2006
            ToUnknownStream.java 2007
            XSLOutputAttributes.java none (so keep the original "DO NOT REMOVE OR ALTER!" block)

Thanks,
Joe

On 11/16/16, 6:22 AM, Langer, Christoph wrote:

Hi,



please review another XALAN fix.



The Serializer should be able to handle text nodes with null input. There has already been some discussion here: http://mail.openjdk.java.net/pipermail/core-libs-dev/2016-November/044567.html



Bug: https://bugs.openjdk.java.net/browse/JDK-8169772

Webrev: http://cr.openjdk.java.net/~clanger/webrevs/8169772.0/



The actual fix is in ToUnknownStream.java, method "public void characters(String chars) throws SAXException". I don't know if one should even directly return after chars being null or of size() 0. The rest of this change is cleanups and a test case.



Thanks for reviewing.



Best regards

Christoph