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

[Langer, Christoph]

Hi Joe,

thanks, looks fine now.

In test/javax/xml/jaxp/unittest/stream/XMLStreamReaderTest/StreamReaderTest.java, the brace in line 63 could still move one line up, but this really is nit-picking ;-)

Best regards
Christoph


> -----Original Message-----
> From: Joe Wang [mailto:huizhe.wang at oracle.com]
> Sent: Donnerstag, 1. Dezember 2016 18:25
> To: Daniel Fuchs <daniel.fuchs at oracle.com>
> Cc: Langer, Christoph <christoph.langer at sap.com>; core-libs-
> dev at openjdk.java.net
> Subject: Re: RFR (JAXP) 8167340: XMLStreamReader.getElementText return
> corrupt content when size of element is > 8192
> 
> 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
> >