From 88a12d91dfe723537e2ad49c30a1373ca9f77671 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Guillaume=20Perr=C3=83=C2=A9al?= <guillaume.perreal@irstea.fr> Date: Wed, 25 Jul 2018 17:00:13 +0200 Subject: [PATCH] =?UTF-8?q?Correction=20de=20d=C3=A9fauts=20de=20typage,?= =?UTF-8?q?=20comparaison=20et=20CS.?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- src/Command/CheckCommand.php | 4 +++- src/Command/CollectGarbageCommand.php | 4 ++-- src/Command/CreateCommand.php | 18 ++++++++++-------- src/Command/ReadCommand.php | 8 ++++++-- src/Controller/UploadedFileController.php | 12 ++++++++++-- src/DependencyInjection/Configuration.php | 5 ++++- src/Entity/UploadedFile.php | 2 +- .../DataTranformer/UploadedFileTransformer.php | 16 ++++++++++++---- src/Http/UploadedFileResponse.php | 14 +++++++------- src/Listener/VirusScannerListener.php | 9 +++++++-- src/Model/UploadedFileInterface.php | 2 +- src/Service/FileManager.php | 12 ++++++++++-- src/Service/FileUrlGenerator.php | 2 +- src/Service/FileUrlGeneratorInterface.php | 6 +++--- src/Utils/MimeTypeIcon.php | 8 ++++++-- src/Validation/FileMimeTypeValidator.php | 2 +- src/Validation/FileSizeValidator.php | 6 +++--- 17 files changed, 87 insertions(+), 43 deletions(-) diff --git a/src/Command/CheckCommand.php b/src/Command/CheckCommand.php index b543b631..efd3e005 100644 --- a/src/Command/CheckCommand.php +++ b/src/Command/CheckCommand.php @@ -6,6 +6,7 @@ namespace Irstea\FileUploadBundle\Command; +use Doctrine\ORM\EntityManagerInterface; use Irstea\FileUploadBundle\Model\FileManagerInterface; use Symfony\Bundle\FrameworkBundle\Command\ContainerAwareCommand; use Symfony\Component\Console\Helper\ProgressBar; @@ -32,9 +33,10 @@ class CheckCommand extends ContainerAwareCommand */ protected function execute(InputInterface $input, OutputInterface $output) { - /* @var $manager FileManagerInterface */ + /** @var FileManagerInterface $manager */ $manager = $this->getContainer()->get('irstea_file_upload.file_manager'); + /** @var EntityManagerInterface $em */ $em = $this->getContainer()->get('doctrine.orm.entity_manager'); $files = $manager->findFilesToValidate(); diff --git a/src/Command/CollectGarbageCommand.php b/src/Command/CollectGarbageCommand.php index 7bd569a1..5596e6fb 100644 --- a/src/Command/CollectGarbageCommand.php +++ b/src/Command/CollectGarbageCommand.php @@ -34,12 +34,12 @@ class CollectGarbageCommand extends ContainerAwareCommand */ protected function execute(InputInterface $input, OutputInterface $output) { - /* @var $manager FileManagerInterface */ + /** @var FileManagerInterface $manager */ $manager = $this->getContainer()->get('irstea_file_upload.file_manager'); $files = $manager->findGarbage(); - if (count($files) == 0) { + if (count($files) === 0) { $output->writeln('Aucun fichier à supprimer.'); return; diff --git a/src/Command/CreateCommand.php b/src/Command/CreateCommand.php index c5f2afd3..67a9723a 100644 --- a/src/Command/CreateCommand.php +++ b/src/Command/CreateCommand.php @@ -6,7 +6,7 @@ namespace Irstea\FileUploadBundle\Command; -use Doctrine\ORM\EntityManager; +use Doctrine\ORM\EntityManagerInterface; use Irstea\FileUploadBundle\Model\FileManagerInterface; use Symfony\Bundle\FrameworkBundle\Command\ContainerAwareCommand; use Symfony\Component\Console\Input\InputArgument; @@ -38,18 +38,18 @@ class CreateCommand extends ContainerAwareCommand */ protected function execute(InputInterface $input, OutputInterface $output) { - /* @var $em EntityManager */ + /** @var EntityManagerInterface $em */ $em = $this->getContainer()->get('doctrine.orm.entity_manager'); $written = null; $em->transactional( - function () use ($input, $output, $em, &$file, &$written) { - /* @var $manager FileManagerInterface */ + function () use ($input, $em, &$file, &$written) { + /** @var FileManagerInterface $manager */ $manager = $this->getContainer()->get('irstea_file_upload.file_manager'); $path = $input->getArgument('path'); - if ('-' === $path) { + if ($path === '-') { $realPath = 'php://stdin'; $displayName = 'stdin'; $lastModified = time(); @@ -62,15 +62,17 @@ class CreateCommand extends ContainerAwareCommand } $mimeType = $input->getOption('mime-type'); - if (null !== $forcedDisplayName = $input->getOption('display-name')) { + $forcedDisplayName = $input->getOption('display-name'); + if (null !== $forcedDisplayName) { $displayName = $forcedDisplayName; } $metadata = null; - if (null !== $jsonMetadata = $input->getOption('metadata')) { + $jsonMetadata = $input->getOption('metadata'); + if ($jsonMetadata !== null) { $metadata = json_decode($jsonMetadata); } - $file = $manager->create($displayName, $size, $mimeType ?: 'application/octet-stream', $lastModified, null, 'console'); + $file = $manager->create($displayName, $size, $mimeType ?: 'application/octet-stream', $lastModified); $fh = fopen($realPath, 'rb'); $written = $file->copyFrom($fh); diff --git a/src/Command/ReadCommand.php b/src/Command/ReadCommand.php index ca9ff5a6..dd608b27 100644 --- a/src/Command/ReadCommand.php +++ b/src/Command/ReadCommand.php @@ -37,10 +37,14 @@ class ReadCommand extends ContainerAwareCommand */ protected function execute(InputInterface $input, OutputInterface $output) { - /* @var $manager FileManagerInterface */ + /** @var FileManagerInterface $manager */ $manager = $this->getContainer()->get('irstea_file_upload.file_manager'); - $file = $manager->get($input->getArgument('id')); + $id = $input->getArgument('id'); + $file = $manager->get($id); + if ($file === null) { + throw new \RuntimeException("File not found: $id"); + } $path = $input->getArgument('filepath'); diff --git a/src/Controller/UploadedFileController.php b/src/Controller/UploadedFileController.php index 2e14b8e9..89ceccd9 100644 --- a/src/Controller/UploadedFileController.php +++ b/src/Controller/UploadedFileController.php @@ -6,9 +6,12 @@ namespace Irstea\FileUploadBundle\Controller; +use Doctrine\ORM\EntityRepository; use Irstea\FileUploadBundle\Entity\UploadedFile; use Pagerfanta\Adapter\DoctrineORMAdapter; use Pagerfanta\Pagerfanta; +use Psr\Log\LoggerInterface; +use Psr\Log\NullLogger; use Sensio\Bundle\FrameworkExtraBundle\Configuration\Method; use Sensio\Bundle\FrameworkExtraBundle\Configuration\Route; use Sensio\Bundle\FrameworkExtraBundle\Configuration\Security; @@ -39,7 +42,10 @@ class UploadedFileController extends Controller { $em = $this->getDoctrine()->getManager(); - $queryBuilder = $em->getRepository('IrsteaFileUploadBundle:UploadedFile') + /** @var EntityRepository $repo */ + $repo = $em->getRepository('IrsteaFileUploadBundle:UploadedFile'); + + $queryBuilder = $repo ->createQueryBuilder('u') ->orderBy('u.createdAt', 'DESC'); @@ -78,6 +84,8 @@ class UploadedFileController extends Controller */ public function notice($message, array $parameters = []) { - $this->get('monolog.logger.irstea_logger')->notice($message, $parameters); + /** @var LoggerInterface $logger */ + $logger = $this->get('monolog.logger.irstea_logger') ?? new NullLogger(); + $logger->notice($message, $parameters); } } diff --git a/src/DependencyInjection/Configuration.php b/src/DependencyInjection/Configuration.php index dd8e44a0..34af086f 100644 --- a/src/DependencyInjection/Configuration.php +++ b/src/DependencyInjection/Configuration.php @@ -6,6 +6,7 @@ namespace Irstea\FileUploadBundle\DependencyInjection; +use Symfony\Component\Config\Definition\Builder\ArrayNodeDefinition; use Symfony\Component\Config\Definition\Builder\TreeBuilder; use Symfony\Component\Config\Definition\ConfigurationInterface; @@ -21,11 +22,14 @@ class Configuration implements ConfigurationInterface public function getConfigTreeBuilder() { $treeBuilder = new TreeBuilder(); + + /** @var ArrayNodeDefinition $rootNode */ $rootNode = $treeBuilder->root('irstea_file_upload'); $rootNode ->children() ->integerNode('max_chunk_size') + ->min(0) ->beforeNormalization() ->ifString() ->then( @@ -45,7 +49,6 @@ class Configuration implements ConfigurationInterface ->defaultValue(0) ->treatNullLike(0) ->treatFalseLike(0) - ->min(0) ->end() ->end(); diff --git a/src/Entity/UploadedFile.php b/src/Entity/UploadedFile.php index 3f2bf22f..d98b05c3 100644 --- a/src/Entity/UploadedFile.php +++ b/src/Entity/UploadedFile.php @@ -282,7 +282,7 @@ class UploadedFile implements UploadedFileInterface ], true )) { - throw new InvalidArgumentException(sprintf("Etat invalide: '%s'", (string) $etat)); + throw new InvalidArgumentException(sprintf("Etat invalide: '%s'", $etat)); } // Déplace le fichier hors de l'orphelinat quand on passe d'orphelin à nouveau diff --git a/src/Form/DataTranformer/UploadedFileTransformer.php b/src/Form/DataTranformer/UploadedFileTransformer.php index 31ad0b8f..e57cf6b2 100644 --- a/src/Form/DataTranformer/UploadedFileTransformer.php +++ b/src/Form/DataTranformer/UploadedFileTransformer.php @@ -27,6 +27,12 @@ class UploadedFileTransformer implements DataTransformerInterface */ private $fileManager; + /** + * UploadedFileTransformer constructor. + * + * @param FileManagerInterface $fileManager + * @param bool $multiple + */ public function __construct(FileManagerInterface $fileManager, $multiple = false) { $this->fileManager = $fileManager; @@ -52,7 +58,8 @@ class UploadedFileTransformer implements DataTransformerInterface $result = []; foreach ($value as $identifier) { - if (null !== $file = $this->reverseTransformFile($identifier)) { + $file = $this->reverseTransformFile($identifier); + if ($file !== null) { $result[] = $file; } } @@ -79,7 +86,8 @@ class UploadedFileTransformer implements DataTransformerInterface $result = []; foreach ($value as $file) { - if (null !== $item = $this->transformFile($file)) { + $item = $this->transformFile($file); + if ($item !== null) { $result[] = $item; } } @@ -108,9 +116,9 @@ class UploadedFileTransformer implements DataTransformerInterface /** * @param mixed $value * - * @return UploadedFileInterface + * @return UploadedFileInterface|null */ - public function reverseTransformFile($value) + public function reverseTransformFile($value): ?UploadedFileInterface { if (empty($value)) { return null; diff --git a/src/Http/UploadedFileResponse.php b/src/Http/UploadedFileResponse.php index c8019f81..abf8deed 100644 --- a/src/Http/UploadedFileResponse.php +++ b/src/Http/UploadedFileResponse.php @@ -138,7 +138,7 @@ class UploadedFileResponse extends Response */ public function prepare(Request $request) { - $this->headers->set('Content-Length', $this->file->getSize()); + $this->headers->set('Content-Length', (string) $this->file->getSize()); if (!$this->headers->has('Accept-Ranges')) { // Only accept ranges on safe HTTP methods @@ -149,7 +149,7 @@ class UploadedFileResponse extends Response $this->headers->set('Content-Type', $this->file->getMimeType() ?: 'application/octet-stream'); } - if ('HTTP/1.0' != $request->server->get('SERVER_PROTOCOL')) { + if ($request->server->get('SERVER_PROTOCOL') !== 'HTTP/1.0') { $this->setProtocolVersion('1.1'); } @@ -160,15 +160,15 @@ class UploadedFileResponse extends Response if ($request->headers->has('Range')) { // Process the range headers. - if (!$request->headers->has('If-Range') || $this->getEtag() == $request->headers->get('If-Range')) { + if (!$request->headers->has('If-Range') || $this->getEtag() === $request->headers->get('If-Range')) { $range = $request->headers->get('Range'); $fileSize = $this->file->getSize(); [$start, $end] = explode('-', substr($range, 6), 2) + [0]; - $end = ('' === $end) ? $fileSize - 1 : (int) $end; + $end = ($end === '') ? $fileSize - 1 : (int) $end; - if ('' === $start) { + if ($start === '') { $start = $fileSize - $end; $end = $fileSize - 1; } else { @@ -184,7 +184,7 @@ class UploadedFileResponse extends Response $this->setStatusCode(206); $this->headers->set('Content-Range', sprintf('bytes %s-%s/%s', $start, $end, $fileSize)); - $this->headers->set('Content-Length', $end - $start + 1); + $this->headers->set('Content-Length', (string) ($end - $start + 1)); } } } @@ -232,6 +232,6 @@ class UploadedFileResponse extends Response */ public function getContent() { - return false; + return ''; } } diff --git a/src/Listener/VirusScannerListener.php b/src/Listener/VirusScannerListener.php index a04bcac0..c5826e62 100644 --- a/src/Listener/VirusScannerListener.php +++ b/src/Listener/VirusScannerListener.php @@ -41,15 +41,20 @@ class VirusScannerListener $file = $event->getUploadedFile(); $path = $file->getLocalPath(); + /** @var array $result */ $result = $client->scanFile($path); - $hasVirus = $result['status'] === Client::RESULT_FOUND; + + $status = $result['status'] ?? Client::RESULT_ERROR; + $hasVirus = $status === Client::RESULT_FOUND; $meta = $file->getMetadata(); $meta['virus_scanner'] = ['has_virus' => $hasVirus]; if ($hasVirus) { - $desc = $result['reason']; + $desc = $result['reason'] ?? '???'; $meta['virus_scanner']['detected'] = $result['reason']; + } else { + $desc = false; } $file->setMetadata($meta); diff --git a/src/Model/UploadedFileInterface.php b/src/Model/UploadedFileInterface.php index 154ff474..bf0ffa97 100644 --- a/src/Model/UploadedFileInterface.php +++ b/src/Model/UploadedFileInterface.php @@ -289,7 +289,7 @@ interface UploadedFileInterface public function isOrphelin(); /** Retourne la date de dernière modification dans le filesystem. - * @return DateTime + * @return DateTime|null * * @api */ diff --git a/src/Service/FileManager.php b/src/Service/FileManager.php index 0e612d5c..16c1da1e 100644 --- a/src/Service/FileManager.php +++ b/src/Service/FileManager.php @@ -74,8 +74,16 @@ final class FileManager implements FileManagerInterface $this->filesystem = $filesystem; $this->eventDispatcher = $eventDispatcher; $this->logger = $logger ?: new NullLogger(); - $this->entityManager = $doctrine->getEntityManagerForClass($entityClass); - $this->repository = $this->entityManager->getRepository($entityClass); + + $em = $doctrine->getEntityManagerForClass($entityClass); + if ($em === null) { + throw new InvalidArgumentException("Cannot find entity manager of $entityClass"); + } + $this->entityManager = $em; + + /** @var EntityRepository $repo */ + $repo = $this->entityManager->getRepository($entityClass); + $this->repository = $repo; } /** diff --git a/src/Service/FileUrlGenerator.php b/src/Service/FileUrlGenerator.php index 3cdd2dab..6a28788b 100644 --- a/src/Service/FileUrlGenerator.php +++ b/src/Service/FileUrlGenerator.php @@ -38,7 +38,7 @@ class FileUrlGenerator implements FileUrlGeneratorInterface /** * {@inheritdoc} */ - public function generate($idFile, $referenceType = UrlGeneratorInterface::ABSOLUTE_PATH) + public function generate(string $idFile, $referenceType = UrlGeneratorInterface::ABSOLUTE_PATH) { $token = $this->tokenManager->getToken(UploadController::CSRF_INTENTION); diff --git a/src/Service/FileUrlGeneratorInterface.php b/src/Service/FileUrlGeneratorInterface.php index ef3e1ee7..be06687d 100644 --- a/src/Service/FileUrlGeneratorInterface.php +++ b/src/Service/FileUrlGeneratorInterface.php @@ -16,10 +16,10 @@ interface FileUrlGeneratorInterface /** * Génère une URL sécurisée pour un fichier. * - * @param string $idFile identifiant du fichier pour lequel générer l'URL - * @param bool|int|string $referenceType type d'URL à générer + * @param string $idFile identifiant du fichier pour lequel générer l'URL + * @param int $referenceType type d'URL à générer * * @return string L'url générée */ - public function generate($idFile, $referenceType = UrlGeneratorInterface::ABSOLUTE_PATH); + public function generate(string $idFile, $referenceType = UrlGeneratorInterface::ABSOLUTE_PATH); } diff --git a/src/Utils/MimeTypeIcon.php b/src/Utils/MimeTypeIcon.php index f442d654..0ee78002 100644 --- a/src/Utils/MimeTypeIcon.php +++ b/src/Utils/MimeTypeIcon.php @@ -11,6 +11,9 @@ namespace Irstea\FileUploadBundle\Utils; */ final class MimeTypeIcon { + /** + * @var array + */ private static $icons = [ 'application/vnd.oasis.opendocument.text' => 'word', 'application/vnd.oasis.opendocument.spreadsheet' => 'excel', @@ -35,13 +38,14 @@ final class MimeTypeIcon * * @return string|null */ - public static function getMimeTypeIcon($mimeType) + public static function getMimeTypeIcon($mimeType): ?string { if (!is_string($mimeType)) { return null; } - if (false !== $sep = strpos($mimeType, ';')) { + $sep = strpos($mimeType, ';'); + if ($sep !== false) { $mimeType = trim(substr($mimeType, 0, $sep)); } diff --git a/src/Validation/FileMimeTypeValidator.php b/src/Validation/FileMimeTypeValidator.php index 136d7191..2f2a4c0e 100644 --- a/src/Validation/FileMimeTypeValidator.php +++ b/src/Validation/FileMimeTypeValidator.php @@ -20,7 +20,7 @@ class FileMimeTypeValidator extends ConstraintValidator */ public function validate($value, Constraint $constraint) { - if (!$value instanceof UploadedFileInterface) { + if (!($value instanceof UploadedFileInterface) || !($constraint instanceof FileMimeType)) { return; } if (preg_match('@^' . $constraint->acceptedMimeTypes . '$@i', $value->getMimeType())) { diff --git a/src/Validation/FileSizeValidator.php b/src/Validation/FileSizeValidator.php index 177333f1..974f3a94 100644 --- a/src/Validation/FileSizeValidator.php +++ b/src/Validation/FileSizeValidator.php @@ -20,19 +20,19 @@ class FileSizeValidator extends ConstraintValidator */ public function validate($value, Constraint $constraint) { - if (!$value instanceof UploadedFileInterface) { + if (!($value instanceof UploadedFileInterface) || !($constraint instanceof FileSize)) { return; } if ($constraint->min && $value->getSize() < $constraint->min) { $this->context->buildViolation($constraint->minMessage) ->setParameter('%displayName%', $value->getDisplayName()) - ->setParameter('%min%', $constraint->min) + ->setParameter('%min%', (string) $constraint->min) ->addViolation(); } if ($constraint->max && $value->getSize() > $constraint->max) { $this->context->buildViolation($constraint->maxMessage) ->setParameter('%displayName%', $value->getDisplayName()) - ->setParameter('%max%', $constraint->max) + ->setParameter('%max%', (string) $constraint->max) ->addViolation(); } } -- GitLab