From fa0067f833193ff14397199977ede01dd9a9a925 Mon Sep 17 00:00:00 2001 From: Kostiantyn Miakshyn Date: Wed, 6 Nov 2024 18:57:47 +0100 Subject: [PATCH] fix: simplify mime-type checks to support jpg and other image formats #2399 Signed-off-by: Kostiantyn Miakshyn --- lib/Controller/ApiController.php | 14 +++++++------- lib/Service/FormsService.php | 16 ++++++---------- tests/Unit/Service/FormsServiceTest.php | 19 ++----------------- 3 files changed, 15 insertions(+), 34 deletions(-) diff --git a/lib/Controller/ApiController.php b/lib/Controller/ApiController.php index 00f08d73..b3188619 100644 --- a/lib/Controller/ApiController.php +++ b/lib/Controller/ApiController.php @@ -764,7 +764,7 @@ class ApiController extends OCSController { $this->logger->debug('This form is archived and can not be modified'); throw new OCSForbiddenException(); } - + try { $question = $this->questionMapper->findById($questionId); } catch (IMapperException $e) { @@ -899,7 +899,7 @@ class ApiController extends OCSController { $this->logger->debug('This form is archived and can not be modified'); throw new OCSForbiddenException(); } - + try { $option = $this->optionMapper->findById($optionId); $question = $this->questionMapper->findById($questionId); @@ -961,14 +961,14 @@ class ApiController extends OCSController { $this->logger->debug('The given array contains duplicates'); throw new OCSBadRequestException('The given array contains duplicates'); } - + $options = $this->optionMapper->findByQuestion($questionId); - + if (sizeof($options) !== sizeof($newOrder)) { $this->logger->debug('The length of the given array does not match the number of stored options'); throw new OCSBadRequestException('The length of the given array does not match the number of stored options'); } - + $options = []; // Clear Array of Entities $response = []; // Array of ['optionId' => ['order' => newOrder]] @@ -1011,7 +1011,7 @@ class ApiController extends OCSController { } $this->formMapper->update($form); - + return new DataResponse($response); } @@ -1365,7 +1365,7 @@ class ApiController extends OCSController { $valid = false; foreach ($extraSettings['allowedFileTypes'] ?? [] as $allowedFileType) { - if (str_starts_with($aliases[$mimeType] ?? '', $allowedFileType)) { + if (str_starts_with($mimeType, $allowedFileType) || str_starts_with($aliases[$mimeType] ?? '', $allowedFileType)) { $valid = true; break; } diff --git a/lib/Service/FormsService.php b/lib/Service/FormsService.php index 540d545d..4098c89b 100644 --- a/lib/Service/FormsService.php +++ b/lib/Service/FormsService.php @@ -39,7 +39,6 @@ use OCA\Forms\Events\FormSubmittedEvent; use OCP\AppFramework\Db\DoesNotExistException; use OCP\AppFramework\Db\IMapperException; use OCP\EventDispatcher\IEventDispatcher; -use OCP\Files\IMimeTypeDetector; use OCP\Files\IRootFolder; use OCP\Files\NotFoundException; use OCP\IGroup; @@ -72,7 +71,6 @@ class FormsService { private CirclesService $circlesService, private IRootFolder $rootFolder, private IL10N $l10n, - private IMimeTypeDetector $mimeTypeDetector, private IEventDispatcher $eventDispatcher, ) { $this->currentUser = $userSession->getUser(); @@ -124,10 +122,9 @@ class FormsService { $question['accept'] = []; if ($question['type'] === Constants::ANSWER_TYPE_FILE) { if ($question['extraSettings']['allowedFileTypes'] ?? null) { - $question['accept'] = array_keys(array_intersect( - $this->mimeTypeDetector->getAllAliases(), - $question['extraSettings']['allowedFileTypes'] - )); + $question['accept'] = array_map(function (string $fileType) { + return str_contains($fileType, '/') ? $fileType : $fileType . '/*'; + }, $question['extraSettings']['allowedFileTypes']); } if ($question['extraSettings']['allowedFileExtensions'] ?? null) { @@ -161,10 +158,9 @@ class FormsService { $question['accept'] = []; if ($question['type'] === Constants::ANSWER_TYPE_FILE) { if ($question['extraSettings']['allowedFileTypes'] ?? null) { - $question['accept'] = array_keys(array_intersect( - $this->mimeTypeDetector->getAllAliases(), - $question['extraSettings']['allowedFileTypes'] - )); + $question['accept'] = array_map(function (string $fileType) { + return str_contains($fileType, '/') ? $fileType : $fileType . '/*'; + }, $question['extraSettings']['allowedFileTypes']); } if ($question['extraSettings']['allowedFileExtensions'] ?? null) { diff --git a/tests/Unit/Service/FormsServiceTest.php b/tests/Unit/Service/FormsServiceTest.php index 34933117..551a6bcd 100644 --- a/tests/Unit/Service/FormsServiceTest.php +++ b/tests/Unit/Service/FormsServiceTest.php @@ -65,7 +65,6 @@ use OCA\Forms\Service\FormsService; use OCP\AppFramework\Db\DoesNotExistException; use OCP\EventDispatcher\IEventDispatcher; use OCP\Files\Folder; -use OCP\Files\IMimeTypeDetector; use OCP\Files\IRootFolder; use OCP\Files\NotFoundException; use OCP\IGroup; @@ -124,9 +123,6 @@ class FormsServiceTest extends TestCase { /** @var IL10N|MockObject */ private $l10n; - /** @var IMimeTypeDetector|MockObject */ - private $mimeTypeDetector; - public function setUp(): void { parent::setUp(); $this->activityManager = $this->createMock(ActivityManager::class); @@ -160,8 +156,6 @@ class FormsServiceTest extends TestCase { return $identity; })); - $this->mimeTypeDetector = $this->createMock(IMimeTypeDetector::class); - $this->formsService = new FormsService( $userSession, $this->activityManager, @@ -177,7 +171,6 @@ class FormsServiceTest extends TestCase { $this->circlesService, $this->storage, $this->l10n, - $this->mimeTypeDetector, \OCP\Server::get(IEventDispatcher::class), ); } @@ -653,7 +646,6 @@ class FormsServiceTest extends TestCase { $this->circlesService, $this->storage, $this->l10n, - $this->mimeTypeDetector, \OCP\Server::get(IEventDispatcher::class), ); @@ -893,7 +885,6 @@ class FormsServiceTest extends TestCase { $this->circlesService, $this->storage, $this->l10n, - $this->mimeTypeDetector, \OCP\Server::get(IEventDispatcher::class), ); @@ -1005,7 +996,6 @@ class FormsServiceTest extends TestCase { $this->circlesService, $this->storage, $this->l10n, - $this->mimeTypeDetector, \OCP\Server::get(IEventDispatcher::class), ); @@ -1237,7 +1227,6 @@ class FormsServiceTest extends TestCase { $this->circlesService, $this->storage, $this->l10n, - $this->mimeTypeDetector, $eventDispatcher, ]) ->getMock(); @@ -1496,22 +1485,18 @@ class FormsServiceTest extends TestCase { $this->createQuestionEntity(['id' => 1, 'type' => 'text']), $this->createQuestionEntity(['id' => 2, 'type' => Constants::ANSWER_TYPE_FILE, 'extraSettings' => [ 'allowedFileTypes' => ['image', 'x-office/document'], - 'allowedFileExtensions' => ['jpg'] + 'allowedFileExtensions' => ['pdf'] ]]) ]; $this->questionMapper->method('findByForm')->willReturn($questionEntities); - $this->mimeTypeDetector->method('getAllAliases')->willReturn([ - 'application/coreldraw' => 'image', - 'application/msonenote' => 'x-office/document', - ]); $result = $this->formsService->getQuestions(1); $this->assertCount(2, $result); $this->assertEquals('text', $result[0]['type']); $this->assertEquals(Constants::ANSWER_TYPE_FILE, $result[1]['type']); - $this->assertEquals(['application/coreldraw', 'application/msonenote', '.jpg'], $result[1]['accept']); + $this->assertEquals(['image/*', 'x-office/document', '.pdf'], $result[1]['accept']); } public function testGetQuestionsHandlesDoesNotExistException(): void {