XMLEventReader.getElementText() properly checks for start element

Issue: SPR-17233

spring-core/src/main/java/org/springframework/util/xml/AbstractXMLEventReader.java

@@ -1,5 +1,5 @@
 /*
- * Copyright 2002-2016 the original author or authors.
+ * Copyright 2002-2018 the original author or authors.
  *
  * Licensed under the Apache License, Version 2.0 (the "License");
  * you may not use this file except in compliance with the License.
@@ -18,18 +18,15 @@ package org.springframework.util.xml;
 
 import java.util.NoSuchElementException;
 import javax.xml.stream.XMLEventReader;
-import javax.xml.stream.XMLStreamConstants;
 import javax.xml.stream.XMLStreamException;
-import javax.xml.stream.events.Characters;
-import javax.xml.stream.events.XMLEvent;
 
-import org.springframework.lang.Nullable;
 import org.springframework.util.ClassUtils;
 
 /**
  * Abstract base class for {@code XMLEventReader}s.
  *
  * @author Arjen Poutsma
+ * @author Juergen Hoeller
  * @since 5.0
  */
 abstract class AbstractXMLEventReader implements XMLEventReader {
@@ -43,7 +40,7 @@ abstract class AbstractXMLEventReader implements XMLEventReader {
 			return nextEvent();
 		}
 		catch (XMLStreamException ex) {
-			throw new NoSuchElementException();
+			throw new NoSuchElementException(ex.getMessage());
 		}
 	}
 
@@ -53,64 +50,9 @@ abstract class AbstractXMLEventReader implements XMLEventReader {
 				"remove not supported on " + ClassUtils.getShortName(getClass()));
 	}
 
