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:[email protected]] > Sent: Dienstag, 22. November 2016 20:11 > To: Langer, Christoph <[email protected]> > Cc: [email protected] > 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:[email protected]] > >>> Sent: Freitag, 18. November 2016 05:53 > >>> To: Langer, Christoph<[email protected]> > >>> Cc: [email protected] > >>> 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:[email protected]] > >>>>> Sent: Dienstag, 15. November 2016 03:23 > >>>>> To: Langer, Christoph<[email protected]> > >>>>> Cc: [email protected] > >>>>> 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
