From 99af7d32d42d7b77bae4f7747c02db959f35d668 Mon Sep 17 00:00:00 2001
From: Bernhard Posselt <dev@bernhard-posselt.com>
Date: Tue, 11 Feb 2014 16:05:37 +0100
Subject: fix XSS when importing articles, speed up update and adding of feeds
 by only purifying content that will be added to the db

---
 .../articleenhancer/XPathArticleEnhancerTest.php   | 35 ----------------------
 tests/unit/businesslayer/FeedBusinessLayerTest.php | 28 ++++++++++++++++-
 tests/unit/fetcher/FeedFetcherTest.php             |  8 +----
 3 files changed, 28 insertions(+), 43 deletions(-)

(limited to 'tests/unit')

diff --git a/tests/unit/articleenhancer/XPathArticleEnhancerTest.php b/tests/unit/articleenhancer/XPathArticleEnhancerTest.php
index 798fa9203..60b8e0a90 100644
--- a/tests/unit/articleenhancer/XPathArticleEnhancerTest.php
+++ b/tests/unit/articleenhancer/XPathArticleEnhancerTest.php
@@ -32,7 +32,6 @@ require_once(__DIR__ . "/../../classloader.php");
 
 class XPathArticleEnhancerTest extends \OCA\AppFramework\Utility\TestUtility {
 
-	private $purifier;
 	private $testEnhancer;
 	private $fileFactory;
 	private $timeout;
@@ -42,10 +41,8 @@ class XPathArticleEnhancerTest extends \OCA\AppFramework\Utility\TestUtility {
 		$this->fileFactory = $this->getMockBuilder('\OCA\News\Utility\SimplePieFileFactory')
 			->disableOriginalConstructor()
 			->getMock();
-		$this->purifier = $this->getMock('purifier', array('purify'));
 
 		$this->testEnhancer = new XPathArticleEnhancer(
-			$this->purifier,
 			$this->fileFactory,
 			array(
 				'/explosm.net\/comics/' => '//*[@id=\'maincontent\']/div[2]/div/span',
@@ -85,10 +82,6 @@ class XPathArticleEnhancerTest extends \OCA\AppFramework\Utility\TestUtility {
 			->with($this->equalTo($item->getUrl()),
 				$this->equalTo($this->timeout))
 			->will($this->returnValue($file));
-		$this->purifier->expects($this->once())
-			->method('purify')
-			->with($this->equalTo('<span>hiho</span>'))
-			->will($this->returnValue('<span>hiho</span>'));
 
 		$result = $this->testEnhancer->enhance($item);
 		$this->assertEquals('<span>hiho</span>', $result->getBody());
@@ -115,10 +108,6 @@ class XPathArticleEnhancerTest extends \OCA\AppFramework\Utility\TestUtility {
 			->with($this->equalTo($item->getUrl()),
 				$this->equalTo($this->timeout))
 			->will($this->returnValue($file));
-		$this->purifier->expects($this->once())
-			->method('purify')
-			->with($this->equalTo('<div>hiho</div><div>rawr</div>'))
-			->will($this->returnValue('<div>hiho</div><div>rawr</div>'));
 
 		$result = $this->testEnhancer->enhance($item);
 		$this->assertEquals('<div>hiho</div><div>rawr</div>', $result->getBody());
@@ -143,10 +132,6 @@ class XPathArticleEnhancerTest extends \OCA\AppFramework\Utility\TestUtility {
 			->with($this->equalTo($item->getUrl()),
 				$this->equalTo($this->timeout))
 			->will($this->returnValue($file));
-		$this->purifier->expects($this->once())
-			->method('purify')
-			->with($this->equalTo(null))
-			->will($this->returnValue(null));
 
 		$result = $this->testEnhancer->enhance($item);
 		$this->assertEquals('Hello thar', $result->getBody());
@@ -166,10 +151,6 @@ class XPathArticleEnhancerTest extends \OCA\AppFramework\Utility\TestUtility {
 			->with($this->equalTo($item->getUrl()),
 				$this->equalTo($this->timeout))
 			->will($this->returnValue($file));
-		$this->purifier->expects($this->once())
-			->method('purify')
-			->with($this->equalTo(null))
-			->will($this->returnValue(null));
 
 		$result = $this->testEnhancer->enhance($item);
 		$this->assertEquals('Hello thar', $result->getBody());
@@ -194,10 +175,6 @@ class XPathArticleEnhancerTest extends \OCA\AppFramework\Utility\TestUtility {
 			->with($this->equalTo($item->getUrl()),
 				$this->equalTo($this->timeout))
 			->will($this->returnValue($file));
-		$this->purifier->expects($this->once())
-			->method('purify')
-			->with($this->equalTo(null))
-			->will($this->returnValue(null));
 
 		$result = $this->testEnhancer->enhance($item);
 		$this->assertEquals('Hello thar', $result->getBody());
@@ -223,10 +200,6 @@ class XPathArticleEnhancerTest extends \OCA\AppFramework\Utility\TestUtility {
 			->with($this->equalTo($item->getUrl()),
 				$this->equalTo($this->timeout))
 			->will($this->returnValue($file));
-		$this->purifier->expects($this->once())
-			->method('purify')
-			->with($this->equalTo('<a href="https://www.explosm.net/a/relative/url.html?a=1#b">link</a><a href="https://www.explosm.net/all/b/relative/url.html">link2</a><img src="https://www.explosm.net/another/relative/link.jpg">'))
-			->will($this->returnValue('<a href="https://www.explosm.net/a/relative/url.html?a=1#b">link</a><a href="https://www.explosm.net/all/b/relative/url.html">link2</a><img src="https://www.explosm.net/another/relative/link.jpg">'));
 
 		$result = $this->testEnhancer->enhance($item);
 		$this->assertEquals('<a target="_blank" href="https://www.explosm.net/a/relative/url.html?a=1#b">link</a><a target="_blank" href="https://www.explosm.net/all/b/relative/url.html">link2</a><img src="https://www.explosm.net/another/relative/link.jpg">', $result->getBody());
@@ -249,10 +222,6 @@ class XPathArticleEnhancerTest extends \OCA\AppFramework\Utility\TestUtility {
 			->with($this->equalTo($item->getUrl()),
 				$this->equalTo($this->timeout))
 			->will($this->returnValue($file));
-		$this->purifier->expects($this->once())
-			->method('purify')
-			->with($this->equalTo('<img src="https://username:secret@www.explosm.net/all/relative/url.png?a=1&amp;b=2">'))
-			->will($this->returnValue('<img src="https://username:secret@www.explosm.net/all/relative/url.png?a=1&amp;b=2">'));
 
 		$result = $this->testEnhancer->enhance($item);
 		$this->assertEquals('<img src="https://username:secret@www.explosm.net/all/relative/url.png?a=1&amp;b=2">', $result->getBody());
@@ -276,10 +245,6 @@ class XPathArticleEnhancerTest extends \OCA\AppFramework\Utility\TestUtility {
 			->with($this->equalTo($item->getUrl()),
 				$this->equalTo($this->timeout))
 			->will($this->returnValue($file));
-		$this->purifier->expects($this->once())
-			->method('purify')
-			->with($this->equalTo('<img src="http://www.url.com/absolute/url.png"><a href="mailto:test@testsite.com">mail</a>'))
-			->will($this->returnValue('<img src="http://www.url.com/absolute/url.png"><a href="mailto:test@testsite.com">mail</a>'));
 
 		$result = $this->testEnhancer->enhance($item);
 		$this->assertEquals('<img src="http://www.url.com/absolute/url.png"><a target="_blank" href="mailto:test@testsite.com">mail</a>', $result->getBody());
diff --git a/tests/unit/businesslayer/FeedBusinessLayerTest.php b/tests/unit/businesslayer/FeedBusinessLayerTest.php
index 454a4966e..357ee73eb 100644
--- a/tests/unit/businesslayer/FeedBusinessLayerTest.php
+++ b/tests/unit/businesslayer/FeedBusinessLayerTest.php
@@ -48,6 +48,7 @@ class FeedBusinessLayerTest extends \OCA\AppFramework\Utility\TestUtility {
 	private $importParser;
 	private $autoPurgeMinimumInterval;
 	private $enhancer;
+	private $purifier;
 
 	protected function setUp(){
 		$this->api = $this->getAPIMock();
@@ -72,10 +73,11 @@ class FeedBusinessLayerTest extends \OCA\AppFramework\Utility\TestUtility {
 		$this->enhancer = $this->getMockBuilder('\OCA\News\ArticleEnhancer\Enhancer')
 			->disableOriginalConstructor()
 			->getMock();
+		$this->purifier = $this->getMock('purifier', array('purify'));
 		$this->feedBusinessLayer = new FeedBusinessLayer($this->feedMapper,
 			$this->fetcher, $this->itemMapper, $this->api,
 			$timeFactory, $this->autoPurgeMinimumInterval,
-			$this->enhancer);
+			$this->enhancer, $this->purifier);
 		$this->user = 'jack';
 		$response = 'hi';
 	}
@@ -150,6 +152,10 @@ class FeedBusinessLayerTest extends \OCA\AppFramework\Utility\TestUtility {
 			->with($this->equalTo($return[1][1]),
 				$this->equalTo($url))
 			->will($this->returnValue($return[1][1]));
+		$this->purifier->expects($this->at(0))
+			->method('purify')
+			->with($this->equalTo($return[1][1]->getBody()))
+			->will($this->returnValue($return[1][1]->getBody()));
 		$this->itemMapper->expects($this->at(1))
 			->method('insert')
 			->with($this->equalTo($return[1][1]));
@@ -165,6 +171,10 @@ class FeedBusinessLayerTest extends \OCA\AppFramework\Utility\TestUtility {
 			->with($this->equalTo($return[1][0]),
 				$this->equalTo($url))
 			->will($this->returnValue($return[1][0]));
+		$this->purifier->expects($this->at(1))
+			->method('purify')
+			->with($this->equalTo($return[1][0]->getBody()))
+			->will($this->returnValue($return[1][0]->getBody()));
 		$this->itemMapper->expects($this->at(3))
 			->method('insert')
 			->with($this->equalTo($return[1][0]));
@@ -219,6 +229,10 @@ class FeedBusinessLayerTest extends \OCA\AppFramework\Utility\TestUtility {
 			->with($this->equalTo($return[1][1]),
 				$this->equalTo($url))
 			->will($this->returnValue($return[1][1]));
+		$this->purifier->expects($this->at(0))
+			->method('purify')
+			->with($this->equalTo($return[1][1]->getBody()))
+			->will($this->returnValue($return[1][1]->getBody()));
 		$this->itemMapper->expects($this->at(1))
 			->method('insert')
 			->with($this->equalTo($return[1][1]));
@@ -274,6 +288,10 @@ class FeedBusinessLayerTest extends \OCA\AppFramework\Utility\TestUtility {
 			->with($this->equalTo($items[0]),
 				$this->equalTo($feed->getUrl()))
 			->will($this->returnValue($items[0]));
+		$this->purifier->expects($this->at(0))
+			->method('purify')
+			->with($this->equalTo($items[0]->getBody()))
+			->will($this->returnValue($items[0]->getBody()));
 		$this->itemMapper->expects($this->once())
 			->method('insert')
 			->with($this->equalTo($items[0]));
@@ -525,6 +543,10 @@ class FeedBusinessLayerTest extends \OCA\AppFramework\Utility\TestUtility {
 			->method('insert')
 			->with($this->equalTo($item));
 
+		$this->purifier->expects($this->once())
+			->method('purify')
+			->with($this->equalTo($item->getBody()))
+			->will($this->returnValue($item->getBody()));
 
 		$result = $this->feedBusinessLayer->importArticles($items, $this->user);
 
@@ -595,6 +617,10 @@ class FeedBusinessLayerTest extends \OCA\AppFramework\Utility\TestUtility {
 		$this->itemMapper->expects($this->at(0))
 			->method('findByGuidHash')
 			->will($this->throwException(new DoesNotExistException('yo')));
+		$this->purifier->expects($this->once())
+			->method('purify')
+			->with($this->equalTo($item->getBody()))
+			->will($this->returnValue($item->getBody()));
 		$this->itemMapper->expects($this->at(1))
 			->method('insert')
 			->with($this->equalTo($item));
diff --git a/tests/unit/fetcher/FeedFetcherTest.php b/tests/unit/fetcher/FeedFetcherTest.php
index da7348fc9..b4ec42e60 100644
--- a/tests/unit/fetcher/FeedFetcherTest.php
+++ b/tests/unit/fetcher/FeedFetcherTest.php
@@ -81,7 +81,6 @@ class FeedFetcherTest extends \OCA\AppFramework\Utility\TestUtility {
 			'\OCA\AppFramework\Utility\FaviconFetcher')
 			->disableOriginalConstructor()
 			->getMock();
-		$this->purifier = $this->getMock('purifier', array('purify'));
 		$this->time = 2323;
 		$timeFactory = $this->getMockBuilder(
 			'\OCA\AppFramework\Utility\TimeFactory')
@@ -99,8 +98,7 @@ class FeedFetcherTest extends \OCA\AppFramework\Utility\TestUtility {
 						 $timeFactory,
 						 $this->cacheDirectory,
 						 $this->cacheDuration,
-						 $this->fetchTimeout,
-						 $this->purifier);
+						 $this->fetchTimeout);
 		$this->url = 'http://tests';
 
 		$this->permalink = 'http://permalink';
@@ -177,10 +175,6 @@ class FeedFetcherTest extends \OCA\AppFramework\Utility\TestUtility {
 
 
 	private function createItem($author=false, $enclosureType=null, $noPubDate=false) {
-		$this->purifier->expects($this->once())
-			->method('purify')
-			->with($this->equalTo($this->body))
-			->will($this->returnValue($this->body));
 		$this->expectItem('get_permalink', $this->permalink);
 		$this->expectItem('get_title', $this->title);
 		$this->expectItem('get_id', $this->guid);
-- 
cgit v1.2.3