From 9455367c8611f13dda7d9659ccb4b96cec72f4dc Mon Sep 17 00:00:00 2001 From: Achal-Aggarwal Date: Fri, 17 Apr 2015 18:15:50 +0530 Subject: [PATCH] Fix #5939 | XMLReader::read returns false on invalid xml and error in php3. Throw an exception and stop loop execution if feed xml is bad. --- libraries/joomla/feed/factory.php | 5 ++++- libraries/joomla/feed/parser.php | 17 ++++++++++++----- tests/unit/stubs/feed/very.bad.test.feed | 1 + .../libraries/joomla/feed/JFeedFactoryTest.php | 13 ++++++++++++- 4 files changed, 29 insertions(+), 7 deletions(-) create mode 100644 tests/unit/stubs/feed/very.bad.test.feed diff --git a/libraries/joomla/feed/factory.php b/libraries/joomla/feed/factory.php index fa9b1c2828187..dee730f00d516 100644 --- a/libraries/joomla/feed/factory.php +++ b/libraries/joomla/feed/factory.php @@ -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); diff --git a/libraries/joomla/feed/parser.php b/libraries/joomla/feed/parser.php index a7f8bac122f24..54815d74069b4 100644 --- a/libraries/joomla/feed/parser.php +++ b/libraries/joomla/feed/parser.php @@ -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.'); } diff --git a/tests/unit/stubs/feed/very.bad.test.feed b/tests/unit/stubs/feed/very.bad.test.feed new file mode 100644 index 0000000000000..83d8a6bfab7b8 --- /dev/null +++ b/tests/unit/stubs/feed/very.bad.test.feed @@ -0,0 +1 @@ +A text file. \ No newline at end of file diff --git a/tests/unit/suites/libraries/joomla/feed/JFeedFactoryTest.php b/tests/unit/suites/libraries/joomla/feed/JFeedFactoryTest.php index 9fb0ca31810ef..2db99b60e0ef6 100644 --- a/tests/unit/suites/libraries/joomla/feed/JFeedFactoryTest.php +++ b/tests/unit/suites/libraries/joomla/feed/JFeedFactoryTest.php @@ -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'); } @@ -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()