fix: simplify mime-type checks to support jpg and other image formats #…

…2399 (#2422) Signed-off-by: Kostiantyn Miakshyn <[email protected]> Co-authored-by: Kostiantyn Miakshyn <[email protected]>
nextcloud · Nov 20, 2024 · 270fc5a · 270fc5a
1 parent 7587b57
commit 270fc5a
Show file tree

Hide file tree

Showing 3 changed files with 16 additions and 35 deletions.
diff --git a/lib/Controller/ApiController.php b/lib/Controller/ApiController.php
@@ -750,7 +750,7 @@ public function newOption(int $formId, int $questionId, array $optionTexts): Dat
 			$this->logger->debug('This form is archived and can not be modified');
 			throw new OCSForbiddenException();
 		}
-		
+
 		try {
 			$question = $this->questionMapper->findById($questionId);
 		} catch (IMapperException $e) {
@@ -885,7 +885,7 @@ public function deleteOption(int $formId, int $questionId, int $optionId): DataR
 			$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);
@@ -947,14 +947,14 @@ public function reorderOptions(int $formId, int $questionId, array $newOrder) {
 			$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]]
 
@@ -997,7 +997,7 @@ public function reorderOptions(int $formId, int $questionId, array $newOrder) {
 		}
 
 		$this->formMapper->update($form);
-	
+
 		return new DataResponse($response);
 	}
 
@@ -1351,7 +1351,7 @@ public function uploadFiles(int $formId, int $questionId, string $shareHash = ''
 
 				$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;
 					}
@@ -2329,7 +2329,7 @@ public function uploadFilesLegacy(int $formId, int $questionId, string $shareHas
 
 				$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
@@ -39,7 +39,6 @@
 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 @@ public function __construct(
 		private CirclesService $circlesService,
 		private IRootFolder $rootFolder,
 		private IL10N $l10n,
-		private IMimeTypeDetector $mimeTypeDetector,
 		private IEventDispatcher $eventDispatcher,
 	) {
 		$this->currentUser = $userSession->getUser();
@@ -124,10 +122,9 @@ public function getQuestions(int $formId): array {
 				$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 @@ public function getQuestion(int $questionId): array {
 			$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
@@ -65,7 +65,6 @@ function microtime(bool|float $asFloat = false) {
 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 @@ public function setUp(): void {
 				return $identity;
 			}));
 
-		$this->mimeTypeDetector = $this->createMock(IMimeTypeDetector::class);
-
 		$this->formsService = new FormsService(
 			$userSession,
 			$this->activityManager,
@@ -177,7 +171,6 @@ public function setUp(): void {
 			$this->circlesService,
 			$this->storage,
 			$this->l10n,
-			$this->mimeTypeDetector,
 			\OCP\Server::get(IEventDispatcher::class),
 		);
 	}
@@ -653,7 +646,6 @@ public function testGetPermissions_NotLoggedIn() {
 			$this->circlesService,
 			$this->storage,
 			$this->l10n,
-			$this->mimeTypeDetector,
 			\OCP\Server::get(IEventDispatcher::class),
 		);
 
@@ -893,7 +885,6 @@ public function testPublicCanSubmit() {
 			$this->circlesService,
 			$this->storage,
 			$this->l10n,
-			$this->mimeTypeDetector,
 			\OCP\Server::get(IEventDispatcher::class),
 		);
 
@@ -1005,7 +996,6 @@ public function testHasUserAccess_NotLoggedIn() {
 			$this->circlesService,
 			$this->storage,
 			$this->l10n,
-			$this->mimeTypeDetector,
 			\OCP\Server::get(IEventDispatcher::class),
 		);
 
@@ -1237,7 +1227,6 @@ public function testNotifyNewSubmission($shares, $shareNotifications) {
 				$this->circlesService,
 				$this->storage,
 				$this->l10n,
-				$this->mimeTypeDetector,
 				$eventDispatcher,
 			])
 			->getMock();
@@ -1496,22 +1485,18 @@ public function testGetQuestionsWithVariousQuestionTypes(): void {
 			$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 {