Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix endless loop by subsequent and aborted processing instruction or comment #124

Closed
wants to merge 3 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
18 changes: 13 additions & 5 deletions src/main/java/org/codehaus/plexus/util/xml/pull/MXParser.java
Expand Up @@ -460,7 +460,7 @@ private void reset()
// System.out.println("reset() called");
location = null;
lineNumber = 1;
columnNumber = 0;
columnNumber = 1;
seenRoot = false;
reachedEnd = false;
eventType = START_DOCUMENT;
Expand Down Expand Up @@ -2918,7 +2918,7 @@ private void parseComment()
posStart = pos;

final int curLine = lineNumber;
final int curColumn = columnNumber;
final int curColumn = columnNumber - 4;
try
{
final boolean normalizeIgnorableWS = tokenize && !roundtripSupported;
Expand All @@ -2930,7 +2930,11 @@ private void parseComment()
{
// scan until it hits -->
ch = more();
if ( seenDashDash && ch != '>' )
if ( ch == (char)-1 )
{
throw new EOFException( "no more data available" + getPositionDescription() );
}
else if ( seenDashDash && ch != '>' )
{
throw new XmlPullParserException( "in comment after two dashes (--) next character must be >"
+ " not " + printable( ch ), this, null );
Expand Down Expand Up @@ -3035,7 +3039,7 @@ private boolean parsePI()
if ( tokenize )
posStart = pos;
final int curLine = lineNumber;
final int curColumn = columnNumber;
final int curColumn = columnNumber - 2;
int piTargetStart = pos;
int piTargetEnd = -1;
final boolean normalizeIgnorableWS = tokenize && !roundtripSupported;
Expand All @@ -3057,7 +3061,11 @@ private boolean parsePI()
// scan until it hits ?>
// ch = more();

if ( ch == '?' )
if ( ch == (char)-1 )
{
throw new EOFException( "no more data available" + getPositionDescription() );
}
else if ( ch == '?' )
{
if ( !seenPITarget )
{
Expand Down
89 changes: 87 additions & 2 deletions src/test/java/org/codehaus/plexus/util/xml/pull/MXParserTest.java
Expand Up @@ -286,6 +286,35 @@ public void testValidCharacterReferenceDecimal()
}
}

@Test
public void testParserPosition()
throws Exception
{
String input = "<?xml version=\"1.0\" encoding=\"UTF-8\"?><!-- A --> \n <!-- B --><test>\tnnn</test>\n<!-- C\nC -->";

MXParser parser = new MXParser();
parser.setInput( new StringReader( input ) );

assertEquals( XmlPullParser.PROCESSING_INSTRUCTION, parser.nextToken() );
assertPosition( 1, 39, parser );
assertEquals( XmlPullParser.COMMENT, parser.nextToken() );
assertPosition( 1, 49, parser );
assertEquals( XmlPullParser.IGNORABLE_WHITESPACE, parser.nextToken() );
assertPosition( 2, 3, parser ); // end when next token starts
assertEquals( XmlPullParser.COMMENT, parser.nextToken() );
assertPosition( 2, 12, parser );
assertEquals( XmlPullParser.START_TAG, parser.nextToken() );
assertPosition( 2, 18, parser );
assertEquals( XmlPullParser.TEXT, parser.nextToken() );
assertPosition( 2, 23, parser ); // end when next token starts
assertEquals( XmlPullParser.END_TAG, parser.nextToken() );
assertPosition( 2, 29, parser );
assertEquals( XmlPullParser.IGNORABLE_WHITESPACE, parser.nextToken() );
assertPosition( 3, 2, parser ); // end when next token starts
assertEquals( XmlPullParser.COMMENT, parser.nextToken() );
assertPosition( 4, 6, parser );
}

@Test
public void testProcessingInstruction()
throws Exception
Expand Down Expand Up @@ -517,7 +546,7 @@ public void testMalformedProcessingInstructionNoClosingQuestionMark()
}
catch ( XmlPullParserException ex )
{
assertTrue( ex.getMessage().contains( "processing instruction started on line 1 and column 2 was not closed" ) );
assertTrue( ex.getMessage().contains( "processing instruction started on line 1 and column 1 was not closed" ) );
}
}

Expand Down Expand Up @@ -545,7 +574,59 @@ public void testSubsequentMalformedProcessingInstructionNoClosingQuestionMark()
}
catch ( XmlPullParserException ex )
{
assertTrue( ex.getMessage().contains( "processing instruction started on line 1 and column 13 was not closed" ) );
assertTrue( ex.getMessage().contains( "processing instruction started on line 1 and column 12 was not closed" ) );
}
}

@Test
public void testSubsequentAbortedProcessingInstruction()
throws Exception
{
MXParser parser = new MXParser();
StringBuilder sb = new StringBuilder();
sb.append( "<project />" );
sb.append( "<?aborted" );

parser.setInput( new StringReader( sb.toString() ) );

try
{
assertEquals( XmlPullParser.START_TAG, parser.next() );
assertEquals( XmlPullParser.END_TAG, parser.next() );
assertEquals( XmlPullParser.PROCESSING_INSTRUCTION, parser.next() );

fail( "Should fail since it has aborted PI" );
}
catch ( XmlPullParserException ex )
{
assertTrue( ex.getMessage().contains( "@1:21" ) );
assertTrue( ex.getMessage().contains( "processing instruction started on line 1 and column 12 was not closed" ) );
}
}

@Test
public void testSubsequentAbortedComment()
throws Exception
{
MXParser parser = new MXParser();
StringBuilder sb = new StringBuilder();
sb.append( "<project />" );
sb.append( "<!-- aborted" );

parser.setInput( new StringReader( sb.toString() ) );

try
{
assertEquals( XmlPullParser.START_TAG, parser.next() );
assertEquals( XmlPullParser.END_TAG, parser.next() );
assertEquals( XmlPullParser.PROCESSING_INSTRUCTION, parser.next() );

fail( "Should fail since it has aborted comment" );
}
catch ( XmlPullParserException ex )
{
assertTrue( ex.getMessage().contains( "@1:24" ) );
assertTrue( ex.getMessage().contains( "comment started on line 1 and column 12 was not closed" ) );
}
}

Expand Down Expand Up @@ -661,4 +742,8 @@ public void testMalformedXMLRootElement5()
}
}

private static void assertPosition(int row, int col, MXParser parser) {
assertEquals("Current line", row, parser.getLineNumber());
assertEquals("Current column", col, parser.getColumnNumber());
}
}