RFR (JAXP) 8167340: XMLStreamReader.getElementText return corrupt content when size of element is > 8192

[Daniel Fuchs]

On 01/12/16 17:25, Joe Wang wrote:
> Thanks Christoph, Daniel! I've updated the test, removing the comments.
>
> As for 80 chars line limit, or slightly longer than that is fine with
> me. Given our codebase, it would be an enormous undertaking. I'm
> therefore fine with taking care of only the extremely long ones or any
> that impedes the reviews, that is, side-by-side view.

+1

best regards,

-- daniel

>
> Best,
> Joe
>
> On 12/1/16, 2:25 AM, Daniel Fuchs wrote:
>> Hi Joe,
>>
>> I agree with Christoph's comments below.
>>
>> +1
>>
>> best regards,
>>
>> -- daniel
>>
>> On 01/12/16 07:40, Langer, Christoph wrote:
>>> Hi Joe,
>>>
>>> to me this looks good.
>>>
>>> Did you already remove the cleanups from
>>> http://cr.openjdk.java.net/~joehw/jdk9/8167340/webrev/ ? I can't see
>>> a lot of them any more...
>>>
>>> A few minor points:
>>>
>>> It seems you still have left some debugging code in
>>> test/javax/xml/jaxp/unittest/stream/XMLStreamReaderTest/StreamReaderTest.java:
>>>
>>>
>>> 59         while (xmlStreamReader.hasNext()) {
>>>   60             int event = xmlStreamReader.next();
>>>   61             if (event == XMLStreamConstants.START_ELEMENT) {
>>>   62                 if
>>> (xmlStreamReader.getLocalName().equals("body"))// && bMessage)
>>>
>>> -> remove the && bMessage)
>>>
>>>   63                 {
>>>   64                     String elementText =
>>> xmlStreamReader.getElementText();
>>>   65                     //System.out.println("elementText=" +
>>> elementText + "EndOfElementText");
>>>
>>> -> the commented System.out.println statement should be removed at
>>> all, I suggest
>>>
>>>   66                     // fail if elementText contains "</body>"
>>>   67
>>> Assert.assertTrue(!elementText.contains("</body>"), "Fail:
>>> elementText contains </body>");
>>>   68                 }
>>>   69             }
>>>   70         }
>>>
>>> Other than that I'm wondering if the 80 chars line limit shall be
>>> held which is not completely true in StreamReaderTest.java. But I
>>> know how difficult that is, while keeping the code still readable.
>>> Especially with the long speaking Class and method names ;-)
>>>
>>> Best regards
>>> Christoph
>>>
>>>
>>>> -----Original Message-----
>>>> From: core-libs-dev [mailto:core-libs-dev-bounces at openjdk.java.net]
>>>> On Behalf
>>>> Of Joe Wang
>>>> Sent: Mittwoch, 30. November 2016 22:21
>>>> To: core-libs-dev at openjdk.java.net
>>>> Subject: RFR (JAXP) 8167340: XMLStreamReader.getElementText return
>>>> corrupt
>>>> content when size of element is > 8192
>>>>
>>>> Hi,
>>>>
>>>> Please review an one-line fix and a bunch of cleanups.
>>>>
>>>> The reported issue was caused by a missed setting when the
>>>> XMLStreamReader initializes the XML 1.1 scanner, so while the changeset
>>>> involved 350 lines, the fix is really just the following:
>>>>
>>>> XMLStreamReaderImpl.java:
>>>> @@ -605,11 +604,12 @@
>>>> ...
>>>> +        fEntityScanner.registerListener(fScanner);
>>>>
>>>>
>>>> All other changes are cleanups, warnings. And BTW, warnings in the jaxp
>>>> repo have come down from 5230 in 2015 to 3265, a result of a bit of
>>>> cleanups here and there when we touch those classes. Still a long
>>>> way to
>>>> go, and it shows we may need to have a few dedicated patches.
>>>>
>>>> JBS: https://bugs.openjdk.java.net/browse/JDK-8167340
>>>> webrevs: http://cr.openjdk.java.net/~joehw/jdk9/8167340/webrev/
>>>>
>>>> Thanks,
>>>> Joe
>>