From b5f4871da2218a3bd8e0b179720db1008beb5dfb Mon Sep 17 00:00:00 2001 From: Bernhard Posselt Date: Sat, 4 Oct 2014 18:22:12 +0200 Subject: [PATCH] further cleanup --- articleenhancer/xpatharticleenhancer.php | 27 +++++++++---------- .../XPathArticleEnhancerTest.php | 19 +++++++++---- 2 files changed, 26 insertions(+), 20 deletions(-) diff --git a/articleenhancer/xpatharticleenhancer.php b/articleenhancer/xpatharticleenhancer.php index 38ce64a40..8d0dba7e8 100644 --- a/articleenhancer/xpatharticleenhancer.php +++ b/articleenhancer/xpatharticleenhancer.php @@ -39,8 +39,6 @@ class XPathArticleEnhancer implements ArticleEnhancer { * match the url and the xpath that should be used for it to extract the * page * @param \OCA\News\Utility\Config $config - * @internal param \OCA\News\ArticleEnhancer\a $SimplePieFileFactory factory for getting a simple pie file instance - * @internal param int $maximumTimeout maximum timeout in seconds, defaults to 10 sec */ public function __construct(SimplePieAPIFactory $fileFactory, array $regexXPathPair, @@ -79,7 +77,8 @@ class XPathArticleEnhancer implements ArticleEnhancer { $xpath = new DOMXpath($dom); $xpathResult = $xpath->evaluate($search); - // in case it wasnt a text query assume its a dom element + // in case it wasnt a text query assume its a dom element and + // convert it to text if(!is_string($xpathResult)) { $xpathResult = $this->domToString($xpathResult); } @@ -118,20 +117,17 @@ class XPathArticleEnhancer implements ArticleEnhancer { $dom = new DOMDocument(); $dom->preserveWhiteSpace = false; - // return, if xml is empty or loading the HTML fails - $isLoaded = Security::scan($xmlString, $dom, function ($xml, $dom) { - return @$dom->loadHTML($xml, LIBXML_NONET); + $noHTMLError = Security::scan($xmlString, $dom, function ($xml, $dom) { + // wrap in div to prevent loadHTML from inserting weird elements + $xml = '
' . $xml . '
'; + return @$dom->loadHTML($xml, LIBXML_NONET | LIBXML_HTML_NODEFDTD + | LIBXML_HTML_NOIMPLIED); }); - if($xmlString === '' || !$isLoaded) { - return $xmlString; + if($xmlString === '' || !$noHTMLError) { + return false; } - // remove removeChild($dom->firstChild); - // remove - $dom->replaceChild($dom->firstChild->firstChild, $dom->firstChild); - foreach (['href', 'src'] as $attribute) { $xpath = new DOMXpath($dom); $xpathResult = $xpath->query( @@ -147,9 +143,10 @@ class XPathArticleEnhancer implements ArticleEnhancer { } // save dom to string and remove - $xmlString = substr(trim($dom->saveHTML()), 6, -7); + $xmlString = $dom->saveHTML(); + // domdocument spoils the string with line breaks between the elements. strip them. - $xmlString = str_replace('\n', '', $xmlString); + $xmlString = str_replace("\n", '', $xmlString); return $xmlString; } diff --git a/tests/unit/articleenhancer/XPathArticleEnhancerTest.php b/tests/unit/articleenhancer/XPathArticleEnhancerTest.php index 2f6f5688e..e7b3d19dd 100644 --- a/tests/unit/articleenhancer/XPathArticleEnhancerTest.php +++ b/tests/unit/articleenhancer/XPathArticleEnhancerTest.php @@ -127,7 +127,7 @@ class XPathArticleEnhancerTest extends \PHPUnit_Framework_TestCase { ->will($this->returnValue($file)); $result = $this->testEnhancer->enhance($item); - $this->assertEquals('hiho', $result->getBody()); + $this->assertEquals('
hiho
', $result->getBody()); } @@ -156,7 +156,8 @@ class XPathArticleEnhancerTest extends \PHPUnit_Framework_TestCase { ->will($this->returnValue($file)); $result = $this->testEnhancer->enhance($item); - $this->assertEquals('
hiho
rawr
', $result->getBody()); + $this->assertEquals('
hiho
rawr
', + $result->getBody()); } @@ -260,7 +261,11 @@ class XPathArticleEnhancerTest extends \PHPUnit_Framework_TestCase { ->will($this->returnValue($file)); $result = $this->testEnhancer->enhance($item); - $this->assertEquals('linklink2', $result->getBody()); + $this->assertEquals('
' . + 'link' . + 'link2' . + '' . + '
', $result->getBody()); } public function testTransformRelativeUrlSpecials() { @@ -285,7 +290,9 @@ class XPathArticleEnhancerTest extends \PHPUnit_Framework_TestCase { ->will($this->returnValue($file)); $result = $this->testEnhancer->enhance($item); - $this->assertEquals('', $result->getBody()); + $this->assertEquals( + '
', + $result->getBody()); } public function testDontTransformAbsoluteUrlsAndMails() { @@ -312,8 +319,10 @@ class XPathArticleEnhancerTest extends \PHPUnit_Framework_TestCase { $result = $this->testEnhancer->enhance($item); $this->assertEquals( + '
' . '' . - 'mail', + 'mail' . + '
', $result->getBody() ); }