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

[Joe Wang]

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.

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
>