From e7c3276541ed02f30d6be0469ae5298cad26f537 Mon Sep 17 00:00:00 2001
From: Vhati <tvtronix@yahoo.com>
Date: Sun, 15 Sep 2013 19:33:50 -0400
Subject: [PATCH] Improved sloppy parser exception handling

---
 skel_common/readme_changelog.txt              |   1 +
 .../vhati/modmanager/core/ModUtilities.java   |  13 +-
 .../modmanager/core/SloppyXMLParser.java      | 365 +++++++++---------
 3 files changed, 205 insertions(+), 174 deletions(-)

diff --git a/skel_common/readme_changelog.txt b/skel_common/readme_changelog.txt
index 76cc96f..5e6de47 100644
--- a/skel_common/readme_changelog.txt
+++ b/skel_common/readme_changelog.txt
@@ -7,6 +7,7 @@ Changelog
 - Fixed perpetually green "Update" button
 - Added --global-panic commandline arg to show mod devs typoed find tags
 - Added commandline tips in readme_modders.txt
+- Fixed sloppy parser Validate error about things not allowed at root
 
 1.2:
 - Added a commandline interface
diff --git a/src/main/java/net/vhati/modmanager/core/ModUtilities.java b/src/main/java/net/vhati/modmanager/core/ModUtilities.java
index b8500b4..d64617b 100644
--- a/src/main/java/net/vhati/modmanager/core/ModUtilities.java
+++ b/src/main/java/net/vhati/modmanager/core/ModUtilities.java
@@ -824,7 +824,7 @@ public class ModUtilities {
 			xmlValid = false;
 		}
 		catch ( Exception e ) {
-			log.error( "Error while validating mod xml.", e );
+			log.error( "Error while validating mod xml with the strict parser.", e );
 			messages.add( new ReportMessage(
 				ReportMessage.EXCEPTION,
 				"An error occurred. See log for details."
@@ -846,6 +846,9 @@ public class ModUtilities {
 		List<ReportMessage> messages = new ArrayList<ReportMessage>();
 		boolean xmlValid = true;
 
+		// Meh, the parser's gonna make its own wrapper with declarations anyway.
+		//text = "<wrapper xmlns:mod='mod' xmlns:mod-append='mod-append' xmlns:mod-overwrite='mod-overwrite'>"+ text +"</wrapper>";
+
 		try {
 			SloppyXMLParser parser = new SloppyXMLParser();
 			parser.build( text );
@@ -876,6 +879,7 @@ public class ModUtilities {
 				) );
 			}
 			else {
+				log.error( "Error while validating mod xml with the sloppy parser.", e );
 				messages.add( new ReportMessage(
 					ReportMessage.EXCEPTION,
 					"An error occurred. See log for details."
@@ -883,6 +887,13 @@ public class ModUtilities {
 			}
 			xmlValid = false;
 		}
+		catch ( Exception e ) {
+			log.error( "Error while validating mod xml with the sloppy parser.", e );
+			messages.add( new ReportMessage(
+				ReportMessage.EXCEPTION,
+				"An error occurred. See log for details."
+			) );
+		}
 
 		return new Report( messages, xmlValid );
 	}
diff --git a/src/main/java/net/vhati/modmanager/core/SloppyXMLParser.java b/src/main/java/net/vhati/modmanager/core/SloppyXMLParser.java
index 6446632..1509e3d 100644
--- a/src/main/java/net/vhati/modmanager/core/SloppyXMLParser.java
+++ b/src/main/java/net/vhati/modmanager/core/SloppyXMLParser.java
@@ -16,6 +16,7 @@ import org.jdom2.Content;
 import org.jdom2.DefaultJDOMFactory;
 import org.jdom2.Document;
 import org.jdom2.Element;
+import org.jdom2.IllegalAddException;
 import org.jdom2.JDOMFactory;
 import org.jdom2.Namespace;
 import org.jdom2.Parent;
@@ -115,198 +116,216 @@ public class SloppyXMLParser {
 		String tmp = null;
 		Matcher m = declPtn.matcher( s );
 
-		while ( pos > lastPos && pos < sLen ) {
-			m.region( pos, sLen );
-			boolean matchedChunk = false;
-
-			for ( Pattern chunkPtn : chunkPtns ) {
-				m.usePattern( chunkPtn );
-				if ( !m.lookingAt() ) continue;
-
-				if ( chunkPtn == declPtn ) {
-					// Don't care.
-					addLineAndCol( lastLineAndCol, m.group(0) );
-				}
-				else if ( chunkPtn == emptyCommentPtn ) {
-					String whitespace = m.group( 1 );
-					if ( whitespace.length() > 0 )
-						factory.addContent( parentNode, factory.text( whitespace ) );
-
-					addLineAndCol( lastLineAndCol, s, m.start(), m.end() );
-				}
-				else if ( chunkPtn == commentPtn ) {
-					String whitespace = m.group( 1 );
-					if ( whitespace.length() > 0 )
-						factory.addContent( parentNode, factory.text( whitespace ) );
-
-					tmp = m.group( 2 );
-					if ( tmp.length() == 0 ) {
-						factory.addContent( parentNode, factory.comment( "" ) );
+		try {
+			while ( pos > lastPos && pos < sLen ) {
+				m.region( pos, sLen );
+				boolean matchedChunk = false;
+	
+				for ( Pattern chunkPtn : chunkPtns ) {
+					m.usePattern( chunkPtn );
+					if ( !m.lookingAt() ) continue;
+	
+					if ( chunkPtn == declPtn ) {
+						// Don't care.
+						addLineAndCol( lastLineAndCol, m.group(0) );
 					}
-					else {
-						Matcher splicedMatcher = Pattern.compile( "(\\s*)<!--" ).matcher( tmp );
-						int commentStart = 0;
-						while ( splicedMatcher.find() ) {
-							if ( splicedMatcher.start() - commentStart > 0 ) {
-								String splicedChunk = tmp.substring( commentStart, splicedMatcher.start() );
-								splicedChunk = splicedChunk.replaceAll( "^-+|(?<=-)-+|-+$", "" );
-								if ( splicedChunk.startsWith( " " ) ) splicedChunk += " ";
-								Comment commentNode = factory.comment( splicedChunk );
+					else if ( chunkPtn == emptyCommentPtn ) {
+						String whitespace = m.group( 1 );
+						if ( whitespace.length() > 0 )
+							factory.addContent( parentNode, factory.text( whitespace ) );
+	
+						addLineAndCol( lastLineAndCol, s, m.start(), m.end() );
+					}
+					else if ( chunkPtn == commentPtn ) {
+						String whitespace = m.group( 1 );
+						if ( whitespace.length() > 0 )
+							factory.addContent( parentNode, factory.text( whitespace ) );
+	
+						tmp = m.group( 2 );
+						if ( tmp.length() == 0 ) {
+							factory.addContent( parentNode, factory.comment( "" ) );
+						}
+						else {
+							Matcher splicedMatcher = Pattern.compile( "(\\s*)<!--" ).matcher( tmp );
+							int commentStart = 0;
+							while ( splicedMatcher.find() ) {
+								if ( splicedMatcher.start() - commentStart > 0 ) {
+									String splicedChunk = tmp.substring( commentStart, splicedMatcher.start() );
+									splicedChunk = splicedChunk.replaceAll( "^-+|(?<=-)-+|-+$", "" );
+									if ( splicedChunk.startsWith( " " ) ) splicedChunk += " ";
+									Comment commentNode = factory.comment( splicedChunk );
+									factory.addContent( parentNode, commentNode );
+								}
+								if ( splicedMatcher.group(1).length() > 0 ) {
+									// Whitespace between comments.
+									factory.addContent( parentNode, factory.text( splicedMatcher.group(1) ) );
+								}
+								commentStart = splicedMatcher.end();
+							}
+							if ( commentStart < tmp.length() ) {
+								String finalChunk = tmp.substring( commentStart );
+								finalChunk = finalChunk.replaceAll( "^-+|(?<=-)-+|-+$", "" );
+								Comment commentNode = factory.comment( finalChunk );
 								factory.addContent( parentNode, commentNode );
 							}
-							if ( splicedMatcher.group(1).length() > 0 ) {
-								// Whitespace between comments.
-								factory.addContent( parentNode, factory.text( splicedMatcher.group(1) ) );
-							}
-							commentStart = splicedMatcher.end();
 						}
-						if ( commentStart < tmp.length() ) {
-							String finalChunk = tmp.substring( commentStart );
-							finalChunk = finalChunk.replaceAll( "^-+|(?<=-)-+|-+$", "" );
-							Comment commentNode = factory.comment( finalChunk );
-							factory.addContent( parentNode, commentNode );
+	
+						addLineAndCol( lastLineAndCol, s, m.start(), m.end() );
+					}
+					else if ( chunkPtn == emptyCDATAPtn ) {
+						String whitespace = m.group( 1 );
+						if ( whitespace.length() > 0 )
+							factory.addContent( parentNode, factory.text( whitespace ) );
+	
+						addLineAndCol( lastLineAndCol, s, m.start(), m.end() );
+					}
+					else if ( chunkPtn == cdataPtn ) {
+						String whitespace = m.group( 1 );
+						if ( whitespace.length() > 0 )
+							factory.addContent( parentNode, factory.text( whitespace ) );
+	
+						CDATA cdataNode = factory.cdata( m.group(2) );
+						factory.addContent( parentNode, cdataNode );
+	
+						addLineAndCol( lastLineAndCol, s, m.start(), m.end() );
+					}
+					else if ( chunkPtn == sTagPtn ) {
+						String whitespace = m.group( 1 );
+						if ( whitespace.length() > 0 )
+							factory.addContent( parentNode, factory.text( whitespace ) );
+	
+						String nodePrefix = m.group( 2 );  // Might be null.
+						String nodeName = m.group( 3 );
+						String attrString = m.group( 4 );
+						boolean selfClosing = ( m.group( 5 ).length() > 0 );
+	
+						addLineAndCol( lastLineAndCol, s, m.start(), m.end() );
+	
+						Element tagNode;
+						if ( nodePrefix != null ) {
+							Namespace nodeNS = Namespace.getNamespace( nodePrefix, nodePrefix );  // URI? *shrug*
+							factory.addNamespaceDeclaration( rootNode, nodeNS );
+							tagNode = factory.element( lastLineAndCol[0]+1, lastLineAndCol[1]+1+1, nodeName, nodeNS );
+						} else {
+							tagNode = factory.element( lastLineAndCol[0]+1, lastLineAndCol[1]+1+1, nodeName );
 						}
-					}
-
-					addLineAndCol( lastLineAndCol, s, m.start(), m.end() );
-				}
-				else if ( chunkPtn == emptyCDATAPtn ) {
-					String whitespace = m.group( 1 );
-					if ( whitespace.length() > 0 )
-						factory.addContent( parentNode, factory.text( whitespace ) );
-
-					addLineAndCol( lastLineAndCol, s, m.start(), m.end() );
-				}
-				else if ( chunkPtn == cdataPtn ) {
-					String whitespace = m.group( 1 );
-					if ( whitespace.length() > 0 )
-						factory.addContent( parentNode, factory.text( whitespace ) );
-
-					CDATA cdataNode = factory.cdata( m.group(2) );
-					factory.addContent( parentNode, cdataNode );
-
-					addLineAndCol( lastLineAndCol, s, m.start(), m.end() );
-				}
-				else if ( chunkPtn == sTagPtn ) {
-					String whitespace = m.group( 1 );
-					if ( whitespace.length() > 0 )
-						factory.addContent( parentNode, factory.text( whitespace ) );
-
-					String nodePrefix = m.group( 2 );  // Might be null.
-					String nodeName = m.group( 3 );
-					String attrString = m.group( 4 );
-					boolean selfClosing = ( m.group( 5 ).length() > 0 );
-
-					addLineAndCol( lastLineAndCol, s, m.start(), m.end() );
-
-					Element tagNode;
-					if ( nodePrefix != null ) {
-						Namespace nodeNS = Namespace.getNamespace( nodePrefix, nodePrefix );  // URI? *shrug*
-						factory.addNamespaceDeclaration( rootNode, nodeNS );
-						tagNode = factory.element( lastLineAndCol[0]+1, lastLineAndCol[1]+1+1, nodeName, nodeNS );
-					} else {
-						tagNode = factory.element( lastLineAndCol[0]+1, lastLineAndCol[1]+1+1, nodeName );
-					}
-
-					if ( attrString.length() > 0 ) {
-						Matcher am = attrPtn.matcher( attrString );
-						while ( am.lookingAt() ) {
-							String attrPrefix = am.group( 1 );  // Might be null.
-							String attrName = am.group( 2 );
-							String attrValue = am.group( 3 );
-							attrValue = attrValue.substring( 1, attrValue.length()-1 );
-							attrValue = unescape( attrValue );
-
-							if ( attrPrefix != null ) {
-								if ( attrPrefix.equals( "xmlns" ) ) {
-									// This is a pseudo attribute declaring a namespace prefix.
-									// Move it to the root node.
-									Namespace attrNS = Namespace.getNamespace( attrName, attrName );  // URI? *shrug*
-									factory.addNamespaceDeclaration( rootNode, attrNS );
-								}
-								else {
-									Namespace attrNS = Namespace.getNamespace( attrPrefix, attrPrefix );  // URI? *shrug*
-									factory.addNamespaceDeclaration( rootNode, attrNS );
-									Attribute attrObj = factory.attribute( attrName, attrValue, AttributeType.UNDECLARED, attrNS );
+	
+						if ( attrString.length() > 0 ) {
+							Matcher am = attrPtn.matcher( attrString );
+							while ( am.lookingAt() ) {
+								String attrPrefix = am.group( 1 );  // Might be null.
+								String attrName = am.group( 2 );
+								String attrValue = am.group( 3 );
+								attrValue = attrValue.substring( 1, attrValue.length()-1 );
+								attrValue = unescape( attrValue );
+	
+								if ( attrPrefix != null ) {
+									if ( attrPrefix.equals( "xmlns" ) ) {
+										// This is a pseudo attribute declaring a namespace prefix.
+										// Move it to the root node.
+										Namespace attrNS = Namespace.getNamespace( attrName, attrName );  // URI? *shrug*
+										factory.addNamespaceDeclaration( rootNode, attrNS );
+									}
+									else {
+										Namespace attrNS = Namespace.getNamespace( attrPrefix, attrPrefix );  // URI? *shrug*
+										factory.addNamespaceDeclaration( rootNode, attrNS );
+										Attribute attrObj = factory.attribute( attrName, attrValue, AttributeType.UNDECLARED, attrNS );
+										factory.setAttribute( tagNode, attrObj );
+									}
+								} else if ( attrName.equals("xmlns") ) {
+										// New default namespace URI within this node.
+										Namespace attrNS = Namespace.getNamespace( attrValue );
+										factory.addNamespaceDeclaration( tagNode, attrNS );
+								} else {
+									// Normal attribute.
+									Attribute attrObj = factory.attribute( attrName, attrValue, AttributeType.UNDECLARED, Namespace.NO_NAMESPACE );
 									factory.setAttribute( tagNode, attrObj );
 								}
-							} else if ( attrName.equals("xmlns") ) {
-									// New default namespace URI within this node.
-									Namespace attrNS = Namespace.getNamespace( attrValue );
-									factory.addNamespaceDeclaration( tagNode, attrNS );
-							} else {
-								// Normal attribute.
-								Attribute attrObj = factory.attribute( attrName, attrValue, AttributeType.UNDECLARED, Namespace.NO_NAMESPACE );
-								factory.setAttribute( tagNode, attrObj );
+								am.region( am.end(), am.regionEnd() );
+							}
+							if ( am.regionStart() < attrString.length() ) {
+								int nonspacePos = findNextNonspace( s, pos );
+								int errorPos = ( (nonspacePos != -1) ? nonspacePos : pos );
+	
+								int[] lineAndCol = getLineAndCol( s, errorPos );
+								int lineNum = lineAndCol[0];
+								int colNum = lineAndCol[1];
+	
+								SAXParseException cause = new SAXParseException( String.format( "At line %d, column %d: Strange attributes.", lineNum, colNum ), null, null, lineNum, colNum);
+								throw new JDOMParseException( String.format( "Error on line %d: %s", lineNum, cause.getMessage() ), cause );
 							}
-							am.region( am.end(), am.regionEnd() );
-						}
-						if ( am.regionStart() < attrString.length() ) {
-							int nonspacePos = findNextNonspace( s, pos );
-							int errorPos = ( (nonspacePos != -1) ? nonspacePos : pos );
-
-							int[] lineAndCol = getLineAndCol( s, errorPos );
-							int lineNum = lineAndCol[0];
-							int colNum = lineAndCol[1];
-
-							SAXParseException cause = new SAXParseException( String.format( "At line %d, column %d: Strange attributes.", lineNum, colNum ), null, null, lineNum, colNum);
-							throw new JDOMParseException( String.format( "Error on line %d: %s", lineNum, cause.getMessage() ), cause );
 						}
+	
+						factory.addContent( parentNode, tagNode );
+						if ( !selfClosing ) parentNode = tagNode;
 					}
-
-					factory.addContent( parentNode, tagNode );
-					if ( !selfClosing ) parentNode = tagNode;
+					else if ( chunkPtn == eTagPtn ) {
+						String interimText = m.group( 1 );
+						interimText = unescape( interimText );
+	
+						factory.addContent( parentNode, factory.text( interimText ) );
+						parentNode = parentNode.getParent();
+	
+						addLineAndCol( lastLineAndCol, s, m.start(), m.end() );
+					}
+					else if ( chunkPtn == endSpacePtn ) {
+						// This is the end of the document.
+					}
+					else if ( chunkPtn == strayCharsPtn ) {
+						// Non-space junk between an end tag and a start tag.
+	
+						String whitespace = m.group( 1 );
+						if ( whitespace.length() > 0 )
+							factory.addContent( parentNode, factory.text( whitespace ) );
+	
+						addLineAndCol( lastLineAndCol, s, m.start(), m.end() );
+					}
+	
+					matchedChunk = true;
+					lastPos = pos;
+					pos = m.end();
+					break;
 				}
-				else if ( chunkPtn == eTagPtn ) {
-					String interimText = m.group( 1 );
-					interimText = unescape( interimText );
-
-					factory.addContent( parentNode, factory.text( interimText ) );
-					parentNode = parentNode.getParent();
-
-					addLineAndCol( lastLineAndCol, s, m.start(), m.end() );
+	
+				if ( !matchedChunk ) {
+					int nonspacePos = findNextNonspace( s, pos );
+					int errorPos = ( (nonspacePos != -1) ? nonspacePos : pos );
+	
+					int[] lineAndCol = getLineAndCol( s, errorPos );
+					int lineNum = lineAndCol[0];
+					int colNum = lineAndCol[1];
+	
+					SAXParseException cause = new SAXParseException( String.format( "At line %d, column %d: Unexpected characters.", lineNum, colNum ), null, null, lineNum, colNum);
+					throw new JDOMParseException( String.format( "Error on line %d: %s", lineNum, cause.getMessage() ), cause );
 				}
-				else if ( chunkPtn == endSpacePtn ) {
-					// This is the end of the document.
+			}
+	
+			if ( rootNode.getChildren().size() == 1 ) {
+				// No need for the wrapper, promote its only child to root.
+	
+				Element newRoot = rootNode.getChildren().get( 0 );
+				newRoot.detach();
+				for ( Namespace ns : rootNode.getAdditionalNamespaces() ) {
+					factory.addNamespaceDeclaration( newRoot, ns );
 				}
-				else if ( chunkPtn == strayCharsPtn ) {
-					// Non-space junk between an end tag and a start tag.
-
-					String whitespace = m.group( 1 );
-					if ( whitespace.length() > 0 )
-						factory.addContent( parentNode, factory.text( whitespace ) );
-
-					addLineAndCol( lastLineAndCol, s, m.start(), m.end() );
-				}
-
-				matchedChunk = true;
-				lastPos = pos;
-				pos = m.end();
-				break;
+				factory.setRoot( doc, newRoot );
 			}
 
-			if ( !matchedChunk ) {
-				int nonspacePos = findNextNonspace( s, pos );
-				int errorPos = ( (nonspacePos != -1) ? nonspacePos : pos );
-
-				int[] lineAndCol = getLineAndCol( s, errorPos );
-				int lineNum = lineAndCol[0];
-				int colNum = lineAndCol[1];
-
-				SAXParseException cause = new SAXParseException( String.format( "At line %d, column %d: Unexpected characters.", lineNum, colNum ), null, null, lineNum, colNum);
-				throw new JDOMParseException( String.format( "Error on line %d: %s", lineNum, cause.getMessage() ), cause );
-			}
 		}
+		catch( IllegalAddException e ) {
+			int nonspacePos = findNextNonspace( s, pos );
+			int errorPos = ( (nonspacePos != -1) ? nonspacePos : pos );
 
-		if ( rootNode.getChildren().size() == 1 ) {
-			// No need for the wrapper, promote its only child to root.
+			int[] lineAndCol = getLineAndCol( s, errorPos );
+			int lineNum = lineAndCol[0];
+			int colNum = lineAndCol[1];
 
-			Element newRoot = rootNode.getChildren().get( 0 );
-			newRoot.detach();
-			for ( Namespace ns : rootNode.getAdditionalNamespaces() ) {
-				factory.addNamespaceDeclaration( newRoot, ns );
+			String hint = "";
+			if ( e.getMessage() != null && e.getMessage().indexOf( "not allowed at the document root" ) != -1 ) {
+				hint = " (There's likely an extraneous closing tag before this point.)";
 			}
-			factory.setRoot( doc, newRoot );
+			SAXParseException cause = new SAXParseException( String.format( "At line %d, column %d: %s%s", lineNum, colNum, e.getMessage(), hint ), null, null, lineNum, colNum, e);
+			throw new JDOMParseException( String.format( "Error on line %d: %s", lineNum, cause.getMessage() ), cause );
 		}
 
 		return doc;