Merge pull request #4888 from nextcloud/theming-fallback-icons

Fallback to default favicon

3 files changed, 85 insertions, 37 deletions

diff --git a/apps/theming/lib/Controller/IconController.php b/apps/theming/lib/Controller/IconController.php
index 7c4e209d0df..5532a08c83c 100644
--- a/apps/theming/lib/Controller/IconController.php
+++ b/apps/theming/lib/Controller/IconController.php
@@ -22,6 +22,7 @@
  */
 namespace OCA\Theming\Controller;
 
+use OC\IntegrityCheck\Helpers\FileAccessHelper;
 use OCA\Theming\IconBuilder;
 use OCA\Theming\ImageManager;
 use OCA\Theming\ThemingDefaults;
@@ -29,6 +30,7 @@ use OCP\AppFramework\Controller;
 use OCP\AppFramework\Http;
 use OCP\AppFramework\Http\NotFoundResponse;
 use OCP\AppFramework\Http\FileDisplayResponse;
+use OCP\AppFramework\Http\DataDisplayResponse;
 use OCP\AppFramework\Utility\ITimeFactory;
 use OCP\Files\NotFoundException;
 use OCP\IRequest;
@@ -48,6 +50,8 @@ class IconController extends Controller {
 	private $iconBuilder;
 	/** @var ImageManager */
 	private $imageManager;
+	/** @var FileAccessHelper */
+	private $fileAccessHelper;
 
 	/**
 	 * IconController constructor.
@@ -69,7 +73,8 @@ class IconController extends Controller {
 		ITimeFactory $timeFactory,
 		IConfig $config,
 		IconBuilder $iconBuilder,
-		ImageManager $imageManager
+		ImageManager $imageManager,
+		FileAccessHelper $fileAccessHelper
 	) {
 		parent::__construct($appName, $request);
 
@@ -79,6 +84,7 @@ class IconController extends Controller {
 		$this->config = $config;
 		$this->iconBuilder = $iconBuilder;
 		$this->imageManager = $imageManager;
+		$this->fileAccessHelper = $fileAccessHelper;
 	}
 
 	/**
@@ -120,9 +126,10 @@ class IconController extends Controller {
 	 * @NoCSRFRequired
 	 *
 	 * @param $app string app name
-	 * @return FileDisplayResponse|NotFoundResponse
+	 * @return FileDisplayResponse|DataDisplayResponse
 	 */
 	public function getFavicon($app = "core") {
+		$response = null;
 		if ($this->themingDefaults->shouldReplaceIcons()) {
 			try {
 				$iconFile = $this->imageManager->getCachedImage('favIcon-' . $app);
@@ -132,16 +139,19 @@ class IconController extends Controller {
 			}
 			if ($iconFile !== false) {
 				$response = new FileDisplayResponse($iconFile, Http::STATUS_OK, ['Content-Type' => 'image/x-icon']);
-				$response->cacheFor(86400);
-				$expires = new \DateTime();
-				$expires->setTimestamp($this->timeFactory->getTime());
-				$expires->add(new \DateInterval('PT24H'));
-				$response->addHeader('Expires', $expires->format(\DateTime::RFC2822));
-				$response->addHeader('Pragma', 'cache');
-				return $response;
 			}
 		}
-		return new NotFoundResponse();
+		if($response === null) {
+			$fallbackLogo = \OC::$SERVERROOT . '/core/img/favicon.png';
+			$response = new DataDisplayResponse($this->fileAccessHelper->file_get_contents($fallbackLogo), Http::STATUS_OK, ['Content-Type' => 'image/x-icon']);
+		}
+		$response->cacheFor(86400);
+		$expires = new \DateTime();
+		$expires->setTimestamp($this->timeFactory->getTime());
+		$expires->add(new \DateInterval('PT24H'));
+		$response->addHeader('Expires', $expires->format(\DateTime::RFC2822));
+		$response->addHeader('Pragma', 'cache');
+		return $response;
 	}
 
 	/**
@@ -154,6 +164,7 @@ class IconController extends Controller {
 	 * @return FileDisplayResponse|NotFoundResponse
 	 */
 	public function getTouchIcon($app = "core") {
+		$response = null;
 		if ($this->themingDefaults->shouldReplaceIcons()) {
 			try {
 				$iconFile = $this->imageManager->getCachedImage('touchIcon-' . $app);
@@ -163,15 +174,18 @@ class IconController extends Controller {
 			}
 			if ($iconFile !== false) {
 				$response = new FileDisplayResponse($iconFile, Http::STATUS_OK, ['Content-Type' => 'image/png']);
-				$response->cacheFor(86400);
-				$expires = new \DateTime();
-				$expires->setTimestamp($this->timeFactory->getTime());
-				$expires->add(new \DateInterval('PT24H'));
-				$response->addHeader('Expires', $expires->format(\DateTime::RFC2822));
-				$response->addHeader('Pragma', 'cache');
-				return $response;
 			}
 		}
-		return new NotFoundResponse();
+		if($response === null) {
+			$fallbackLogo = \OC::$SERVERROOT . '/core/img/favicon-touch.png';
+			$response = new DataDisplayResponse($this->fileAccessHelper->file_get_contents($fallbackLogo), Http::STATUS_OK, ['Content-Type' => 'image/png']);
+		}
+		$response->cacheFor(86400);
+		$expires = new \DateTime();
+		$expires->setTimestamp($this->timeFactory->getTime());
+		$expires->add(new \DateInterval('PT24H'));
+		$response->addHeader('Expires', $expires->format(\DateTime::RFC2822));
+		$response->addHeader('Pragma', 'cache');
+		return $response;
 	}
 }
diff --git a/apps/theming/lib/IconBuilder.php b/apps/theming/lib/IconBuilder.php
index 42a7031f405..4383ccc7cd9 100644
--- a/apps/theming/lib/IconBuilder.php
+++ b/apps/theming/lib/IconBuilder.php
@@ -53,14 +53,18 @@ class IconBuilder {
 	 * @return string|false image blob
 	 */
 	public function getFavicon($app) {
-		$icon = $this->renderAppIcon($app, 32);
-		if($icon === false) {
+		try {
+			$icon = $this->renderAppIcon($app, 32);
+			if ($icon === false) {
+				return false;
+			}
+			$icon->setImageFormat("png24");
+			$data = $icon->getImageBlob();
+			$icon->destroy();
+			return $data;
+		} catch (\ImagickException $e) {
 			return false;
 		}
-		$icon->setImageFormat("png24");
-		$data = $icon->getImageBlob();
-		$icon->destroy();
-		return $data;
 	}
 
 	/**
@@ -68,14 +72,18 @@ class IconBuilder {
 	 * @return string|false image blob
 	 */
 	public function getTouchIcon($app) {
-		$icon = $this->renderAppIcon($app, 512);
-		if($icon === false) {
+		try {
+			$icon = $this->renderAppIcon($app, 512);
+			if ($icon === false) {
+				return false;
+			}
+			$icon->setImageFormat("png24");
+			$data = $icon->getImageBlob();
+			$icon->destroy();
+			return $data;
+		} catch (\ImagickException $e) {
 			return false;
 		}
-		$icon->setImageFormat("png24");
-		$data = $icon->getImageBlob();
-		$icon->destroy();
-		return $data;
 	}
 
 	/**
diff --git a/apps/theming/tests/Controller/IconControllerTest.php b/apps/theming/tests/Controller/IconControllerTest.php
index add11df3e6d..c6a40b5942e 100644
--- a/apps/theming/tests/Controller/IconControllerTest.php
+++ b/apps/theming/tests/Controller/IconControllerTest.php
@@ -24,10 +24,12 @@ namespace OCA\Theming\Tests\Controller;
 
 
 use OC\Files\SimpleFS\SimpleFile;
+use OC\IntegrityCheck\Helpers\FileAccessHelper;
 use OCA\Theming\IconBuilder;
 use OCA\Theming\ImageManager;
 use OCA\Theming\ThemingDefaults;
 use OCP\AppFramework\Http;
+use OCP\AppFramework\Http\DataDisplayResponse;
 use OCP\AppFramework\Http\NotFoundResponse;
 use OCP\Files\NotFoundException;
 use OCP\IConfig;
@@ -53,6 +55,8 @@ class IconControllerTest extends TestCase {
 	private $config;
 	/** @var IconBuilder|\PHPUnit_Framework_MockObject_MockObject */
 	private $iconBuilder;
+	/** @var FileAccessHelper|\PHPUnit_Framework_MockObject_MockObject */
+	private $fileAccessHelper;
 	/** @var ImageManager */
 	private $imageManager;
 
@@ -69,6 +73,7 @@ class IconControllerTest extends TestCase {
 		$this->iconBuilder = $this->getMockBuilder('OCA\Theming\IconBuilder')
 			->disableOriginalConstructor()->getMock();
 		$this->imageManager = $this->getMockBuilder('OCA\Theming\ImageManager')->disableOriginalConstructor()->getMock();
+		$this->fileAccessHelper = $this->createMock(FileAccessHelper::class);
 		$this->timeFactory->expects($this->any())
 			->method('getTime')
 			->willReturn(123);
@@ -81,7 +86,8 @@ class IconControllerTest extends TestCase {
 			$this->timeFactory,
 			$this->config,
 			$this->iconBuilder,
-			$this->imageManager
+			$this->imageManager,
+			$this->fileAccessHelper
 		);
 
 		parent::setUp();
@@ -150,11 +156,19 @@ class IconControllerTest extends TestCase {
 		$this->themingDefaults->expects($this->any())
 			->method('shouldReplaceIcons')
 			->willReturn(false);
-		$expected = new Http\Response();
-		$expected->setStatus(Http::STATUS_NOT_FOUND);
-		$expected->cacheFor(0);
-		$expected->setLastModified(new \DateTime('now', new \DateTimeZone('GMT')));
-		$this->assertInstanceOf(NotFoundResponse::class, $this->iconController->getFavicon());
+		$fallbackLogo = \OC::$SERVERROOT . '/core/img/favicon.png';
+		$this->fileAccessHelper->expects($this->once())
+			->method('file_get_contents')
+			->with($fallbackLogo)
+			->willReturn(file_get_contents($fallbackLogo));
+		$expected = new DataDisplayResponse(file_get_contents($fallbackLogo), Http::STATUS_OK, ['Content-Type' => 'image/x-icon']);
+		$expected->cacheFor(86400);
+		$expires = new \DateTime();
+		$expires->setTimestamp($this->timeFactory->getTime());
+		$expires->add(new \DateInterval('PT24H'));
+		$expected->addHeader('Expires', $expires->format(\DateTime::RFC2822));
+		$expected->addHeader('Pragma', 'cache');
+		$this->assertEquals($expected, $this->iconController->getFavicon());
 	}
 
 	public function testGetTouchIconDefault() {
@@ -195,7 +209,19 @@ class IconControllerTest extends TestCase {
 		$this->themingDefaults->expects($this->any())
 			->method('shouldReplaceIcons')
 			->willReturn(false);
-		$this->assertInstanceOf(NotFoundResponse::class, $this->iconController->getTouchIcon());
+		$fallbackLogo = \OC::$SERVERROOT . '/core/img/favicon-touch.png';
+		$this->fileAccessHelper->expects($this->once())
+			->method('file_get_contents')
+			->with($fallbackLogo)
+			->willReturn(file_get_contents($fallbackLogo));
+		$expected = new DataDisplayResponse(file_get_contents($fallbackLogo), Http::STATUS_OK, ['Content-Type' => 'image/png']);
+		$expected->cacheFor(86400);
+		$expires = new \DateTime();
+		$expires->setTimestamp($this->timeFactory->getTime());
+		$expires->add(new \DateInterval('PT24H'));
+		$expected->addHeader('Expires', $expires->format(\DateTime::RFC2822));
+		$expected->addHeader('Pragma', 'cache');
+		$this->assertEquals($expected, $this->iconController->getTouchIcon());
 	}
 
 }