fix(files): more conversion tests and translate error messages

Signed-off-by: skjnldsv <skjnldsv@protonmail.com>

1 files changed, 35 insertions, 9 deletions

diff --git a/lib/private/Files/Conversion/ConversionManager.php b/lib/private/Files/Conversion/ConversionManager.php
index e6ec11b1cf4..0a9803b3f42 100644
--- a/lib/private/Files/Conversion/ConversionManager.php
+++ b/lib/private/Files/Conversion/ConversionManager.php
@@ -10,13 +10,16 @@ declare(strict_types=1);
 namespace OC\Files\Conversion;
 
 use OC\AppFramework\Bootstrap\Coordinator;
+use OC\ForbiddenException;
 use OC\SystemConfig;
 use OCP\Files\Conversion\IConversionManager;
 use OCP\Files\Conversion\IConversionProvider;
 use OCP\Files\File;
 use OCP\Files\GenericFileException;
 use OCP\Files\IRootFolder;
+use OCP\IL10N;
 use OCP\ITempManager;
+use OCP\L10N\IFactory;
 use OCP\PreConditionNotMetException;
 use Psr\Container\ContainerExceptionInterface;
 use Psr\Container\ContainerInterface;
@@ -37,6 +40,7 @@ class ConversionManager implements IConversionManager {
 	/** @var list<IConversionProvider> */
 	private array $providers = [];
 
+	private IL10N $l10n;
 	public function __construct(
 		private Coordinator $coordinator,
 		private ContainerInterface $serverContainer,
@@ -44,7 +48,9 @@ class ConversionManager implements IConversionManager {
 		private ITempManager $tempManager,
 		private LoggerInterface $logger,
 		private SystemConfig $config,
+		IFactory $l10nFactory,
 	) {
+		$this->l10n = $l10nFactory->get('files');
 	}
 
 	public function hasProviders(): bool {
@@ -62,22 +68,21 @@ class ConversionManager implements IConversionManager {
 
 	public function convert(File $file, string $targetMimeType, ?string $destination = null): string {
 		if (!$this->hasProviders()) {
-			throw new PreConditionNotMetException('No file conversion providers available');
+			throw new PreConditionNotMetException($this->l10n->t('No file conversion providers available'));
 		}
 
 		// Operate in mebibytes
 		$fileSize = $file->getSize() / (1024 * 1024);
 		$threshold = $this->config->getValue('max_file_conversion_filesize', 100);
 		if ($fileSize > $threshold) {
-			throw new GenericFileException('File is too large to convert');
+			throw new GenericFileException($this->l10n->t('File is too large to convert'));
 		}
 
 		$fileMimeType = $file->getMimetype();
 		$validProvider = $this->getValidProvider($fileMimeType, $targetMimeType);
 
 		if ($validProvider !== null) {
-			$convertedFile = $validProvider->convertFile($file, $targetMimeType);
-			
+			// Get the target extension given by the provider
 			$targetExtension = '';
 			foreach ($validProvider->getSupportedMimeTypes() as $mimeProvider) {
 				if ($mimeProvider->getTo() === $targetMimeType) {
@@ -85,7 +90,7 @@ class ConversionManager implements IConversionManager {
 					break;
 				}
 			}
-
+	
 			// If destination not provided, we use the same path
 			// as the original file, but with the new extension
 			if ($destination === null) {
@@ -94,11 +99,21 @@ class ConversionManager implements IConversionManager {
 				$destination = $parent->getFullPath($basename . '.' . $targetExtension);
 			}
 
+			// If destination doesn't match the target extension, we throw an error
+			if (pathinfo($destination, PATHINFO_EXTENSION) !== $targetExtension) {
+				throw new GenericFileException($this->l10n->t('Destination does not match conversion extension'));
+			}
+
+			// Check destination before converting
+			$this->checkDestination($destination);
+
+			// Convert the file and write it to the destination
+			$convertedFile = $validProvider->convertFile($file, $targetMimeType);
 			$convertedFile = $this->writeToDestination($destination, $convertedFile);
 			return $convertedFile->getPath();
 		}
 
-		throw new RuntimeException('Could not convert file');
+		throw new RuntimeException($this->l10n->t('Could not convert file'));
 	}
 
 	/**
@@ -127,14 +142,25 @@ class ConversionManager implements IConversionManager {
 		return array_values(array_merge([], $this->preferredProviders, $this->providers));
 	}
 
+	private function checkDestination(string $destination): void {
+		if (!$this->rootFolder->nodeExists(dirname($destination))) {
+			throw new ForbiddenException($this->l10n->t('Destination does not exist'));
+		}
+
+		$folder = $this->rootFolder->get(dirname($destination));
+		if (!$folder->isCreatable()) {
+			throw new ForbiddenException($this->l10n->t('Destination is not creatable'));
+		}
+	}
+
 	private function writeToDestination(string $destination, mixed $content): File {
+		$this->checkDestination($destination);
+
 		if ($this->rootFolder->nodeExists($destination)) {
 			$file = $this->rootFolder->get($destination);
 			$parent = $file->getParent();
-			if (!$parent->isCreatable()) {
-				throw new GenericFileException('Destination is not creatable');
-			}
 
+			// Folder permissions is already checked in checkDestination method
 			$newName = $parent->getNonExistingName(basename($destination));
 			$destination = $parent->getFullPath($newName);
 		}