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

Joe Wang huizhe.wang at oracle.com
Mon Nov 28 19:05:33 UTC 2016 

Hi Christoph,

The changes look good. I also run the other tests (smoke test and etc.), 
and they all passed.

Best,
Joe

On 11/25/16, 4:25 AM, Langer, Christoph wrote:
> 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

More information about the core-libs-dev mailing list