gnodet commented on code in PR #1043:
URL: https://github.com/apache/maven/pull/1043#discussion_r1129437408


##########
maven-xml-impl/src/main/java/org/apache/maven/internal/xml/XmlNodeBuilder.java:
##########
@@ -84,14 +81,9 @@ public static XmlNodeImpl build(Reader reader, boolean trim) 
throws XmlPullParse
      */
     public static XmlNodeImpl build(Reader reader, boolean trim, 
InputLocationBuilder locationBuilder)
             throws XmlPullParserException, IOException {
-        try (Reader closeMe = reader) {
-            final XmlPullParser parser = new MXParser();
-            parser.setInput(reader);
-
-            final XmlNodeImpl node = build(parser, trim, locationBuilder);
-
-            return node;
-        }
+        XmlPullParser parser = new MXParser();

Review Comment:
   > Unless very specially coded the XML parser will consume the entire stream. 
It does not stop at the end of the first document.
   > I'm not sure about calling setInput multiple times. It can perhaps read 
from different streams in sequence, but not from the same stream twice, which 
will be fully consumed by a single parse.
   
   Note that MXParser is a pull parser, so it does not consume anything unless 
asked.  The one that asks is that `XmlNodeBuilder`.  The loop is 
[here](https://github.com/apache/maven/blob/4e098a3205e261ce8bc7fdfe30fa6f46928424d6/maven-xml-impl/src/main/java/org/apache/maven/internal/xml/XmlNodeBuilder.java#L124)
 and the parsing ends as soon as the  `END_DOCUMENT` token is received.   In a 
usual scenario, the stream can't really be reused, but it's not because the 
parser consume the entire stream, but rather because the parser buffers the 
data and consumes more that what is needed.  Forcing an unbuffered reader leads 
to a different behaviour.
   
   Fwiw, the following test passes:
   ```
       @Test
       public void testReadMultiDoc() throws Exception {
           String doc = "<?xml version='1.0'?><doc><child>foo</child></doc>";
           StringReader r = new StringReader(doc + doc) {
               @Override
               public int read(char[] cbuf, int off, int len) throws 
IOException {
                   return super.read(cbuf, off, 1);
               }
           };
           XmlNode node1 = XmlNodeBuilder.build(r);
           XmlNode node2 = XmlNodeBuilder.build(r);
           assertEquals(node1, node2);
       }
   ```
   
   Though I do agree, this may not be a common use case, or one that we do run 
into with maven.  Another similar use case with streams would be a 
`ZipInputStream` where the stream can be fully consumed, but should not be 
closed.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]

Reply via email to