From 84ec382201b1f786660219d5e5e3c51a2d0bcfae Mon Sep 17 00:00:00 2001 From: Juergen Hoeller Date: Fri, 7 Sep 2018 12:56:15 +0200 Subject: [PATCH] XMLEventReader.getElementText() properly checks for start element Issue: SPR-17233 --- .../util/xml/AbstractXMLEventReader.java | 81 ++----------------- .../util/xml/ListBasedXMLEventReader.java | 71 +++++++++++++++- .../xml/ListBasedXMLEventReaderTests.java | 46 +++++++++-- 3 files changed, 115 insertions(+), 83 deletions(-) diff --git a/spring-core/src/main/java/org/springframework/util/xml/AbstractXMLEventReader.java b/spring-core/src/main/java/org/springframework/util/xml/AbstractXMLEventReader.java index 79bc00a192..4565a25d6a 100644 --- a/spring-core/src/main/java/org/springframework/util/xml/AbstractXMLEventReader.java +++ b/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. - * @throws IllegalArgumentException when called. + * This implementation throws an {@code IllegalArgumentException} for any property. + * @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. - * @see #close() - */ - protected boolean isClosed() { - return this.closed; - } - - /** - * Checks if the reader is closed, and throws a {@code XMLStreamException} if so. + * Check 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"); } } diff --git a/spring-core/src/main/java/org/springframework/util/xml/ListBasedXMLEventReader.java b/spring-core/src/main/java/org/springframework/util/xml/ListBasedXMLEventReader.java index ed79471b3d..392b753747 100644 --- a/spring-core/src/main/java/org/springframework/util/xml/ListBasedXMLEventReader.java +++ b/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 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()) { - return this.events.get(this.cursor++); + if (hasNext()) { + 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(); diff --git a/spring-core/src/test/java/org/springframework/util/xml/ListBasedXMLEventReaderTests.java b/spring-core/src/test/java/org/springframework/util/xml/ListBasedXMLEventReaderTests.java index 8ee0cf4eab..2be303a3a1 100644 --- a/spring-core/src/test/java/org/springframework/util/xml/ListBasedXMLEventReaderTests.java +++ b/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 = "baz"; @@ -55,9 +58,42 @@ public class ListBasedXMLEventReaderTests { assertThat(resultWriter.toString(), isSimilarTo(xml)); } + @Test + public void testGetElementText() throws Exception { + String xml = "baz"; + List 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 = "baz"; + List 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 readEvents(String xml) throws XMLStreamException { - XMLEventReader reader = - this.inputFactory.createXMLEventReader(new StringReader(xml)); + XMLEventReader reader = this.inputFactory.createXMLEventReader(new StringReader(xml)); List events = new ArrayList<>(); while (reader.hasNext()) { events.add(reader.nextEvent());