-	@Override
-	public String getElementText() throws XMLStreamException {
-		checkIfClosed();
-		if (!peek().isStartElement()) {
-			throw new XMLStreamException("Not at START_ELEMENT");
-		}
-
-		StringBuilder builder = new StringBuilder();
-		while (true) {
-			XMLEvent event = nextEvent();
-			if (event.isEndElement()) {
-				break;
-			}
-			else if (!event.isCharacters()) {
-				throw new XMLStreamException(
-						"Unexpected event [" + event + "] in getElementText()");
-			}
-			Characters characters = event.asCharacters();
-			if (!characters.isIgnorableWhiteSpace()) {
-				builder.append(event.asCharacters().getData());
-			}
-		}
-		return builder.toString();
-	}
-
-	@Override
-	@Nullable
-	public XMLEvent nextTag() throws XMLStreamException {
-		checkIfClosed();
-		while (true) {
-			XMLEvent event = nextEvent();
-			switch (event.getEventType()) {
-				case XMLStreamConstants.START_ELEMENT:
-				case XMLStreamConstants.END_ELEMENT:
-					return event;
-				case XMLStreamConstants.END_DOCUMENT:
-					return null;
-				case XMLStreamConstants.SPACE:
-				case XMLStreamConstants.COMMENT:
-				case XMLStreamConstants.PROCESSING_INSTRUCTION:
-					continue;
-				case XMLStreamConstants.CDATA:
-				case XMLStreamConstants.CHARACTERS:
-					if (!event.asCharacters().isWhiteSpace()) {
-						throw new XMLStreamException(
-								"Non-ignorable whitespace CDATA or CHARACTERS event in nextTag()");
-					}
-					break;
-				default:
-					throw new XMLStreamException("Received event [" + event +
-							"], instead of START_ELEMENT or END_ELEMENT.");
-			}
-		}
-	}
-
 	/**
-	 * Throws an {@code IllegalArgumentException} when called.
+	 * This implementation throws an {@code IllegalArgumentException} for any property.
-	 * @throws IllegalArgumentException when called.
+	 * @throws IllegalArgumentException when called
 	 */
 	@Override
 	public Object getProperty(String name) throws IllegalArgumentException {
@@ -123,21 +65,12 @@ abstract class AbstractXMLEventReader implements XMLEventReader {
 	}
 
 	/**
-	 * Returns {@code true} if closed; {@code false} otherwise.
+	 * Check if the reader is closed, and throws a {@code XMLStreamException} if so.
-	 * @see #close()
-	 */
-	protected boolean isClosed() {
-		return this.closed;
-	}
-
-	/**
-	 * Checks if the reader is closed, and throws a {@code XMLStreamException} if so.
 	 * @throws XMLStreamException if the reader is closed
 	 * @see #close()
-	 * @see #isClosed()
 	 */
 	protected void checkIfClosed() throws XMLStreamException {
-		if (isClosed()) {
+		if (this.closed) {
 			throw new XMLStreamException("XMLEventReader has been closed");
 		}
 	}

spring-core/src/main/java/org/springframework/util/xml/ListBasedXMLEventReader.java

@@ -19,6 +19,9 @@ package org.springframework.util.xml;
 import java.util.ArrayList;
 import java.util.List;
 import java.util.NoSuchElementException;
+import javax.xml.stream.XMLStreamConstants;
+import javax.xml.stream.XMLStreamException;
+import javax.xml.stream.events.Characters;
 import javax.xml.stream.events.XMLEvent;
 
 import org.springframework.lang.Nullable;
@@ -29,12 +32,16 @@ import org.springframework.util.Assert;
  * of {@link XMLEvent} elements.
  *
  * @author Arjen Poutsma
+ * @author Juergen Hoeller
  * @since 5.0
  */
 class ListBasedXMLEventReader extends AbstractXMLEventReader {
 
 	private final List<XMLEvent> events;
 
+	@Nullable
+	private XMLEvent currentEvent;
+
 	private int cursor = 0;
 
 
@@ -46,13 +53,15 @@ class ListBasedXMLEventReader extends AbstractXMLEventReader {
 
 	@Override
 	public boolean hasNext() {
-		return (this.cursor != this.events.size());
+		return (this.cursor < this.events.size());
 	}
 
 	@Override
 	public XMLEvent nextEvent() {
-		if (this.cursor < this.events.size()) {
+		if (hasNext()) {
-			return this.events.get(this.cursor++);
+			this.currentEvent = this.events.get(this.cursor);
+			this.cursor++;
+			return this.currentEvent;
 		}
 		else {
 			throw new NoSuchElementException();
@@ -62,7 +71,7 @@ class ListBasedXMLEventReader extends AbstractXMLEventReader {
 	@Override
 	@Nullable
 	public XMLEvent peek() {
-		if (this.cursor < this.events.size()) {
+		if (hasNext()) {
 			return this.events.get(this.cursor);
 		}
 		else {
@@ -70,6 +79,60 @@ class ListBasedXMLEventReader extends AbstractXMLEventReader {
 		}
 	}
 
+	@Override
+	public String getElementText() throws XMLStreamException {
+		checkIfClosed();
+		if (this.currentEvent == null || !this.currentEvent.isStartElement()) {
+			throw new XMLStreamException("Not at START_ELEMENT: " + this.currentEvent);
+		}
+
+		StringBuilder builder = new StringBuilder();
+		while (true) {
+			XMLEvent event = nextEvent();
+			if (event.isEndElement()) {
+				break;
+			}
+			else if (!event.isCharacters()) {
+				throw new XMLStreamException("Unexpected non-text event: " + event);
+			}
+			Characters characters = event.asCharacters();
+			if (!characters.isIgnorableWhiteSpace()) {
+				builder.append(event.asCharacters().getData());
+			}
+		}
+		return builder.toString();
+	}
+
+	@Override
+	@Nullable
+	public XMLEvent nextTag() throws XMLStreamException {
+		checkIfClosed();
+
+		while (true) {
+			XMLEvent event = nextEvent();
+			switch (event.getEventType()) {
+				case XMLStreamConstants.START_ELEMENT:
+				case XMLStreamConstants.END_ELEMENT:
+					return event;
+				case XMLStreamConstants.END_DOCUMENT:
+					return null;
+				case XMLStreamConstants.SPACE:
+				case XMLStreamConstants.COMMENT:
+				case XMLStreamConstants.PROCESSING_INSTRUCTION:
+					continue;
+				case XMLStreamConstants.CDATA:
+				case XMLStreamConstants.CHARACTERS:
+					if (!event.asCharacters().isWhiteSpace()) {
+						throw new XMLStreamException(
+								"Non-ignorable whitespace CDATA or CHARACTERS event: " + event);
+					}
+					break;
+				default:
+					throw new XMLStreamException("Expected START_ELEMENT or END_ELEMENT: " + event);
+			}
+		}
+	}
+
 	@Override
 	public void close() {
 		super.close();

spring-core/src/test/java/org/springframework/util/xml/ListBasedXMLEventReaderTests.java

@@ -16,9 +16,6 @@
 
 package org.springframework.util.xml;
 
-import static org.junit.Assert.assertThat;
-import static org.xmlunit.matchers.CompareMatcher.isSimilarTo;
-
 import java.io.StringReader;
 import java.io.StringWriter;
 import java.util.ArrayList;
@@ -32,8 +29,13 @@ import javax.xml.stream.events.XMLEvent;
 
 import org.junit.Test;
 
+import static javax.xml.stream.XMLStreamConstants.*;
+import static org.junit.Assert.*;
+import static org.xmlunit.matchers.CompareMatcher.*;
+
 /**
  * @author Arjen Poutsma
+ * @author Andrzej Hołowko
  */
 public class ListBasedXMLEventReaderTests {
 
@@ -41,6 +43,7 @@ public class ListBasedXMLEventReaderTests {
 
 	private final XMLOutputFactory outputFactory = XMLOutputFactory.newFactory();
 
+
 	@Test
 	public void standard() throws Exception {
 		String xml = "<foo><bar>baz</bar></foo>";
@@ -55,9 +58,42 @@ public class ListBasedXMLEventReaderTests {
 		assertThat(resultWriter.toString(), isSimilarTo(xml));
 	}
 
+	@Test
+	public void testGetElementText() throws Exception {
+		String xml = "<foo><bar>baz</bar></foo>";
+		List<XMLEvent> events = readEvents(xml);
+
+		ListBasedXMLEventReader reader = new ListBasedXMLEventReader(events);
+
+		assertEquals(START_DOCUMENT, reader.nextEvent().getEventType());
+		assertEquals(START_ELEMENT, reader.nextEvent().getEventType());
+		assertEquals(START_ELEMENT, reader.nextEvent().getEventType());
+		assertEquals("baz", reader.getElementText());
+		assertEquals(END_ELEMENT, reader.nextEvent().getEventType());
+		assertEquals(END_DOCUMENT, reader.nextEvent().getEventType());
+	}
+
+	@Test
+	public void testGetElementTextThrowsExceptionAtWrongPosition() throws Exception {
+		String xml = "<foo><bar>baz</bar></foo>";
+		List<XMLEvent> events = readEvents(xml);
+
+		ListBasedXMLEventReader reader = new ListBasedXMLEventReader(events);
+
+		assertEquals(START_DOCUMENT, reader.nextEvent().getEventType());
+
+		try {
+			reader.getElementText();
+			fail("Should have thrown XMLStreamException");
+		}
+		catch (XMLStreamException ex) {
+			// expected
+			assertTrue(ex.getMessage().startsWith("Not at START_ELEMENT"));
+		}
+	}
+
 	private List<XMLEvent> readEvents(String xml) throws XMLStreamException {
-		XMLEventReader reader =
+		XMLEventReader reader = this.inputFactory.createXMLEventReader(new StringReader(xml));
-				this.inputFactory.createXMLEventReader(new StringReader(xml));
 		List<XMLEvent> events = new ArrayList<>();
 		while (reader.hasNext()) {
 			events.add(reader.nextEvent());