diff --git a/lib/Service/FileService.php b/lib/Service/FileService.php index 2112a30f1..2c5e359d3 100644 --- a/lib/Service/FileService.php +++ b/lib/Service/FileService.php @@ -27,10 +27,9 @@ use OCA\Deck\Db\Attachment; use OCA\Deck\Db\AttachmentMapper; use OCA\Deck\StatusException; use OCA\Deck\Exceptions\ConflictException; -use OCP\AppFramework\Http\ContentSecurityPolicy; -use OCP\AppFramework\Http\FileDisplayResponse; use OCP\AppFramework\Http\StreamResponse; use OCP\Files\IAppData; +use OCP\Files\IMimeTypeDetector; use OCP\Files\IRootFolder; use OCP\Files\NotFoundException; use OCP\Files\NotPermittedException; @@ -49,6 +48,7 @@ class FileService implements IAttachmentService { private $rootFolder; private $config; private $attachmentMapper; + private $mimeTypeDetector; public function __construct( IL10N $l10n, @@ -57,7 +57,8 @@ class FileService implements IAttachmentService { ILogger $logger, IRootFolder $rootFolder, IConfig $config, - AttachmentMapper $attachmentMapper + AttachmentMapper $attachmentMapper, + IMimeTypeDetector $mimeTypeDetector ) { $this->l10n = $l10n; $this->appData = $appData; @@ -66,6 +67,7 @@ class FileService implements IAttachmentService { $this->rootFolder = $rootFolder; $this->config = $config; $this->attachmentMapper = $attachmentMapper; + $this->mimeTypeDetector = $mimeTypeDetector; } /** @@ -225,27 +227,14 @@ class FileService implements IAttachmentService { /** * @param Attachment $attachment - * @return FileDisplayResponse|\OCP\AppFramework\Http\Response|StreamResponse + * @return StreamResponse * @throws \Exception */ public function display(Attachment $attachment) { $file = $this->getFileFromRootFolder($attachment); - if (method_exists($file, 'fopen')) { - $response = new StreamResponse($file->fopen('r')); - $response->addHeader('Content-Disposition', 'inline; filename="' . rawurldecode($file->getName()) . '"'); - } else { - $response = new FileDisplayResponse($file); - } - // We need those since otherwise chrome won't show the PDF file with CSP rule object-src 'none' - // https://bugs.chromium.org/p/chromium/issues/detail?id=271452 - $policy = new ContentSecurityPolicy(); - $policy->addAllowedObjectDomain('\'self\''); - $policy->addAllowedObjectDomain('blob:'); - $policy->addAllowedMediaDomain('\'self\''); - $policy->addAllowedMediaDomain('blob:'); - $response->setContentSecurityPolicy($policy); - - $response->addHeader('Content-Type', $file->getMimeType()); + $response = new StreamResponse($file->fopen('rb')); + $response->addHeader('Content-Disposition', 'attachment; filename="' . rawurldecode($file->getName()) . '"'); + $response->addHeader('Content-Type', $this->mimeTypeDetector->getSecureMimeType($file->getMimeType())); return $response; } diff --git a/lib/Service/FilesAppService.php b/lib/Service/FilesAppService.php index 089417900..ea6c06506 100644 --- a/lib/Service/FilesAppService.php +++ b/lib/Service/FilesAppService.php @@ -26,10 +26,9 @@ namespace OCA\Deck\Service; use OCA\Deck\Db\Attachment; use OCA\Deck\Sharing\DeckShareProvider; use OCA\Deck\StatusException; -use OCP\AppFramework\Http\ContentSecurityPolicy; -use OCP\AppFramework\Http\FileDisplayResponse; use OCP\AppFramework\Http\StreamResponse; use OCP\Constants; +use OCP\Files\IMimeTypeDetector; use OCP\Files\IRootFolder; use OCP\Files\NotFoundException; use OCP\IDBConnection; @@ -50,6 +49,7 @@ class FilesAppService implements IAttachmentService, ICustomAttachmentService { private $l10n; private $preview; private $permissionService; + private $mimeTypeDetector; public function __construct( IRequest $request, @@ -60,6 +60,7 @@ class FilesAppService implements IAttachmentService, ICustomAttachmentService { DeckShareProvider $shareProvider, IPreview $preview, PermissionService $permissionService, + IMimeTypeDetector $mimeTypeDetector, string $userId = null ) { $this->request = $request; @@ -70,6 +71,7 @@ class FilesAppService implements IAttachmentService, ICustomAttachmentService { $this->shareManager = $shareManager; $this->userId = $userId; $this->preview = $preview; + $this->mimeTypeDetector = $mimeTypeDetector; } public function listAttachments(int $cardId): array { @@ -147,22 +149,10 @@ class FilesAppService implements IAttachmentService, ICustomAttachmentService { if ($file === null || $share->getSharedWith() !== (string)$attachment->getCardId()) { throw new NotFoundException('File not found'); } - if (method_exists($file, 'fopen')) { - $response = new StreamResponse($file->fopen('r')); - $response->addHeader('Content-Disposition', 'inline; filename="' . rawurldecode($file->getName()) . '"'); - } else { - $response = new FileDisplayResponse($file); - } - // We need those since otherwise chrome won't show the PDF file with CSP rule object-src 'none' - // https://bugs.chromium.org/p/chromium/issues/detail?id=271452 - $policy = new ContentSecurityPolicy(); - $policy->addAllowedObjectDomain('\'self\''); - $policy->addAllowedObjectDomain('blob:'); - $policy->addAllowedMediaDomain('\'self\''); - $policy->addAllowedMediaDomain('blob:'); - $response->setContentSecurityPolicy($policy); - $response->addHeader('Content-Type', $file->getMimeType()); + $response = new StreamResponse($file->fopen('rb')); + $response->addHeader('Content-Disposition', 'attachment; filename="' . rawurldecode($file->getName()) . '"'); + $response->addHeader('Content-Type', $this->mimeTypeDetector->getSecureMimeType($file->getMimeType())); return $response; } diff --git a/tests/unit/Service/FileServiceTest.php b/tests/unit/Service/FileServiceTest.php index e8662b775..a94dd8253 100644 --- a/tests/unit/Service/FileServiceTest.php +++ b/tests/unit/Service/FileServiceTest.php @@ -25,10 +25,10 @@ namespace OCA\Deck\Service; use OCA\Deck\Db\Attachment; use OCA\Deck\Db\AttachmentMapper; -use OCP\AppFramework\Http\ContentSecurityPolicy; use OCP\AppFramework\Http\StreamResponse; use OCP\Files\Folder; use OCP\Files\IAppData; +use OCP\Files\IMimeTypeDetector; use OCP\Files\IRootFolder; use OCP\Files\SimpleFS\ISimpleFile; use OCP\Files\SimpleFS\ISimpleFolder; @@ -57,6 +57,8 @@ class FileServiceTest extends TestCase { private $config; /** @var AttachmentMapper|MockObject */ private $attachmentMapper; + /** @var IMimeTypeDetector|MockObject */ + private $mimeTypeDetector; public function setUp(): void { parent::setUp(); @@ -67,7 +69,8 @@ class FileServiceTest extends TestCase { $this->rootFolder = $this->createMock(IRootFolder::class); $this->config = $this->createMock(IConfig::class); $this->attachmentMapper = $this->createMock(AttachmentMapper::class); - $this->fileService = new FileService($this->l10n, $this->appData, $this->request, $this->logger, $this->rootFolder, $this->config, $this->attachmentMapper); + $this->mimeTypeDetector = $this->createMock(IMimeTypeDetector::class); + $this->fileService = new FileService($this->l10n, $this->appData, $this->request, $this->logger, $this->rootFolder, $this->config, $this->attachmentMapper, $this->mimeTypeDetector); } public function mockGetFolder($cardId) { @@ -268,51 +271,13 @@ class FileServiceTest extends TestCase { $file->expects($this->any()) ->method('fopen') ->willReturn('fileresource'); + $this->mimeTypeDetector->expects($this->once()) + ->method('getSecureMimeType') + ->willReturn('image/jpeg'); $actual = $this->fileService->display($attachment); $expected = new StreamResponse('fileresource'); $expected->addHeader('Content-Type', 'image/jpeg'); - $expected->addHeader('Content-Disposition', 'inline; filename="' . rawurldecode($file->getName()) . '"'); - $policy = new ContentSecurityPolicy(); - $policy->addAllowedObjectDomain('\'self\''); - $policy->addAllowedObjectDomain('blob:'); - $policy->addAllowedMediaDomain('\'self\''); - $policy->addAllowedMediaDomain('blob:'); - $expected->setContentSecurityPolicy($policy); - $this->assertEquals($expected, $actual); - } - - public function testDisplayPdf() { - $this->config->expects($this->once()) - ->method('getSystemValue') - ->willReturn('123'); - $appDataFolder = $this->createMock(Folder::class); - $deckAppDataFolder = $this->createMock(Folder::class); - $cardFolder = $this->createMock(Folder::class); - $this->rootFolder->expects($this->once())->method('get')->willReturn($appDataFolder); - $appDataFolder->expects($this->once())->method('get')->willReturn($deckAppDataFolder); - $deckAppDataFolder->expects($this->once())->method('get')->willReturn($cardFolder); - $attachment = $this->getAttachment(); - $file = $this->createMock(\OCP\Files\File::class); - $cardFolder->expects($this->once())->method('get')->willReturn($file); - $file->expects($this->any()) - ->method('getMimeType') - ->willReturn('application/pdf'); - $file->expects($this->any()) - ->method('getName') - ->willReturn('file1'); - $file->expects($this->any()) - ->method('fopen') - ->willReturn('fileresource'); - $actual = $this->fileService->display($attachment); - $expected = new StreamResponse('fileresource'); - $expected->addHeader('Content-Disposition', 'inline; filename="' . rawurldecode($file->getName()) . '"'); - $expected->addHeader('Content-Type', 'application/pdf'); - $policy = new ContentSecurityPolicy(); - $policy->addAllowedObjectDomain('\'self\''); - $policy->addAllowedObjectDomain('blob:'); - $policy->addAllowedMediaDomain('\'self\''); - $policy->addAllowedMediaDomain('blob:'); - $expected->setContentSecurityPolicy($policy); + $expected->addHeader('Content-Disposition', 'attachment; filename="' . rawurldecode($file->getName()) . '"'); $this->assertEquals($expected, $actual); }