nextcloud
/
news
зеркало из https://github.com/nextcloud/news.git
1
0
Форкнуть 0
Код Задачи Пакеты Проекты Релизы Вики Активность

fix XSS when importing articles, speed up update and adding of feeds by only purifying content that will be added to the db

This commit is contained in:
Bernhard Posselt 2014-02-11 16:05:37 +01:00
Родитель d5eab3852c
Коммит 99af7d32d4
7 изменённых файлов: 45 добавлений и 67 удалений
Показать статистику Скачать Patch файл Скачать Diff файл Показать все файлы Свернуть все файлы

11
articleenhancer/xpatharticleenhancer.php
Просмотреть файл

@ -33,23 +33,19 @@ class XPathArticleEnhancer implements ArticleEnhancer {
private $feedRegex;
private $purifier;
private $fileFactory;
private $maximumTimeout;
/**
* @param $purifier the purifier object to clean the html which will be
* matched
* @param SimplePieFileFactory a factory for getting a simple pie file instance
* @param array $regexXPathPair an associative array containing regex to
* match the url and the xpath that should be used for it to extract the
* page
* @param int $maximumTimeout maximum timeout in seconds, defaults to 10 sec
*/
public function __construct($purifier, SimplePieFileFactory $fileFactory,
public function __construct(SimplePieFileFactory $fileFactory,
array $regexXPathPair, $maximumTimeout=10){
$this->purifier = $purifier;
$this->regexXPathPair = $regexXPathPair;
$this->fileFactory = $fileFactory;
$this->maximumTimeout = $maximumTimeout;
@ -85,9 +81,8 @@ class XPathArticleEnhancer implements ArticleEnhancer {
// convert all relative to absolute URLs
$xpathResult = $this->substituteRelativeLinks($xpathResult, $item->getUrl());
$sanitizedResult = $this->purifier->purify($xpathResult);
if( $sanitizedResult ) {
$item->setBody($sanitizedResult);
if( $xpathResult ) {
$item->setBody($xpathResult);
}
}
}

8
businesslayer/feedbusinesslayer.php
Просмотреть файл

@ -45,12 +45,14 @@ class FeedBusinessLayer extends BusinessLayer {
private $timeFactory;
private $autoPurgeMinimumInterval;
private $enhancer;
private $purifier;
public function __construct(FeedMapper $feedMapper, Fetcher $feedFetcher,
ItemMapper $itemMapper, API $api,
TimeFactory $timeFactory,
$autoPurgeMinimumInterval,
Enhancer $enhancer){
Enhancer $enhancer,
$purifier){
parent::__construct($feedMapper);
$this->feedFetcher = $feedFetcher;
$this->itemMapper = $itemMapper;
@ -58,6 +60,7 @@ class FeedBusinessLayer extends BusinessLayer {
$this->timeFactory = $timeFactory;
$this->autoPurgeMinimumInterval = $autoPurgeMinimumInterval;
$this->enhancer = $enhancer;
$this->purifier = $purifier;
}
/**
@ -122,6 +125,7 @@ class FeedBusinessLayer extends BusinessLayer {
} catch(DoesNotExistException $ex){
$unreadCount += 1;
$item = $this->enhancer->enhance($item, $feed->getLink());
$item->setBody($this->purifier->purify($item->getBody()));
$this->itemMapper->insert($item);
}
}
@ -192,6 +196,7 @@ class FeedBusinessLayer extends BusinessLayer {
} catch(DoesNotExistException $ex){
$item = $this->enhancer->enhance($item,
$existingFeed->getLink());
$item->setBody($this->purifier->purify($item->getBody()));
$this->itemMapper->insert($item);
}
}
@ -294,6 +299,7 @@ class FeedBusinessLayer extends BusinessLayer {
$existingItem->setStatus($item->getStatus());
$this->itemMapper->update($existingItem);
} catch(DoesNotExistException $ex){
$item->setBody($this->purifier->purify($item->getBody()));
$this->itemMapper->insert($item);
}
}

7
dependencyinjection/dicontainer.php
Просмотреть файл

@ -194,7 +194,8 @@ class DIContainer extends BaseContainer {
$c['API'],
$c['TimeFactory'],
$c['autoPurgeMinimumInterval'],
$c['Enhancer']);
$c['Enhancer'],
$c['HTMLPurifier']);
});
$this['ItemBusinessLayer'] = $this->share(function($c){
@ -264,7 +265,6 @@ class DIContainer extends BaseContainer {
foreach(json_decode($xpathEnhancerConfig, true) as $feed => $config) {
$articleEnhancer = new XPathArticleEnhancer(
$c['HTMLPurifier'],
$c['SimplePieFileFactory'],
$config,
$c['feedFetcherTimeout']
@ -303,8 +303,7 @@ class DIContainer extends BaseContainer {
$c['TimeFactory'],
$c['simplePieCacheDirectory'],
$c['simplePieCacheDuration'],
$c['feedFetcherTimeout'],
$c['HTMLPurifier']);
$c['feedFetcherTimeout']);
});
$this['StatusFlag'] = $this->share(function($c){

15
fetcher/feedfetcher.php
Просмотреть файл

@ -42,8 +42,7 @@ class FeedFetcher implements IFeedFetcher {
private $faviconFetcher;
private $simplePieFactory;
private $fetchTimeout;
private $time;
private $purifier;
private $time;
public function __construct(API $api,
SimplePieAPIFactory $simplePieFactory,
@ -51,15 +50,13 @@ class FeedFetcher implements IFeedFetcher {
TimeFactory $time,
$cacheDirectory,
$cacheDuration,
$fetchTimeout,
$purifier){
$fetchTimeout){
$this->api = $api;
$this->cacheDirectory = $cacheDirectory;
$this->cacheDuration = $cacheDuration;
$this->faviconFetcher = $faviconFetcher;
$this->simplePieFactory = $simplePieFactory;
$this->time = $time;
$this->purifier = $purifier;
$this->fetchTimeout = $fetchTimeout;
}
@ -143,12 +140,8 @@ class FeedFetcher implements IFeedFetcher {
$guid = $simplePieItem->get_id();
$item->setGuid($guid);
// links should always open in a new window
$item->setBody(
$this->purifier->purify(
$simplePieItem->get_content()
)
);
// purification is done in the businesslayer
$item->setBody($simplePieItem->get_content());
// pubdate is not required. if not given use the current date
$date = $simplePieItem->get_date('U');

35
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());

28
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));

8
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);