diff --git a/apps/theming/lib/Controller/IconController.php b/apps/theming/lib/Controller/IconController.php index 7c4e209d0d..5532a08c83 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 42a7031f40..4383ccc7cd 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 add11df3e6..c6a40b5942 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()); } }