FELIX-2962 Ensure certain invalid XML is not accepted, particularly a subsequent open element instead of a closing element, as in "<a><a>"

git-svn-id: https://svn.apache.org/repos/asf/felix/trunk@1129144 13f79535-47bb-0310-9956-ffa450edef68
diff --git a/scr/src/main/java/org/apache/felix/scr/impl/parser/KXml2SAXParser.java b/scr/src/main/java/org/apache/felix/scr/impl/parser/KXml2SAXParser.java
index 643ae64..097cb61 100644
--- a/scr/src/main/java/org/apache/felix/scr/impl/parser/KXml2SAXParser.java
+++ b/scr/src/main/java/org/apache/felix/scr/impl/parser/KXml2SAXParser.java
@@ -21,6 +21,7 @@
 

 import java.io.Reader;

 import java.util.Properties;

+import java.util.Stack;

 

 import org.kxml2.io.KXmlParser;

 import org.xmlpull.v1.XmlPullParser;

@@ -59,21 +60,33 @@
     public void parseXML( KXml2SAXHandler handler ) throws Exception

     {

 

+        final Stack openElements = new Stack();

+        XmlElement currentElement = null;

+

         while ( next() != XmlPullParser.END_DOCUMENT )

         {

             handler.setLineNumber( getLineNumber() );

             handler.setColumnNumber( getColumnNumber() );

+

             if ( getEventType() == XmlPullParser.START_TAG )

             {

+                currentElement = new XmlElement( getNamespace(), getName(), getLineNumber(), getColumnNumber() );

+                openElements.push( currentElement );

+

                 Properties props = new Properties();

                 for ( int i = 0; i < getAttributeCount(); i++ )

                 {

                     props.put( getAttributeName( i ), getAttributeValue( i ) );

                 }

+

                 handler.startElement( getNamespace(), getName(), props );

             }

             else if ( getEventType() == XmlPullParser.END_TAG )

             {

+                ensureMatchingCurrentElement(currentElement);

+                openElements.pop();

+                currentElement = openElements.isEmpty() ? null : ( XmlElement ) openElements.peek();

+

                 handler.endElement( getNamespace(), getName() );

             }

             else if ( getEventType() == XmlPullParser.TEXT )

@@ -91,5 +104,57 @@
                 // do nothing

             }

         }

+

+        if ( !openElements.isEmpty() )

+        {

+            throw new ParseException( "Unclosed elements found: " + openElements, null );

+        }

+    }

+

+

+    private void ensureMatchingCurrentElement( final XmlElement currentElement ) throws Exception

+    {

+        if ( currentElement == null )

+        {

+            throw new ParseException( "Unexpected closing element "

+                + new XmlElement( getNamespace(), getName(), getLineNumber(), getColumnNumber() ), null );

+        }

+

+        if ( !currentElement.match( getNamespace(), getName() ) )

+        {

+            throw new ParseException( "Unexpected closing element "

+                + new XmlElement( getNamespace(), getName(), getLineNumber(), getColumnNumber() )

+                + ": Does not match opening element " + currentElement, null );

+        }

+    }

+

+    private static class XmlElement

+    {

+

+        final String namespaceUri;

+        final String name;

+        final int line;

+        final int col;

+

+

+        XmlElement( final String namespaceUri, final String name, final int line, final int col )

+        {

+            this.namespaceUri = namespaceUri;

+            this.name = name;

+            this.line = line;

+            this.col = col;

+        }

+

+

+        boolean match( final String namespaceUri, final String name )

+        {

+            return namespaceUri.equals( this.namespaceUri ) && name.equals( this.name );

+        }

+

+        @Override

+        public String toString()

+        {

+            return name + "@" + line + ":" + col;

+        }

     }

 }

