RFR: 8169631: [JAXP] XALAN: transformation of XML via namespace-unaware SAX input yields a different result than namespace-unaware DOM input

Fri Nov 25 12:25:24 UTC 2016

Hi Joe,

when trying to finish this up, I had a closer look again.

I found out that it's necessary to look at attributes as well and handle the case where namespace prefixes are used.

Here is a new version of the changeset that passes all jaxp jtreg tests:
http://cr.openjdk.java.net/~clanger/webrevs/8169631.1/

Now I would also throw out prefixes in localName if we don't have namespace support - for both, elements and attributes (in SAX2DTM2.startElement()).

I also removed some qname handling in DOM2SAX as it is not needed with my changes to SAX2DTM2 anymore.

The test case was adopted.

Thanks & best regards
Christoph

> -----Original Message-----
> From: Joe Wang [mailto:huizhe.wang at oracle.com]
> Sent: Dienstag, 22. November 2016 20:11
> To: Langer, Christoph <christoph.langer at sap.com>
> Cc: core-libs-dev at openjdk.java.net
> Subject: Re: RFR: 8169631: [JAXP] XALAN: transformation of XML via
> namespace-unaware SAX input yields a different result than namespace-
> unaware DOM input
> 
> Hi Christoph,
> 
> Once you're able to run all tests, feel free to push the changeset.
> Frank has fixed the Smoke test.
> 
> Thanks,
> Joe
> 
> On 11/18/16, 3:37 PM, Joe Wang wrote:
> > Hi Christoph,
> >
> > Thanks for explaining the customer's dilemma with regard to their
> > legacy process.
> >
> > The testcase I sent you was extracted from an internal SQE smoke test.
> > I agree with your analysis, the 'golden' file which has been in there
> > for over 10 years turns out to be wrong and needs to be updated.
> >
> > To fix this issue, we need to get that test fixed, and the check-in of
> > your patch and that of the test need needs to happen simultaneously.
> > Would you mind wanting for me to go through an internal process to get
> > a patch ready, then we can check in almost at the same time?
> >
> > Best,
> > Joe
> >
> > On 11/18/16, 2:51 PM, Langer, Christoph wrote:
> >> Hi Joe,
> >>
> >> thanks for the feedback.
> >>
> >> I've now understood the testcase that you've sent over and the reason
> >> that it is reporting failure after my fix is that the output of its
> >> transform operation is rather correct now. And before it was wrong. :)
> >> The test is comparing the actual result to a "golden" result file in
> >> the end and both of these were not looking healthy so far. The reason
> >> is that your test is using a namespace unaware SAX Parser as input.
> >> With the current JDK XALAN, you could already modify your smoketest
> >> to use a namespace aware parser.
> >>
> >> E.g. replace lines
> >>
> >> 82        // Use the JAXP way to get an XMLReader
> >> 83        XMLReader reader =
> >> SAXParserFactory.newInstance().newSAXParser().getXMLReader();
> >>
> >> with
> >>
> >> 82        // Use the JAXP way to get an XMLReader
> >> 83        SAXParserFactory spf = SAXParserFactory.newInstance();
> >> 84        spf.setNamespaceAware(true);
> >> 85        XMLReader reader = spf.newSAXParser().getXMLReader();
> >>
> >> ...and you would already get correct results that also DOM input or
> >> Stream Input would yield.
> >>
> >> So, are there other concerns/issues with this fix? Do you want me to
> >> include a transformation operation like the one that your SmokeTest
> >> does to TransformerTest which would illustrate the problem with
> >> namespace unaware SAX input data?
> >>
> >> Best regards
> >> Christoph
> >>
> >>> -----Original Message-----
> >>> From: Joe Wang [mailto:huizhe.wang at oracle.com]
> >>> Sent: Freitag, 18. November 2016 05:53
> >>> To: Langer, Christoph<christoph.langer at sap.com>
> >>> Cc: core-libs-dev at openjdk.java.net
> >>> Subject: Re: RFR: 8169631: [JAXP] XALAN: transformation of XML via
> >>> namespace-unaware SAX input yields a different result than namespace-
> >>> unaware DOM input
> >>>
> >>>
> >>>
> >>> On 11/14/16, 11:43 PM, Langer, Christoph wrote:
> >>>> Hi Joe,
> >>>>
> >>>> thanks for looking.
> >>>>
> >>>> Can you let me know which smoke test is failing? I didn't see
> >>>> issues so far - I
> >>> was merely running the jtreg unittests for transformer.
> >>>
> >>> I sent the test to your mailbox.
> >>>> I stepped back from replacing Vector with ArrayList for
> >>>> m_prefixMappings
> >>> because the code is using methods indexOf() with a start index and
> >>> setSize() for
> >>> which ArrayList has no direct matchings. One could, for sure, add
> >>> some other
> >>> coding, e.g. use ArrayList's subList() method for the index based
> >>> search - but I
> >>> wouldn't want to run the risk of adding a regression here just
> >>> because I
> >>> modified the code and did not well test it. But if you insist, I
> >>> could have another
> >>> look.
> >>>
> >>> Ok, that's fine. subList would do, but setSize may need a bit more
> >>> work.
> >>>
> >>> Best,
> >>> Joe
> >>>> Best regards
> >>>> Christoph
> >>>>
> >>>>> -----Original Message-----
> >>>>> From: Joe Wang [mailto:huizhe.wang at oracle.com]
> >>>>> Sent: Dienstag, 15. November 2016 03:23
> >>>>> To: Langer, Christoph<christoph.langer at sap.com>
> >>>>> Cc: core-libs-dev at openjdk.java.net
> >>>>> Subject: Re: RFR: 8169631: [JAXP] XALAN: transformation of XML via
> >>>>> namespace-unaware SAX input yields a different result than namespace-
> >>>>> unaware DOM input
> >>>>>
> >>>>> Hi Christoph,
> >>>>>
> >>>>> Not all tests have finished yet, but there's at least one failure
> >>>>> in the
> >>>>> smoke test. I'll get to the details when I have time.
> >>>>>
> >>>>> Any reason why m_prefixMappings can not be replaced with ArrayList?
> >>>>>
> >>>>> Thanks,
> >>>>> Joe
> >>>>>
> >>>>> On 11/14/16, 6:10 AM, Langer, Christoph wrote:
> >>>>>> Hi,
> >>>>>>
> >>>>>> please review this fix for bug 8169631.
> >>>>>>
> >>>>>> Bug: https://bugs.openjdk.java.net/browse/JDK-8169631
> >>>>>> Webrev: http://cr.openjdk.java.net/~clanger/webrevs/8169631.0/
> >>>>>>
> >>>>>> When XALAN is handling namespace unaware input, it behaves
> >>>>>> differently
> >>>>> while using SAX input compared to DOM input.
> >>>>>> With both input source types, the class
> >>>>> com.sun.org.apache.xml.internal.dtm.ref.sax2dtm.SAX2DTM2 converts
> SAX
> >>>>> input into a DTM representation for processing by the XALAN
> >>>>> transformer.
> >>> Its
> >>>>> method startElement takes URI, localname and qName as attribute.
> >>>>> In case
> >>> of
> >>>>> missing feature namespaces, startElement and localname can be empty.
> >>>>> However, the function uses the localname value for the call to
> >>>>> m_expandedNameTable.getExpandedTypeID() and further processing. In
> >>>>> the
> >>>>> case where only qName has data, this leads to issues.
> >>>>>> When using DOM input, the class
> >>>>> com.sun.org.apache.xalan.internal.xsltc.trax.DOM2SAX converts the
> DOM
> >>> input
> >>>>> into SAX input. In the case of empty localname, it fills localname
> >>>>> with qname
> >>>>> data. See method getLocalName() [1], called by parse() [2].
> >>>>>> When directly using SAX input, the SAX parser calls the
> >>>>>> startElement()
> >>>>> function on XALAN's handler with empty uri and localname - which
> >>>>> seems
> >>>>> correct, as per the spec.
> >>>>>> Both paths end up in SAX2DTM2's startElement(). So I suggest to
> >>>>>> change
> >>> this
> >>>>> method to handle the case when uri and localname are empty and
> >>>>> then set
> >>>>> qname as localname. Maybe one should even change DOM2SAX's
> >>>>> getLocalName handling to not fill localname with qname in case it
> >>>>> is empty
> >>>>> after SAX2DTM was changed..
> >>>>>> Generally, JavaDoc for SAXSource says that "Attempting to
> >>>>>> transform an
> >>> input
> >>>>> source that is not generated with a namespace-aware parser may
> >>>>> result in
> >>>>> errors." But why not fix some of these :)
> >>>>>> Furthermore I did some cleanups in the code.
> >>>>>>
> >>>>>> Thanks and best regards
> >>>>>> Christoph
> >>>>>>
> >>>>>> [1]
> >>>
> http://hg.openjdk.java.net/jdk9/dev/jaxp/file/71558b38bad7/src/java.xml/shar
> >>>
> >>>
> e/classes/com/sun/org/apache/xalan/internal/xsltc/trax/DOM2SAX.java#l139
> >>>
> >>>>>> [2]
> >>>
> http://hg.openjdk.java.net/jdk9/dev/jaxp/file/71558b38bad7/src/java.xml/shar
> >>>
> >>>
> e/classes/com/sun/org/apache/xalan/internal/xsltc/trax/DOM2SAX.java#l279
> >>>
> >>>>>> [3]
> >>>
> https://docs.oracle.com/javase/8/docs/api/javax/xml/transform/sax/SAXSource
> >>>
> >>>>> .html