Merge pull request #12664 from owncloud/fix-12164

Use httphelper and cache response even when it is empty

core/ajax/update.php

@@ -9,7 +9,11 @@ if (OC::checkUpgrade(false)) {
 
 	$l = new \OC_L10N('core');
 	$eventSource = \OC::$server->createEventSource();
-	$updater = new \OC\Updater(\OC_Log::$object);
+	$updater = new \OC\Updater(
+			\OC::$server->getHTTPHelper(),
+			\OC::$server->getAppConfig(),
+			\OC_Log::$object
+	);
 	$updater->listen('\OC\Updater', 'maintenanceStart', function () use ($eventSource, $l) {
 		$eventSource->send('success', (string)$l->t('Turned on maintenance mode'));
 	});

core/command/upgrade.php

@@ -84,7 +84,7 @@ class Upgrade extends Command {
 
 		if(\OC::checkUpgrade(false)) {
 			$self = $this;
-			$updater = new Updater();
+			$updater = new Updater(\OC::$server->getHTTPHelper(), \OC::$server->getAppConfig());
 
 			$updater->setSimulateStepEnabled($simulateStepEnabled);
 			$updater->setUpdateStepEnabled($updateStepEnabled);

lib/private/templatelayout.php

@@ -44,7 +44,7 @@ class OC_TemplateLayout extends OC_Template {
 			// Update notification
 			if($this->config->getSystemValue('updatechecker', true) === true &&
 				OC_User::isAdminUser(OC_User::getUser())) {
-				$updater = new \OC\Updater();
+				$updater = new \OC\Updater(\OC::$server->getHTTPHelper(), \OC::$server->getAppConfig());
 				$data = $updater->check();
 
 				if(isset($data['version']) && $data['version'] != '' and $data['version'] !== Array()) {

lib/private/updater.php

@@ -26,6 +26,16 @@ class Updater extends BasicEmitter {
 	 */
 	private $log;
 	
+	/**
+	 * @var \OC\HTTPHelper $helper;
+	 */
+	private $httpHelper;
+	
+	/**
+	 * @var \OCP\IAppConfig;
+	 */
+	private $config;
+
 	private $simulateStepEnabled;
 
 	private $updateStepEnabled;
@@ -33,8 +43,10 @@ class Updater extends BasicEmitter {
 	/**
 	 * @param \OC\Log $log
 	 */
-	public function __construct($log = null) {
+	public function __construct($httpHelper, $config,  $log = null) {
+		$this->httpHelper = $httpHelper;
 		$this->log = $log;
+		$this->config = $config;
 		$this->simulateStepEnabled = true;
 		$this->updateStepEnabled = true;
 	}
@@ -69,23 +81,23 @@ class Updater extends BasicEmitter {
 	public function check($updaterUrl = null) {
 
 		// Look up the cache - it is invalidated all 30 minutes
-		if ((\OC_Appconfig::getValue('core', 'lastupdatedat') + 1800) > time()) {
-			return json_decode(\OC_Appconfig::getValue('core', 'lastupdateResult'), true);
+		if (($this->config->getValue('core', 'lastupdatedat') + 1800) > time()) {
+			return json_decode($this->config->getValue('core', 'lastupdateResult'), true);
 		}
 
 		if (is_null($updaterUrl)) {
 			$updaterUrl = 'https://apps.owncloud.com/updater.php';
 		}
 
-		\OC_Appconfig::setValue('core', 'lastupdatedat', time());
+		$this->config->setValue('core', 'lastupdatedat', time());
 
-		if (\OC_Appconfig::getValue('core', 'installedat', '') == '') {
-			\OC_Appconfig::setValue('core', 'installedat', microtime(true));
+		if ($this->config->getValue('core', 'installedat', '') == '') {
+			$this->config->setValue('core', 'installedat', microtime(true));
 		}
 
 		$version = \OC_Util::getVersion();
-		$version['installed'] = \OC_Appconfig::getValue('core', 'installedat');
-		$version['updated'] = \OC_Appconfig::getValue('core', 'lastupdatedat');
+		$version['installed'] = $this->config->getValue('core', 'installedat');
+		$version['updated'] = $this->config->getValue('core', 'lastupdatedat');
 		$version['updatechannel'] = \OC_Util::getChannel();
 		$version['edition'] = \OC_Util::getEditionString();
 		$version['build'] = \OC_Util::getBuild();
@@ -95,30 +107,25 @@ class Updater extends BasicEmitter {
 		$url = $updaterUrl . '?version=' . $versionString;
 
 		// set a sensible timeout of 10 sec to stay responsive even if the update server is down.
-		$ctx = stream_context_create(
-			array(
-				'http' => array(
-					'timeout' => 10
-				)
-			)
-		);
-		$xml = @file_get_contents($url, 0, $ctx);
-		if ($xml == false) {
-			return array();
-		}
+
+		$tmp = array();
+		$xml = $this->httpHelper->getUrlContent($url);
+		if ($xml) {
 			$loadEntities = libxml_disable_entity_loader(true);
 			$data = @simplexml_load_string($xml);
 			libxml_disable_entity_loader($loadEntities);
-
-		$tmp = array();
+			if ($data !== false) {
 				$tmp['version'] = $data->version;
 				$tmp['versionstring'] = $data->versionstring;
 				$tmp['url'] = $data->url;
 				$tmp['web'] = $data->web;
+			}
+		} else {
+			$data = array();
+		}
 
 		// Cache the result
-		\OC_Appconfig::setValue('core', 'lastupdateResult', json_encode($data));
-
+		$this->config->setValue('core', 'lastupdateResult', json_encode($data));
 		return $tmp;
 	}
 
@@ -218,7 +225,7 @@ class Updater extends BasicEmitter {
 			$repair->run();
 
 			//Invalidate update feed
-			\OC_Appconfig::setValue('core', 'lastupdatedat', 0);
+			$this->config->setValue('core', 'lastupdatedat', 0);
 
 			// only set the final version if everything went well
 			\OC_Config::setValue('version', implode('.', \OC_Util::getVersion()));

tests/lib/updater.php

@@ -26,9 +26,82 @@ class UpdaterTest extends \Test\TestCase {
 	/**
 	 * @dataProvider versionCompatibilityTestData
 	 */
-	function testIsUpgradePossible($oldVersion, $newVersion, $result) {
-		$updater = new Updater();
+	public function testIsUpgradePossible($oldVersion, $newVersion, $result) {
+		$updater = new Updater(\OC::$server->getHTTPHelper(), \OC::$server->getConfig());
 		$this->assertSame($result, $updater->isUpgradePossible($oldVersion, $newVersion));
 	}
 	
+	public function testBrokenXmlResponse(){
+		$invalidUpdater = $this->getUpdaterMock('OMG!');
+		$invalidResult = $invalidUpdater->check();
+		$this->assertEmpty($invalidResult);
+	}
+	
+	public function testEmptyResponse(){
+		$emptyUpdater = $this->getUpdaterMock('');
+		$emptyResult = $emptyUpdater->check();
+		$this->assertEmpty($emptyResult);
+		
+		// Error while fetching new contents e.g. too many redirects
+		$falseUpdater = $this->getUpdaterMock(false);
+		$falseResult = $falseUpdater->check();
+		$this->assertEmpty($falseResult);
+	}
+	
+	public function testValidEmptyXmlResponse(){
+		$updater = $this->getUpdaterMock(
+				'<?xml version="1.0"?><owncloud><version></version><versionstring></versionstring><url></url><web></web></owncloud>'
+		);
+		$result = array_map('strval', $updater->check());
+		
+		$this->assertArrayHasKey('version', $result);
+		$this->assertArrayHasKey('versionstring', $result);
+		$this->assertArrayHasKey('url', $result);
+		$this->assertArrayHasKey('web', $result);
+		$this->assertEmpty($result['version']);
+		$this->assertEmpty($result['versionstring']);
+		$this->assertEmpty($result['url']);
+		$this->assertEmpty($result['web']);
+	}
+	
+	public function testValidUpdateResponse(){
+		$newUpdater = $this->getUpdaterMock(
+				'<?xml version="1.0"?>
+<owncloud>
+  <version>7.0.3.4</version>
+  <versionstring>ownCloud 7.0.3</versionstring>
+  <url>http://download.owncloud.org/community/owncloud-7.0.3.zip</url>
+  <web>http://owncloud.org/</web>
+</owncloud>'
+		);
+		$newResult = array_map('strval', $newUpdater->check());
+		
+		$this->assertArrayHasKey('version', $newResult);
+		$this->assertArrayHasKey('versionstring', $newResult);
+		$this->assertArrayHasKey('url', $newResult);
+		$this->assertArrayHasKey('web', $newResult);
+		$this->assertEquals('7.0.3.4', $newResult['version']);
+		$this->assertEquals('ownCloud 7.0.3', $newResult['versionstring']);
+		$this->assertEquals('http://download.owncloud.org/community/owncloud-7.0.3.zip', $newResult['url']);
+		$this->assertEquals('http://owncloud.org/', $newResult['web']);
+	}
+	
+	protected function getUpdaterMock($content){
+		// Invalidate cache
+		$mockedAppConfig = $this->getMockBuilder('\OC\AppConfig')
+				->disableOriginalConstructor()
+				->getMock()
+		;
+		
+		$mockedHTTPHelper = $this->getMockBuilder('\OC\HTTPHelper')
+				->setConstructorArgs(array(\OC::$server->getConfig()))
+				->getMock()
+		;
+		
+		$mockedHTTPHelper->method('getUrlContent')
+				->willReturn($content)
+		;
+		return new Updater($mockedHTTPHelper, $mockedAppConfig);
+	}
+
 }