diff --git a/scr/src/test/java/org/apache/felix/scr/impl/metadata/XmlHandlerTest.java b/scr/src/test/java/org/apache/felix/scr/impl/metadata/XmlHandlerTest.java
index 1647a9e..9b6b29b 100644
--- a/scr/src/test/java/org/apache/felix/scr/impl/metadata/XmlHandlerTest.java
+++ b/scr/src/test/java/org/apache/felix/scr/impl/metadata/XmlHandlerTest.java
@@ -22,6 +22,8 @@
 import java.io.BufferedReader;
 import java.io.IOException;
 import java.io.InputStreamReader;
+import java.io.Reader;
+import java.io.StringReader;
 import java.util.Iterator;
 import java.util.List;
 
@@ -30,6 +32,7 @@
 import org.apache.felix.scr.impl.MockBundle;
 import org.apache.felix.scr.impl.MockLogger;
 import org.apache.felix.scr.impl.parser.KXml2SAXParser;
+import org.apache.felix.scr.impl.parser.ParseException;
 import org.osgi.service.component.ComponentException;
 import org.xmlpull.v1.XmlPullParserException;
 
@@ -47,6 +50,48 @@
     }
 
 
+    public void test_unclosed_elements() throws Exception
+    {
+        try
+        {
+            readMetadataFromString( "<component name=\"n\"><implementation class=\"n\" /><component name=\"x\">" );
+            fail( "ParseException expected for unclosed elements" );
+        }
+        catch ( ParseException pe )
+        {
+            // exptected
+        }
+    }
+
+
+    public void test_no_opening_element() throws Exception
+    {
+        try
+        {
+            readMetadataFromString( "</component>" );
+            fail( "Exception expected for element without opening element" );
+        }
+        catch ( Exception p )
+        {
+            // exptected
+        }
+    }
+
+
+    public void test_interleaved_elements() throws Exception
+    {
+        try
+        {
+            readMetadataFromString( "<component name=\"n\" ><implementation class=\"n\"></component></implementation>" );
+            fail( "Exception expected for interleaved elements" );
+        }
+        catch ( Exception p )
+        {
+            // exptected
+        }
+    }
+
+
     public void test_no_namespace() throws Exception
     {
         final List metadataList = readMetadata( "/components_no_namespace.xml" );
@@ -77,8 +122,8 @@
         assertEquals( "Expected Deactivate Method set", "mydeactivate", cm11.getDeactivate() );
         assertTrue( "Activate method expected to be declared", cm11.isDeactivateDeclared() );
         assertEquals( "Expected Modified Method set", "mymodified", cm11.getModified() );
-        assertEquals( "Expected Configuration Policy set", ComponentMetadata.CONFIGURATION_POLICY_IGNORE, cm11
-            .getConfigurationPolicy() );
+        assertEquals( "Expected Configuration Policy set", ComponentMetadata.CONFIGURATION_POLICY_IGNORE,
+            cm11.getConfigurationPolicy() );
     }
 
 
@@ -274,17 +319,45 @@
 
     //---------- helper
 
+    private List readMetadata( final Reader reader ) throws IOException, ComponentException, XmlPullParserException,
+        Exception
+    {
+
+        try
+        {
+            final KXml2SAXParser parser = new KXml2SAXParser( reader );
+
+            XmlHandler handler = new XmlHandler( new MockBundle(), logger );
+            parser.parseXML( handler );
+
+            return handler.getComponentMetadataList();
+        }
+        finally
+        {
+            try
+            {
+                reader.close();
+            }
+            catch ( IOException ignore )
+            {
+            }
+        }
+    }
+
+
     private List readMetadata( String filename ) throws IOException, ComponentException, XmlPullParserException,
         Exception
     {
         BufferedReader in = new BufferedReader( new InputStreamReader( getClass().getResourceAsStream( filename ),
             "UTF-8" ) );
-        final KXml2SAXParser parser = new KXml2SAXParser( in );
+        return readMetadata( in );
+    }
 
-        XmlHandler handler = new XmlHandler( new MockBundle(), logger );
-        parser.parseXML( handler );
 
-        return handler.getComponentMetadataList();
+    private List readMetadataFromString( final String source ) throws IOException, ComponentException,
+        XmlPullParserException, Exception
+    {
+        return readMetadata( new StringReader( source ) );
     }