Issue #94: Update the documentation to reflect the new directory stru…

…cture
prestaconcept · Feb 21, 2023 · c45ddb5 · c45ddb5
1 parent 1f09f9b
commit c45ddb5
Show file tree

Hide file tree

Showing 13 changed files with 191 additions and 30 deletions.
diff --git 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
@@ -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
@@ -178,17 +178,15 @@ 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();
         if (null === $data) {
             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
@@ -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
@@ -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
@@ -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
@@ -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
diff --git a/tests/Unit/Controller/UrlToBase64ControllerTest.php b/tests/Unit/Controller/UrlToBase64ControllerTest.php
@@ -0,0 +1,74 @@
+<?php
+
+declare(strict_types=1);
+
+namespace Presta\ImageBundle\Tests\Unit\Controller;
+
+use PHPUnit\Framework\TestCase;
+use Presta\ImageBundle\Controller\UrlToBase64Controller;
+use Presta\ImageBundle\Exception\UnexpectedTypeException;
+use Symfony\Component\HttpFoundation\JsonResponse;
+use Symfony\Component\HttpFoundation\Request;
+use Symfony\Component\HttpFoundation\Response;
+
+final class UrlToBase64ControllerTest extends TestCase
+{
+    public function testShouldCauseAnExceptionIfTheRequestDoesNotHaveAUrlParameterInItsBody(): void
+    {
+        $this->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
@@ -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
@@ -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,15 +94,15 @@ 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,
         ];
     }
 
     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
@@ -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