ahtlon/deck
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases 1 Wiki Activity

Merge pull request #2847 from nextcloud/bugfix/noid/content-disposition

Browse Source 
Switch to Content-Disposition attachment and check for sane mimetypes
This commit is contained in:
Julius Härtl
2021-03-04 08:25:46 +01:00
committed by GitHub
parent 6aa63e0906 9f7901519b
commit 225a22c93e
3 changed files with 25 additions and 81 deletions
Download Patch File Download Diff File Expand all files Collapse all files

29
lib/Service/FileService.php
View File

@@ -27,10 +27,9 @@ use OCA\Deck\Db\Attachment;
use OCA\Deck\Db\AttachmentMapper; use OCA\Deck\Db\AttachmentMapper;
use OCA\Deck\StatusException; use OCA\Deck\StatusException;
use OCA\Deck\Exceptions\ConflictException; use OCA\Deck\Exceptions\ConflictException;
use OCP\AppFramework\Http\ContentSecurityPolicy;
use OCP\AppFramework\Http\FileDisplayResponse;
use OCP\AppFramework\Http\StreamResponse; use OCP\AppFramework\Http\StreamResponse;
use OCP\Files\IAppData; use OCP\Files\IAppData;
use OCP\Files\IMimeTypeDetector;
use OCP\Files\IRootFolder; use OCP\Files\IRootFolder;
use OCP\Files\NotFoundException; use OCP\Files\NotFoundException;
use OCP\Files\NotPermittedException; use OCP\Files\NotPermittedException;
@@ -49,6 +48,7 @@ class FileService implements IAttachmentService {
private $rootFolder; private $rootFolder;
private $config; private $config;
private $attachmentMapper; private $attachmentMapper;
private $mimeTypeDetector;
public function __construct( public function __construct(
IL10N $l10n, IL10N $l10n,
@@ -57,7 +57,8 @@ class FileService implements IAttachmentService {
ILogger $logger, ILogger $logger,
IRootFolder $rootFolder, IRootFolder $rootFolder,
IConfig $config, IConfig $config,
AttachmentMapper $attachmentMapper AttachmentMapper $attachmentMapper,
IMimeTypeDetector $mimeTypeDetector
) { ) {
$this->l10n = $l10n; $this->l10n = $l10n;
$this->appData = $appData; $this->appData = $appData;
@@ -66,6 +67,7 @@ class FileService implements IAttachmentService {
$this->rootFolder = $rootFolder; $this->rootFolder = $rootFolder;
$this->config = $config; $this->config = $config;
$this->attachmentMapper = $attachmentMapper; $this->attachmentMapper = $attachmentMapper;
$this->mimeTypeDetector = $mimeTypeDetector;
} }
/** /**
@@ -225,27 +227,14 @@ class FileService implements IAttachmentService {
/** /**
* @param Attachment $attachment * @param Attachment $attachment
* @return FileDisplayResponse|\OCP\AppFramework\Http\Response|StreamResponse * @return StreamResponse
* @throws \Exception * @throws \Exception
*/ */
public function display(Attachment $attachment) { public function display(Attachment $attachment) {
$file = $this->getFileFromRootFolder($attachment); $file = $this->getFileFromRootFolder($attachment);
if (method_exists($file, 'fopen')) { $response = new StreamResponse($file->fopen('rb'));
$response = new StreamResponse($file->fopen('r')); $response->addHeader('Content-Disposition', 'attachment; filename="' . rawurldecode($file->getName()) . '"');
$response->addHeader('Content-Disposition', 'inline; filename="' . rawurldecode($file->getName()) . '"'); $response->addHeader('Content-Type', $this->mimeTypeDetector->getSecureMimeType($file->getMimeType()));
} 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());
return $response; return $response;
} }

24
lib/Service/FilesAppService.php
View File

@@ -26,10 +26,9 @@ namespace OCA\Deck\Service;
use OCA\Deck\Db\Attachment; use OCA\Deck\Db\Attachment;
use OCA\Deck\Sharing\DeckShareProvider; use OCA\Deck\Sharing\DeckShareProvider;
use OCA\Deck\StatusException; use OCA\Deck\StatusException;
use OCP\AppFramework\Http\ContentSecurityPolicy;
use OCP\AppFramework\Http\FileDisplayResponse;
use OCP\AppFramework\Http\StreamResponse; use OCP\AppFramework\Http\StreamResponse;
use OCP\Constants; use OCP\Constants;
use OCP\Files\IMimeTypeDetector;
use OCP\Files\IRootFolder; use OCP\Files\IRootFolder;
use OCP\Files\NotFoundException; use OCP\Files\NotFoundException;
use OCP\IDBConnection; use OCP\IDBConnection;
@@ -50,6 +49,7 @@ class FilesAppService implements IAttachmentService, ICustomAttachmentService {
private $l10n; private $l10n;
private $preview; private $preview;
private $permissionService; private $permissionService;
private $mimeTypeDetector;
public function __construct( public function __construct(
IRequest $request, IRequest $request,
@@ -60,6 +60,7 @@ class FilesAppService implements IAttachmentService, ICustomAttachmentService {
DeckShareProvider $shareProvider, DeckShareProvider $shareProvider,
IPreview $preview, IPreview $preview,
PermissionService $permissionService, PermissionService $permissionService,
IMimeTypeDetector $mimeTypeDetector,
string $userId = null string $userId = null
) { ) {
$this->request = $request; $this->request = $request;
@@ -70,6 +71,7 @@ class FilesAppService implements IAttachmentService, ICustomAttachmentService {
$this->shareManager = $shareManager; $this->shareManager = $shareManager;
$this->userId = $userId; $this->userId = $userId;
$this->preview = $preview; $this->preview = $preview;
$this->mimeTypeDetector = $mimeTypeDetector;
} }
public function listAttachments(int $cardId): array { public function listAttachments(int $cardId): array {
@@ -147,22 +149,10 @@ class FilesAppService implements IAttachmentService, ICustomAttachmentService {
if ($file === null || $share->getSharedWith() !== (string)$attachment->getCardId()) { if ($file === null || $share->getSharedWith() !== (string)$attachment->getCardId()) {
throw new NotFoundException('File not found'); 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; return $response;
} }

53
tests/unit/Service/FileServiceTest.php
View File

@@ -25,10 +25,10 @@ namespace OCA\Deck\Service;
use OCA\Deck\Db\Attachment; use OCA\Deck\Db\Attachment;
use OCA\Deck\Db\AttachmentMapper; use OCA\Deck\Db\AttachmentMapper;
use OCP\AppFramework\Http\ContentSecurityPolicy;
use OCP\AppFramework\Http\StreamResponse; use OCP\AppFramework\Http\StreamResponse;
use OCP\Files\Folder; use OCP\Files\Folder;
use OCP\Files\IAppData; use OCP\Files\IAppData;
use OCP\Files\IMimeTypeDetector;
use OCP\Files\IRootFolder; use OCP\Files\IRootFolder;
use OCP\Files\SimpleFS\ISimpleFile; use OCP\Files\SimpleFS\ISimpleFile;
use OCP\Files\SimpleFS\ISimpleFolder; use OCP\Files\SimpleFS\ISimpleFolder;
@@ -57,6 +57,8 @@ class FileServiceTest extends TestCase {
private $config; private $config;
/** @var AttachmentMapper|MockObject */ /** @var AttachmentMapper|MockObject */
private $attachmentMapper; private $attachmentMapper;
/** @var IMimeTypeDetector|MockObject */
private $mimeTypeDetector;
public function setUp(): void { public function setUp(): void {
parent::setUp(); parent::setUp();
@@ -67,7 +69,8 @@ class FileServiceTest extends TestCase {
$this->rootFolder = $this->createMock(IRootFolder::class); $this->rootFolder = $this->createMock(IRootFolder::class);
$this->config = $this->createMock(IConfig::class); $this->config = $this->createMock(IConfig::class);
$this->attachmentMapper = $this->createMock(AttachmentMapper::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) { public function mockGetFolder($cardId) {
@@ -268,51 +271,13 @@ class FileServiceTest extends TestCase {
$file->expects($this->any()) $file->expects($this->any())
->method('fopen') ->method('fopen')
->willReturn('fileresource'); ->willReturn('fileresource');
$this->mimeTypeDetector->expects($this->once())
->method('getSecureMimeType')
->willReturn('image/jpeg');
$actual = $this->fileService->display($attachment); $actual = $this->fileService->display($attachment);
$expected = new StreamResponse('fileresource'); $expected = new StreamResponse('fileresource');
$expected->addHeader('Content-Type', 'image/jpeg'); $expected->addHeader('Content-Type', 'image/jpeg');
$expected->addHeader('Content-Disposition', 'inline; filename="' . rawurldecode($file->getName()) . '"'); $expected->addHeader('Content-Disposition', 'attachment; 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);
$this->assertEquals($expected, $actual); $this->assertEquals($expected, $actual);
} }