SANTUARIO-523 XMLSecurityStreamReader now correctly reads info from XML declaration

src/main/java/org/apache/xml/security/stax/impl/XMLSecurityStreamReader.java

@@ -33,6 +33,7 @@
 import javax.xml.stream.events.EntityReference;
 import javax.xml.stream.events.Namespace;
 import javax.xml.stream.events.ProcessingInstruction;
+import javax.xml.stream.events.StartDocument;
 
 import org.apache.xml.security.exceptions.XMLSecurityException;
 import org.apache.xml.security.stax.ext.InputProcessorChain;
@@ -49,7 +50,11 @@ public class XMLSecurityStreamReader implements XMLStreamReader {
 
     private final InputProcessorChain inputProcessorChain;
     private XMLSecEvent currentXMLSecEvent;
-    private boolean skipDocumentEvents = false;
+    private final boolean skipDocumentEvents;
+    private String version;
+    private boolean standalone;
+    private boolean standaloneSet;
+    private String characterEncodingScheme;
 
     private static final String ERR_STATE_NOT_ELEM = "Current state not START_ELEMENT or END_ELEMENT";
     private static final String ERR_STATE_NOT_STELEM = "Current state not START_ELEMENT";
@@ -75,9 +80,18 @@ public int next() throws XMLStreamException {
             inputProcessorChain.reset();
             currentXMLSecEvent = inputProcessorChain.processEvent();
             eventType = currentXMLSecEvent.getEventType();
-            if (eventType == START_DOCUMENT && this.skipDocumentEvents) {
-                currentXMLSecEvent = inputProcessorChain.processEvent();
-                eventType = currentXMLSecEvent.getEventType();
+            if (eventType == START_DOCUMENT) {
+                // Even when skipDocumentEvents is true, we still want to get the information out of the event.
+                // We only skip the event itself.
+                StartDocument startDocument = (StartDocument) currentXMLSecEvent;
+                version = startDocument.getVersion();
+                characterEncodingScheme = startDocument.getCharacterEncodingScheme();
+                standalone = startDocument.isStandalone();
+                standaloneSet = startDocument.standaloneSet();
+                if (skipDocumentEvents) {
+                    currentXMLSecEvent = inputProcessorChain.processEvent();
+                    eventType = currentXMLSecEvent.getEventType();
+                }
             }
         } catch (XMLSecurityException e) {
             throw new XMLStreamException(e);
@@ -592,22 +606,22 @@ public String getPrefix() {
 
     @Override
     public String getVersion() {
-        return null;
+        return version;
     }
 
     @Override
     public boolean isStandalone() {
-        return false;
+        return standalone;
     }
 
     @Override
     public boolean standaloneSet() {
-        return false;
+        return standaloneSet;
     }
 
     @Override
     public String getCharacterEncodingScheme() {
-        return null;
+        return characterEncodingScheme;
     }
 
     @Override

src/test/java/org/apache/xml/security/test/stax/XMLSecurityStreamReaderTest.java

@@ -19,6 +19,7 @@
 package org.apache.xml.security.test.stax;
 
 import java.io.BufferedReader;
+import java.io.ByteArrayInputStream;
 import java.io.ByteArrayOutputStream;
 import java.io.InputStream;
 import java.io.InputStreamReader;
@@ -51,6 +52,9 @@
 import org.junit.jupiter.api.Test;
 import org.xmlunit.matchers.CompareMatcher;
 
+import static org.hamcrest.MatcherAssert.assertThat;
+import static org.hamcrest.Matchers.equalTo;
+import static org.hamcrest.Matchers.is;
 import static org.junit.jupiter.api.Assertions.assertEquals;
 import static org.junit.jupiter.api.Assertions.assertFalse;
 import static org.junit.jupiter.api.Assertions.assertTrue;
@@ -105,6 +109,70 @@ public void testIdentityTransformSource() throws Exception {
         MatcherAssert.assertThat(readTestFile(), CompareMatcher.isSimilarTo(baos.toString(StandardCharsets.UTF_8.name())));
     }
 
+    @Test
+    public void testDocumentDeclaration() throws Exception {
+        String xml = "<?xml version='1.1' encoding='ISO-8859-1' standalone='yes'?>\n"
+                + "<Document/>";
+        ByteArrayInputStream xmlInput = new ByteArrayInputStream(xml.getBytes(StandardCharsets.ISO_8859_1));
+        XMLInputFactory xmlInputFactory = XMLInputFactory.newInstance();
+        XMLStreamReader stdXmlStreamReader = xmlInputFactory.createXMLStreamReader(xmlInput);
+        InboundSecurityContextImpl securityContext = new InboundSecurityContextImpl();
+        InputProcessorChainImpl inputProcessorChain = new InputProcessorChainImpl(securityContext);
+        inputProcessorChain.addProcessor(new EventReaderProcessor(stdXmlStreamReader));
+        XMLSecurityProperties securityProperties = new XMLSecurityProperties();
+        securityProperties.setSkipDocumentEvents(false);
+        XMLSecurityStreamReader xmlSecurityStreamReader = new XMLSecurityStreamReader(inputProcessorChain, securityProperties);
+        advanceToFirstEvent(xmlSecurityStreamReader);
+        assertThat(xmlSecurityStreamReader.getEventType(), is(XMLStreamConstants.START_DOCUMENT));
+        assertThat(xmlSecurityStreamReader.getVersion(), is(equalTo("1.1")));
+        assertThat(xmlSecurityStreamReader.getCharacterEncodingScheme(), is(equalTo("ISO-8859-1")));
+        assertThat(xmlSecurityStreamReader.isStandalone(), is(true));
+        assertThat(xmlSecurityStreamReader.standaloneSet(), is(true));
+
+        assertThat(xmlSecurityStreamReader.hasNext(), is(true));
+        assertThat(xmlSecurityStreamReader.next(), is(XMLStreamConstants.START_ELEMENT));
+        // Strictly speaking, we should assert that getVersion() etc. throw when not on a START_DOCUMENT event.
+        // However, we have to be lenient to compensate for XML reader implementations such as Xalan,
+        // which access getVersion() etc. when they're positioned on START_ELEMENT, _beyond_ START_DOCUMENT.
+        assertThat(xmlSecurityStreamReader.getVersion(), is(equalTo("1.1")));
+        assertThat(xmlSecurityStreamReader.getCharacterEncodingScheme(), is(equalTo("ISO-8859-1")));
+        assertThat(xmlSecurityStreamReader.isStandalone(), is(true));
+        assertThat(xmlSecurityStreamReader.standaloneSet(), is(true));
+    }
+
+    @Test
+    public void testDocumentDeclarationWhenSkipDocumentEvents() throws Exception {
+        String xml = "<?xml version='1.1' encoding='ISO-8859-1' standalone='yes'?>\n"
+                + "<Document/>";
+        ByteArrayInputStream xmlInput = new ByteArrayInputStream(xml.getBytes(StandardCharsets.ISO_8859_1));
+        XMLInputFactory xmlInputFactory = XMLInputFactory.newInstance();
+        XMLStreamReader stdXmlStreamReader = xmlInputFactory.createXMLStreamReader(xmlInput);
+        InboundSecurityContextImpl securityContext = new InboundSecurityContextImpl();
+        InputProcessorChainImpl inputProcessorChain = new InputProcessorChainImpl(securityContext);
+        inputProcessorChain.addProcessor(new EventReaderProcessor(stdXmlStreamReader));
+        XMLSecurityProperties securityProperties = new XMLSecurityProperties();
+        securityProperties.setSkipDocumentEvents(true);
+        XMLSecurityStreamReader xmlSecurityStreamReader = new XMLSecurityStreamReader(inputProcessorChain, securityProperties);
+        advanceToFirstEvent(xmlSecurityStreamReader);
+        assertThat(xmlSecurityStreamReader.getEventType(), is(XMLStreamConstants.START_ELEMENT));
+        assertThat(xmlSecurityStreamReader.getVersion(), is(equalTo("1.1")));
+        assertThat(xmlSecurityStreamReader.getCharacterEncodingScheme(), is(equalTo("ISO-8859-1")));
+        assertThat(xmlSecurityStreamReader.isStandalone(), is(true));
+        assertThat(xmlSecurityStreamReader.standaloneSet(), is(true));
+    }
+
+    /**
+     * This method advances the reader until it's <i>on</i> the first event, if that's not already the case.
+     * Depending on the implementation, the {@code xmlStreamReader} may be positioned <i>before</i> or <i>on</i>
+     * the first event upon creation.
+     */
+    private static void advanceToFirstEvent(XMLStreamReader xmlStreamReader) throws XMLStreamException {
+        if (xmlStreamReader.getEventType() <= 0) {
+            assertThat(xmlStreamReader.hasNext(), is(true));
+            xmlStreamReader.next();
+        }
+    }
+
     @Test
     public void testCorrectness() throws Exception {
         XMLSecurityProperties securityProperties = new XMLSecurityProperties();
@@ -123,6 +191,9 @@ public void testCorrectness() throws Exception {
                 "org/apache/xml/security/c14n/inExcl/plain-soap-1.1.xml"));
 
         //hmm why does a streamreader return a DOCUMENT_EVENT before we did call next() ??
+        // A: Because some implementations of XMLStreamReader are positioned _on_ the first event rather than before it.
+        // Woodstox is a typical example of such an implementation.
+        // We have to compensate for both types of implementation, see the method advanceToFirstEvent(XMLStreamReader).
         int stdXMLEventType = stdXmlStreamReader.getEventType();
         int secXMLEventType = xmlSecurityStreamReader.getEventType();
         do {
@@ -213,9 +284,9 @@ public void testCorrectness() throws Exception {
                     assertEquals(stdXmlStreamReader.getTextLength(), xmlSecurityStreamReader.getTextLength());
                     break;
                 case XMLStreamConstants.START_DOCUMENT:
-                    assertEquals(stdXmlStreamReader.getCharacterEncodingScheme(), xmlSecurityStreamReader.getCharacterEncodingScheme());
+                    assertEquals(StandardCharsets.UTF_8.name(), xmlSecurityStreamReader.getCharacterEncodingScheme());
                     assertEquals(stdXmlStreamReader.getEncoding(), xmlSecurityStreamReader.getEncoding());
-                    //assertEquals(stdXmlStreamReader.getVersion(), xmlSecurityStreamReader.getVersion());
+                    assertEquals(stdXmlStreamReader.getVersion(), xmlSecurityStreamReader.getVersion());
                     break;
                 case XMLStreamConstants.END_DOCUMENT:
                     break;
@@ -269,17 +340,24 @@ private String readTestFile() throws Exception {
         return stringBuilder.toString();
     }
 
+    private static XMLStreamReader createXmlStreamReader() throws XMLStreamException {
+        XMLInputFactory xmlInputFactory = XMLInputFactory.newInstance();
+        xmlInputFactory.setProperty(XMLInputFactory.IS_COALESCING, true);
+        xmlInputFactory.setProperty(XMLInputFactory.IS_NAMESPACE_AWARE, true);
+        return xmlInputFactory.createXMLStreamReader(InputProcessor.class.getClassLoader().getResourceAsStream(
+                        "org/apache/xml/security/c14n/inExcl/plain-soap-1.1.xml"));
+    }
+
     class EventReaderProcessor implements InputProcessor {
 
         private XMLStreamReader xmlStreamReader;
 
+        EventReaderProcessor(XMLStreamReader xmlStreamReader) {
+            this.xmlStreamReader = xmlStreamReader;
+        }
+
         EventReaderProcessor() throws Exception {
-            XMLInputFactory xmlInputFactory = XMLInputFactory.newInstance();
-            xmlInputFactory.setProperty(XMLInputFactory.IS_COALESCING, true);
-            xmlInputFactory.setProperty(XMLInputFactory.IS_NAMESPACE_AWARE, true);
-            xmlStreamReader =
-                xmlInputFactory.createXMLStreamReader(this.getClass().getClassLoader().getResourceAsStream(
-                    "org/apache/xml/security/c14n/inExcl/plain-soap-1.1.xml"));
+            this(createXmlStreamReader());
         }
 
         @Override

src/test/java/org/apache/xml/security/test/stax/utils/XmlReaderToWriter.java

@@ -31,6 +31,13 @@ private XmlReaderToWriter() {
 
     public static void writeAll(XMLStreamReader xmlr, XMLStreamWriter writer)
             throws XMLStreamException {
+        // Some implementations, Woodstox for example, already position their reader ON the first event, which is.
+        // typically a START_DOCUMENT event.
+        // If already positioned on an event, that is indicated by the event type.
+        // Make sure we don't miss the initial event.
+        if (xmlr.getEventType() > 0) {
+            write(xmlr, writer);
+        }
         while (xmlr.hasNext()) {
             xmlr.next();
             write(xmlr, writer);