Fixed #58

FasterXML · Jul 15, 2019 · 17c6fb6 · 17c6fb6
1 parent 01b0c1d
commit 17c6fb6
Show file tree

Hide file tree

Showing 5 changed files with 230 additions and 23 deletions.
diff --git a/release-notes/VERSION b/release-notes/VERSION
@@ -4,10 +4,14 @@ Project: woodstox
 === Releases ===
 ------------------------------------------------------------------------
 
+6.0.0 (not yet released)
+
+#78: Shade MSV dependency
+
 5.3.0 (not yet released)
 
+#58: Reading comment comments after Root Element closed sometimes throws `NullPointerException`
 #61: Add support for `XMLConstants.FEATURE_SECURE_PROCESSING` via SAX/Stax factories
-#78: Shade MSV dependency
 
 5.2.1 (14-May-2019)
 

diff --git a/src/main/java/com/ctc/wstx/sr/BasicStreamReader.java b/src/main/java/com/ctc/wstx/sr/BasicStreamReader.java
@@ -2962,7 +2962,7 @@ && readCDataSecondary(mCfgLazyParsing
     }
 
     /**
-     * Method called when advacing stream past the end tag that closes
+     * Method called when advancing stream past the end tag that closes
      * the root element of the open document.
      * Document can be either the singular one, in regular mode, or one of
      * possibly multiple, in multi-doc mode: this method is never called
@@ -2987,10 +2987,8 @@ private int closeContentTree()
         if (mSymbols.isDirty()) {
             mOwner.updateSymbolTable(mSymbols);
         }
-        /* May be able to recycle, but not certain; and
-         * definitely can not just clean contents (may
-         * contain space(s) read)
-         */
+        // May be able to recycle, but not certain; and definitely can not just
+        // clean contents (may contain space(s) read)
         mTextBuffer.recycle(false);
         return mCurrToken;
     }
@@ -3886,9 +3884,8 @@ private void readComment()
                 // Ok; need to get '->', can not get '--'
 
                 if ((ptr + 1) >= inputLen) {
-                    /* Can't check next 2, let's push '-' back, for rest of
-                     * code to take care of
-                     */
+                    // Can't check next 2, let's push '-' back, for rest of
+                    // code to take care of
                     --ptr;
                     break;
                 }
@@ -3907,7 +3904,6 @@ private void readComment()
                 return;
             }
         }
-
         mInputPtr = ptr;
         mTextBuffer.resetWithCopy(inputBuf, start, ptr-start);
         readComment2(mTextBuffer);

diff --git a/src/main/java/com/ctc/wstx/util/TextBuffer.java b/src/main/java/com/ctc/wstx/util/TextBuffer.java
@@ -183,15 +183,12 @@ public void recycle(boolean force)
     {
         if (mConfig != null && mCurrentSegment != null) {
             if (force) {
-                /* If we are allowed to wipe out all existing data, it's
-                 * quite easy; we'll just wipe out contents, and return
-                 * biggest buffer:
-                 */
+                // If we are allowed to wipe out all existing data, it's quite easy;
+                // we'll just wipe out contents, and return biggest buffer:
                 resetWithEmpty();
             } else {
-                /* But if there's non-shared data (ie. buffer is still
-                 * in use), can't return it yet:
-                 */
+                // But if there's non-shared data (i.e. buffer is still in use),
+                // can't return it yet:
                 if (mInputStart < 0 && (mSegmentSize + mCurrentSize) > 0) {
                     return;
                 }
@@ -202,7 +199,6 @@ public void recycle(boolean force)
                     mSegmentSize = 0;
                 }
             }
-
             char[] buf = mCurrentSegment;
             mCurrentSegment = null;
             mConfig.freeMediumCBuffer(buf);
@@ -282,12 +278,15 @@ public void resetWithCopy(char[] buf, int start, int len)
         // And then reset internal input buffers, if necessary:
         if (mHasSegments) {
             clearSegments();
-        } else {
-            if (mCurrentSegment == null) {
-                mCurrentSegment = allocBuffer(len);
-            }
-            mCurrentSize = mSegmentSize = 0;
         }
+
+        // 14-Jul-2019, tatu: As per [woodstox-core#58] there are cases
+        //   in which current segment has been recycled and needs to be restored
+        if (mCurrentSegment == null) {
+            mCurrentSegment = allocBuffer(len);
+        }
+        mCurrentSize = mSegmentSize = 0;
+
         append(buf, start, len);
     }
 

diff --git a/src/test/java/org/codehaus/stax/test/stream/TestCommentRead.java b/src/test/java/org/codehaus/stax/test/stream/TestCommentRead.java
@@ -1,5 +1,7 @@
 package org.codehaus.stax.test.stream;
 
+import java.io.InputStream;
+
 import javax.xml.stream.*;
 
 /**
@@ -23,6 +25,24 @@ public void testValidComments()
         streamThrough(getReader(XML, false));
     }
 
+    // for [woodstox-58]: comment read outside main XML Ntree, after long (enough) CHARACTERS
+    // (probably also affect PIs)
+    public void testIssue58CommentRead() throws Exception
+    {
+        XMLInputFactory f = getNewInputFactory();
+        setCoalescing(f, true);
+        InputStream in = getClass().getResource("issue58.xml").openStream();
+        XMLStreamReader r = f.createXMLStreamReader(in);
+
+        // starts with couple of comments:
+        assertTokenType(COMMENT, r.next());
+        r.getText();
+        // but should be enough to stream and access contents
+        streamThrough(r);
+
+        in.close();
+    }
+
     /**
      * Method that checks properties of COMMENT
      * returned by the stream reader are correct according to StAX specs.