Skip to content

Commit

Permalink
Fix #5939 | XMLReader::read returns false on invalid xml and error in…
Browse files Browse the repository at this point in the history
… php3. Throw an exception and stop loop execution if feed xml is bad.
  • Loading branch information
Achal-Aggarwal committed Apr 17, 2015
1 parent d5fc4bc commit 9455367
Show file tree
Hide file tree
Showing 4 changed files with 29 additions and 7 deletions.
5 changes: 4 additions & 1 deletion libraries/joomla/feed/factory.php
Expand Up @@ -66,7 +66,10 @@ public function getFeed($uri)
// Skip ahead to the root node.
do
{
$reader->read();
if (!$reader->read())
{
throw new RuntimeException('Error reading feed.');
}
}

while ($reader->nodeType !== XMLReader::ELEMENT);
Expand Down
17 changes: 12 additions & 5 deletions libraries/joomla/feed/parser.php
Expand Up @@ -267,15 +267,22 @@ protected function moveToClosingElement()
$name = $this->stream->name;
$depth = $this->stream->depth;

// Only keep looking until the end of the stream.
while ($this->stream->read())
try
{
// If we have an END_ELEMENT node with the same name and depth as the node we started with we have a bingo. :-)
if (($this->stream->name == $name) && ($this->stream->depth == $depth) && ($this->stream->nodeType == XMLReader::END_ELEMENT))
// Only keep looking until the end of the stream.
while ($this->stream->read())
{
return;
// If we have an END_ELEMENT node with the same name and depth as the node we started with we have a bingo. :-)
if (($this->stream->name == $name) && ($this->stream->depth == $depth) && ($this->stream->nodeType == XMLReader::END_ELEMENT))
{
return;
}
}
}
catch (Exception $e)
{
throw new RuntimeException('Error reading feed.');
}

throw new RuntimeException('Unable to find the closing XML node.');
}
Expand Down
1 change: 1 addition & 0 deletions tests/unit/stubs/feed/very.bad.test.feed
@@ -0,0 +1 @@
A text file.
13 changes: 12 additions & 1 deletion tests/unit/suites/libraries/joomla/feed/JFeedFactoryTest.php
Expand Up @@ -52,7 +52,6 @@ protected function tearDown()
*/
public function testGetFeedBad()
{
$this->markTestSkipped('This test is failing to execute and is locking up the test suite.');
$this->_instance->getFeed(JPATH_TEST_STUBS . '/feed/test.bad.feed');
}

Expand All @@ -61,6 +60,18 @@ public function testGetFeedBad()
*
* @return void
*
* @expectedException RuntimeException
*/
public function testGetFeedWithASimpleTextFile()
{
$this->_instance->getFeed(JPATH_TEST_STUBS . '/feed/very.bad.test.feed');
}

/**
* Tests JFeedFactory::getFeed() with no parser.
*
* @return void
*
* @expectedException LogicException
*/
public function testGetFeedNoParser()
Expand Down

0 comments on commit 9455367

Please sign in to comment.