diff --git a/README.md b/README.md index bba5b63..cbc7094 100644 --- a/README.md +++ b/README.md @@ -70,7 +70,7 @@ You must include the routing into `config/routes.yaml`: ```yaml presta_image: - resource: "@PrestaImageBundle/Resources/config/routing.yaml" + resource: "@PrestaImageBundle/config/routing.yaml" ``` See VichUploader [documentation][2] to configure the bundle. @@ -82,8 +82,8 @@ See Cropper.js [documentation][3] to install assets. Don't forget to include the following assets in your page: -- `@PrestaImageBundle/Resources/public/css/cropper.css` -- `@PrestaImageBundle/Resources/public/js/cropper.js` +- `@PrestaImageBundle/public/css/cropper.css` +- `@PrestaImageBundle/public/js/cropper.js` ### How to: implementation examples diff --git a/src/Controller/UrlToBase64Controller.php b/src/Controller/UrlToBase64Controller.php index 456db69..48d8820 100644 --- a/src/Controller/UrlToBase64Controller.php +++ b/src/Controller/UrlToBase64Controller.php @@ -2,6 +2,7 @@ namespace Presta\ImageBundle\Controller; +use Presta\ImageBundle\Exception\UnexpectedTypeException; use Presta\ImageBundle\Helper\Base64Helper; use Symfony\Component\HttpFoundation\JsonResponse; use Symfony\Component\HttpFoundation\Request; @@ -16,10 +17,11 @@ public function __invoke(Request $request): JsonResponse throw new \RuntimeException('Parameter "url" is required.'); } - return new JsonResponse( - [ - 'base64' => $this->contentToBase64($request->request->getAlpha('url')), - ] - ); + $url = $request->request->get('url'); + if (!\is_string($url)) { + throw new UnexpectedTypeException($url, 'string'); + } + + return new JsonResponse(['base64' => $this->contentToBase64($url)]); } } diff --git a/src/Form/Type/ImageType.php b/src/Form/Type/ImageType.php index 6538784..dd8804c 100644 --- a/src/Form/Type/ImageType.php +++ b/src/Form/Type/ImageType.php @@ -178,7 +178,7 @@ private function generateDownloadUri(FormInterface $form): ?string { $parent = $form->getParent(); if (null === $parent) { - return null; + throw new \RuntimeException(get_class($form) . ' should not be used as root form.'); } $data = $parent->getData(); @@ -186,9 +186,7 @@ private function generateDownloadUri(FormInterface $form): ?string return null; } - if (!\is_array($data) && !\is_object($data)) { - throw new UnexpectedTypeException($data, 'array|object'); - } + \assert(\is_array($data) || \is_object($data)); return $this->storage->resolveUri($data, $form->getName()); } diff --git a/src/Helper/Base64Helper.php b/src/Helper/Base64Helper.php index 4c46906..02a2d28 100644 --- a/src/Helper/Base64Helper.php +++ b/src/Helper/Base64Helper.php @@ -7,19 +7,19 @@ trait Base64Helper private function contentToBase64(string $filename): string { $imageData = \file_get_contents($filename); + // @codeCoverageIgnoreStart if (!\is_string($imageData)) { throw new \RuntimeException('Could not read the file\'s content.'); } + // @codeCoverageIgnoreEnd $imageInfo = getimagesizefromstring($imageData); - $mimeType = $imageInfo['mime'] ?? 'image/png'; - - if (false === \strpos($mimeType, 'image/')) { + if (false === $imageInfo || !array_key_exists('mime', $imageInfo)) { throw new \RuntimeException('The file does not seem to be an image.'); } $base64 = base64_encode($imageData); - return "data:$mimeType;base64,$base64"; + return "data:{$imageInfo['mime']};base64,$base64"; } } diff --git a/src/Model/AspectRatio.php b/src/Model/AspectRatio.php index ba61789..3d515d4 100644 --- a/src/Model/AspectRatio.php +++ b/src/Model/AspectRatio.php @@ -15,16 +15,25 @@ public function __construct(?float $value, string $label, bool $checked = false) $this->checked = $checked; } + /** + * @codeCoverageIgnore + */ public function getValue(): ?float { return $this->value; } + /** + * @codeCoverageIgnore + */ public function getLabel(): string { return $this->label; } + /** + * @codeCoverageIgnore + */ public function isChecked(): bool { return $this->checked; diff --git a/src/PrestaImageBundle.php b/src/PrestaImageBundle.php index a0cb371..9402d4d 100644 --- a/src/PrestaImageBundle.php +++ b/src/PrestaImageBundle.php @@ -6,6 +6,9 @@ class PrestaImageBundle extends Bundle { + /** + * @codeCoverageIgnore + */ public function getPath(): string { return \dirname(__DIR__); diff --git a/tests/App/Model/Book.php b/tests/App/Model/Book.php index a80deed..7d3fc6d 100644 --- a/tests/App/Model/Book.php +++ b/tests/App/Model/Book.php @@ -15,14 +15,15 @@ private function __construct() { } - public static function withoutFile(): self + public static function empty(): self { return new self(); } - public static function withFile(string $imageName): self + public static function illustrated(string $imageName): self { $book = new self(); + $book->image = new File("/tmp/$imageName", false); $book->imageName = $imageName; return $book; diff --git a/tests/App/Resources/files/dummy.pdf b/tests/App/Resources/files/dummy.pdf new file mode 100644 index 0000000..774c2ea Binary files /dev/null and b/tests/App/Resources/files/dummy.pdf differ diff --git a/tests/Unit/Controller/UrlToBase64ControllerTest.php b/tests/Unit/Controller/UrlToBase64ControllerTest.php new file mode 100644 index 0000000..bd1ed16 --- /dev/null +++ b/tests/Unit/Controller/UrlToBase64ControllerTest.php @@ -0,0 +1,74 @@ +expectException(\RuntimeException::class); + + $controller = new UrlToBase64Controller(); + $controller(Request::create('/url_to_base64', Request::METHOD_POST)); + } + + public function testShouldCauseAnExceptionIfTheUrlParameterIsNotAString(): void + { + $this->expectException(UnexpectedTypeException::class); + + $controller = new UrlToBase64Controller(); + $controller(Request::create('/url_to_base64', Request::METHOD_POST, ['url' => false])); + } + + /** + * Note: the "url" parameter should be a valid url, but technically nothing prevents a user to path a valid file + * path to get the file's contents. + * Restricting the reading to images prevents a major security issue. + */ + public function testShouldCauseAnExceptionIfTheUrlDoesNotReferenceAnImage(): void + { + $this->expectException(\RuntimeException::class); + + $controller = new UrlToBase64Controller(); + $controller( + Request::create( + '/url_to_base64', + Request::METHOD_POST, + ['url' => dirname(__DIR__) . '/../App/Resources/files/dummy.pdf'] + ) + ); + } + + public function testShouldReturnAJsonResponseContainingTheImageBase64Representation(): void + { + $controller = new UrlToBase64Controller(); + $response = $controller( + Request::create( + '/url_to_base64', + Request::METHOD_POST, + ['url' => dirname(__DIR__) . '/../App/Resources/images/A.jpg'] + ) + ); + + self::assertInstanceOf(JsonResponse::class, $response); + self::assertSame(Response::HTTP_OK, $response->getStatusCode()); + + $content = $response->getContent(); + self::assertIsString($content); + + $data = json_decode($content, true); + self::assertIsArray($data); + + self::assertArrayHasKey('base64', $data); + self::assertStringStartsWith('data:image/jpeg;base64', $data['base64']); + } +} diff --git a/tests/Unit/Form/EventListener/ImageType/AddDeleteCheckboxListenerTest.php b/tests/Unit/Form/EventListener/ImageType/AddDeleteCheckboxListenerTest.php index 3bfc23a..c4b1b0e 100644 --- a/tests/Unit/Form/EventListener/ImageType/AddDeleteCheckboxListenerTest.php +++ b/tests/Unit/Form/EventListener/ImageType/AddDeleteCheckboxListenerTest.php @@ -19,11 +19,13 @@ final class AddDeleteCheckboxListenerTest extends ImageTypeTestCase */ public function testAnImageTypeChildShouldHaveADeleteCheckboxIfCreated(Book $data): void { + \assert(null !== $data->image); + $this->storage ->expects($this->once()) ->method('resolvePath') ->with($data, 'image') - ->willReturn('/tmp/foo.png') + ->willReturn($data->image->getPathname()) ; $form = $this->factory->create(FormType::class, $data)->add('image', ImageType::class); @@ -73,12 +75,12 @@ public function testShouldCauseAnExceptionIfCreatedWithAnArrayAsData(): void public function deletableData(): iterable { - yield 'an object related to a file stored on the filesystem' => [Book::withoutFile()]; + yield 'an object related to a file stored on the filesystem' => [Book::illustrated('foo.png')]; } public function notDeletableData(): iterable { yield 'no data (null)' => [null]; - yield 'an object not related to a file stored on the filesystem' => [Book::withoutFile()]; + yield 'an object not related to a file stored on the filesystem' => [Book::empty()]; } } diff --git a/tests/Unit/Form/EventListener/ImageType/DeleteFileListenerTest.php b/tests/Unit/Form/EventListener/ImageType/DeleteFileListenerTest.php index c26ea26..fb4dc3b 100644 --- a/tests/Unit/Form/EventListener/ImageType/DeleteFileListenerTest.php +++ b/tests/Unit/Form/EventListener/ImageType/DeleteFileListenerTest.php @@ -20,11 +20,13 @@ final class DeleteFileListenerTest extends ImageTypeTestCase */ public function testShouldTriggerRemovingTheFileFromTheFilesystemIfSubmitted(Book $data, bool $delete): void { + \assert(null !== $data->image); + $this->storage ->expects($this->once()) ->method('resolvePath') ->with($data, 'image') - ->willReturn($data->imageName) + ->willReturn($data->image->getPathname()) ; $this->storage @@ -92,7 +94,7 @@ public function testShouldCauseAnExceptionIfCreatedWithAnArrayAsData(): void public function deletableConfig(): iterable { yield 'the "delete" checkbox checked when created with an object related to an existing file' => [ - Book::withFile('/tmp/foo.png'), + Book::illustrated('foo.png'), true, ]; } @@ -100,7 +102,7 @@ public function deletableConfig(): iterable public function notDeletableConfig(): iterable { yield 'no data (null)' => [null, self::ALLOW_DELETE_OPTIONS]; - yield 'no "delete" checkbox' => [Book::withoutFile(), ['allow_delete' => false]]; - yield 'the "delete" checkbox not checked' => [Book::withoutFile(), self::ALLOW_DELETE_OPTIONS, false]; + yield 'no "delete" checkbox' => [Book::illustrated('foo.png'), ['allow_delete' => false]]; + yield 'the "delete" checkbox not checked' => [Book::illustrated('foo.png'), self::ALLOW_DELETE_OPTIONS, false]; } } diff --git a/tests/Unit/Form/ImageTypeTestCase.php b/tests/Unit/Form/ImageTypeTestCase.php index a606375..abdf3bf 100644 --- a/tests/Unit/Form/ImageTypeTestCase.php +++ b/tests/Unit/Form/ImageTypeTestCase.php @@ -22,6 +22,7 @@ abstract class ImageTypeTestCase extends TypeTestCase { protected const ALLOW_DELETE_OPTIONS = ['allow_delete' => true, 'required' => false]; + protected const ALLOW_DOWNLOAD_OPTIONS = ['download_link' => true]; /** * @var MockObject&StorageInterface diff --git a/tests/Unit/Form/Type/ImageTypeTest.php b/tests/Unit/Form/Type/ImageTypeTest.php index 7aeb3c9..fedfe1d 100644 --- a/tests/Unit/Form/Type/ImageTypeTest.php +++ b/tests/Unit/Form/Type/ImageTypeTest.php @@ -17,10 +17,12 @@ final class ImageTypeTest extends ImageTypeTestCase */ public function testShouldHaveADeleteCheckboxIfCreated(bool $allowDelete, bool $required): void { - $this->storage->method('resolvePath')->willReturn('/tmp/foo.png'); - + $data = Book::illustrated('foo.png'); $options = ['allow_delete' => $allowDelete, 'required' => $required]; - $data = Book::withoutFile(); + + \assert(null !== $data->image); + + $this->storage->method('resolvePath')->willReturn($data->image->getPathname()); $form = $this->factory->create(FormType::class, $data)->add('image', ImageType::class, $options); @@ -72,14 +74,16 @@ public function testShouldNotClearTheBase64SubmittedDataIfCreated(bool $allowDel */ public function testShouldRemoveTheFileFromTheFilesystemIfCreated(bool $allowDelete, bool $required): void { + $data = Book::illustrated('foo.png'); $options = ['allow_delete' => $allowDelete, 'required' => $required]; - $data = Book::withFile('/tmp/foo.png'); + + \assert(null !== $data->image); $this->storage ->expects($this->once()) ->method('resolvePath') ->with($data, 'image') - ->willReturn($data->imageName) + ->willReturn($data->image->getPathname()) ; $this->storage @@ -109,6 +113,57 @@ public function testShouldNotRemoveTheFileFromTheFilesystemIfCreated(bool $allow $this->assertTrue($form->isSynchronized()); } + /** + * @dataProvider downloadableConfig + */ + public function testShouldAddDownloadUriToTheViewVars(Book $data, bool $downloadLink): void + { + \assert(null !== $data->imageName); + + $expected = "/book/$data->imageName"; + $options = ['download_link' => $downloadLink]; + + $form = $this->factory + ->create(FormType::class, $data) + ->add('image', ImageType::class, $options) + ; + + $this->storage + ->expects($this->once()) + ->method('resolveUri') + ->with($data, 'image') + ->willReturn($expected) + ; + + $view = $form->createView(); + + self::assertArrayHasKey('download_uri', $view->children['image']->vars); + self::assertSame($expected, $view->children['image']->vars['download_uri']); + } + + /** + * @dataProvider notDownloadableConfig + */ + public function testShouldNotAddDownloadUriToTheViewVars(?Book $data, array $options): void + { + $this->storage->expects($this->never())->method('resolveUri'); + + $form = $this->factory->create(FormType::class, $data)->add('image', ImageType::class, $options); + + $view = $form->createView(); + + self::assertArrayNotHasKey('download_uri', $view->children['image']->vars); + } + + public function testShouldCauseAnExceptionIfCreatedAsRootForm(): void + { + $this->expectException(\RuntimeException::class); + + $form = $this->factory->create(ImageType::class); + + $form->createView(); + } + public function deletableOptions(): iterable { yield 'option "allow_delete" set to true and option "required" set to false' => [true, false]; @@ -120,4 +175,18 @@ public function notDeletableOptions(): iterable yield 'option "allow_delete" set to false and option "required" set to true' => [false, true]; yield 'option "allow_delete" set to true and option "required" set to true' => [true, true]; } + + public function downloadableConfig(): iterable + { + yield 'the "download_link" option set to true when created with an object related to an existing file' => [ + Book::illustrated('foo.png'), + true, + ]; + } + + public function notDownloadableConfig(): iterable + { + yield 'no data (null)' => [null, self::ALLOW_DELETE_OPTIONS]; + yield 'no download link' => [Book::illustrated('foo.png'), ['download_link' => false]]; + } }