Skip to content

Commit 381ad4e

Browse files
escopeczescopecz
authored
Merge commit from fork
feat: Restrict image upload file extensions and mime types for the GrapesJsEditor (5.2)
2 parents af2d3fe + 7be4a7c commit 381ad4e

File tree

9 files changed

+149
-33
lines changed

9 files changed

+149
-33
lines changed

‎app/bundles/CoreBundle/Config/config.php‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -347,6 +347,7 @@
347347
'class' => Mautic\CoreBundle\Helper\FileUploader::class,
348348
'arguments' => [
349349
'mautic.helper.file_path_resolver',
350+
'translator',
350351
],
351352
],
352353
'mautic.helper.file_path_resolver' => [

‎app/bundles/CoreBundle/Controller/FileController.php‎

Lines changed: 18 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,8 @@
66
use Mautic\CoreBundle\Helper\FileUploader;
77
use Mautic\CoreBundle\Helper\InputHelper;
88
use Mautic\CoreBundle\Helper\PathsHelper;
9+
use Symfony\Component\HttpFoundation\File\File;
10+
use Symfony\Component\HttpFoundation\File\UploadedFile;
911
use Symfony\Component\HttpFoundation\JsonResponse;
1012
use Symfony\Component\HttpFoundation\Request;
1113
use Symfony\Component\HttpFoundation\Response;
@@ -16,16 +18,6 @@ class FileController extends AjaxController
1618

1719
public const EDITOR_CKEDITOR = 'ckeditor';
1820

19-
protected $imageMimes = [
20-
'image/gif',
21-
'image/jpeg',
22-
'image/pjpeg',
23-
'image/jpeg',
24-
'image/pjpeg',
25-
'image/png',
26-
'image/x-png',
27-
];
28-
2921
protected $response = [];
3022

3123
protected $statusCode = Response::HTTP_OK;
@@ -41,10 +33,12 @@ public function uploadAction(Request $request, PathsHelper $pathsHelper, FileUpl
4133
$mediaDir = $this->getMediaAbsolutePath($pathsHelper);
4234
if (!isset($this->response['error'])) {
4335
foreach ($request->files as $file) {
44-
if (in_array($file->getMimeType(), $this->imageMimes)) {
36+
/** @var UploadedFile $file */
37+
try {
38+
$fileUploader->validateImage($file);
4539
$fileName = $fileUploader->upload($mediaDir, $file);
4640
$this->successfulResponse($request, $fileName, $editor);
47-
} else {
41+
} catch (FileUploadException) {
4842
$this->failureResponse($editor);
4943
}
5044
}
@@ -56,20 +50,25 @@ public function uploadAction(Request $request, PathsHelper $pathsHelper, FileUpl
5650
/**
5751
* List the files in /media directory.
5852
*/
59-
public function listAction(Request $request, PathsHelper $pathsHelper): JsonResponse
53+
public function listAction(Request $request, PathsHelper $pathsHelper, FileUploader $fileUploader): JsonResponse
6054
{
6155
$fnames = scandir($this->getMediaAbsolutePath($pathsHelper));
6256

6357
if ($fnames) {
6458
foreach ($fnames as $name) {
6559
$imagePath = $this->getMediaAbsolutePath($pathsHelper).'/'.$name;
6660
$imageUrl = $this->getMediaUrl($request).'/'.$name;
67-
if (!is_dir($name) && in_array(mime_content_type($imagePath), $this->imageMimes)) {
68-
$this->response[] = [
69-
'url' => $imageUrl,
70-
'thumb' => $imageUrl,
71-
'name' => $name,
72-
];
61+
$imageFile = new File($imagePath, checkPath: false);
62+
if (!is_dir($name)) {
63+
try {
64+
$fileUploader->validateImage($imageFile);
65+
$this->response[] = [
66+
'url' => $imageUrl,
67+
'thumb' => $imageUrl,
68+
'name' => $name,
69+
];
70+
} catch (FileUploadException) {
71+
}
7372
}
7473
}
7574
} else {

‎app/bundles/CoreBundle/Helper/FileUploader.php‎

Lines changed: 80 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -4,13 +4,37 @@
44

55
use Mautic\CoreBundle\Exception\FilePathException;
66
use Mautic\CoreBundle\Exception\FileUploadException;
7+
use Mautic\CoreBundle\Translation\Translator;
78
use Symfony\Component\HttpFoundation\File\Exception\FileException;
9+
use Symfony\Component\HttpFoundation\File\File;
810
use Symfony\Component\HttpFoundation\File\UploadedFile;
911

1012
class FileUploader
1113
{
14+
/** @var string[] */
15+
protected array $imageMimes = [
16+
'image/gif',
17+
'image/jpeg',
18+
'image/pjpeg',
19+
'image/png',
20+
'image/x-png',
21+
'image/webp',
22+
'image/svg+xml',
23+
];
24+
25+
/** @var string[] */
26+
protected array $imageExtensions = [
27+
'jpg',
28+
'jpeg',
29+
'png',
30+
'gif',
31+
'webp',
32+
'svg',
33+
];
34+
1235
public function __construct(
13-
private FilePathResolver $filePathResolver
36+
private FilePathResolver $filePathResolver,
37+
private Translator $translator,
1438
) {
1539
}
1640

@@ -32,18 +56,72 @@ public function upload($uploadDir, UploadedFile $file)
3256

3357
return $fileName;
3458
} catch (FileException) {
35-
throw new FileUploadException('Could not upload file');
59+
throw new FileUploadException($this->translator->trans('mautic.core.fileuploader.upload_error'));
3660
}
3761
} catch (FilePathException $e) {
3862
throw new FileUploadException($e->getMessage());
3963
}
4064
}
4165

66+
/**
67+
* Verify that the file is an image.
68+
*
69+
* @throws FileUploadException
70+
*/
71+
public function validateImage(File $file): void
72+
{
73+
// Check if the file is an image
74+
if (!in_array($file->getMimeType(), $this->getAllowedImageMimeTypes())) {
75+
throw new FileUploadException($this->translator->trans('mautic.core.fileuploader.unsupported_image', ['%types%' => implode(', ', $this->getAllowedImageExtensions())]));
76+
}
77+
// Also check the file extension
78+
$extension = strtolower(pathinfo($file instanceof UploadedFile ? $file->getClientOriginalName() : $file->getFilename(), PATHINFO_EXTENSION));
79+
if (!in_array($extension, $this->getAllowedImageExtensions())) {
80+
throw new FileUploadException($this->translator->trans('mautic.core.fileuploader.unsupported_image', ['%types%' => implode(', ', $this->getAllowedImageExtensions())]));
81+
}
82+
}
83+
4284
/**
4385
* @param string $path
4486
*/
4587
public function delete($path): void
4688
{
4789
$this->filePathResolver->delete($path);
4890
}
91+
92+
/**
93+
* @return string[]
94+
*/
95+
public function getAllowedImageMimeTypes(): array
96+
{
97+
return $this->imageMimes;
98+
}
99+
100+
/**
101+
* @return string[]
102+
*/
103+
public function getAllowedImageExtensions(): array
104+
{
105+
return $this->imageExtensions;
106+
}
107+
108+
/**
109+
* Allow compiler passes to add additional image MIME types.
110+
*/
111+
public function addAllowedImageMimeType(string $mimeType): self
112+
{
113+
$this->imageMimes[] = $mimeType;
114+
115+
return $this;
116+
}
117+
118+
/**
119+
* Allow compiler passes to add additional image extensions.
120+
*/
121+
public function addAllowedImageExtension(string $extension): self
122+
{
123+
$this->imageExtensions[] = $extension;
124+
125+
return $this;
126+
}
49127
}

‎app/bundles/CoreBundle/Tests/Unit/Helper/FileUploaderTest.php‎

Lines changed: 24 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@
66
use Mautic\CoreBundle\Exception\FileUploadException;
77
use Mautic\CoreBundle\Helper\FilePathResolver;
88
use Mautic\CoreBundle\Helper\FileUploader;
9+
use Mautic\CoreBundle\Translation\Translator;
910
use Symfony\Component\HttpFoundation\File\Exception\FileException;
1011
use Symfony\Component\HttpFoundation\File\UploadedFile;
1112

@@ -25,6 +26,10 @@ public function testSuccessfulUpload(): void
2526
->disableOriginalConstructor()
2627
->getMock();
2728

29+
$translatorMock = $this->getMockBuilder(Translator::class)
30+
->disableOriginalConstructor()
31+
->getMock();
32+
2833
$fileMock = $this->getMockBuilder(UploadedFile::class)
2934
->disableOriginalConstructor()
3035
->getMock();
@@ -42,7 +47,7 @@ public function testSuccessfulUpload(): void
4247
->method('createDirectory')
4348
->with($uploadDir);
4449

45-
$fileUploader = new FileUploader($filePathResolverMock);
50+
$fileUploader = new FileUploader($filePathResolverMock, $translatorMock);
4651

4752
$fileUploader->upload($uploadDir, $fileMock);
4853
}
@@ -61,6 +66,10 @@ public function testCouldNotCreateDirectory(): void
6166
->disableOriginalConstructor()
6267
->getMock();
6368

69+
$translatorMock = $this->getMockBuilder(Translator::class)
70+
->disableOriginalConstructor()
71+
->getMock();
72+
6473
$fileMock = $this->getMockBuilder(UploadedFile::class)
6574
->disableOriginalConstructor()
6675
->getMock();
@@ -78,7 +87,7 @@ public function testCouldNotCreateDirectory(): void
7887
->with($uploadDir)
7988
->willThrowException(new FilePathException('Could not create directory'));
8089

81-
$fileUploader = new FileUploader($filePathResolverMock);
90+
$fileUploader = new FileUploader($filePathResolverMock, $translatorMock);
8291

8392
$this->expectException(FileUploadException::class);
8493
$this->expectExceptionMessage('Could not create directory');
@@ -100,6 +109,13 @@ public function testCouldNotMoveFile(): void
100109
->disableOriginalConstructor()
101110
->getMock();
102111

112+
$translatorMock = $this->getMockBuilder(Translator::class)
113+
->disableOriginalConstructor()
114+
->getMock();
115+
116+
$translatorMock->method('trans')
117+
->willReturn('Could not upload filed');
118+
103119
$fileMock = $this->getMockBuilder(UploadedFile::class)
104120
->disableOriginalConstructor()
105121
->getMock();
@@ -118,7 +134,7 @@ public function testCouldNotMoveFile(): void
118134
->method('createDirectory')
119135
->with($uploadDir);
120136

121-
$fileUploader = new FileUploader($filePathResolverMock);
137+
$fileUploader = new FileUploader($filePathResolverMock, $translatorMock);
122138

123139
$this->expectException(FileUploadException::class);
124140
$this->expectExceptionMessage('Could not upload file');
@@ -139,11 +155,15 @@ public function testDeleteFile(): void
139155
->disableOriginalConstructor()
140156
->getMock();
141157

158+
$translatorMock = $this->getMockBuilder(Translator::class)
159+
->disableOriginalConstructor()
160+
->getMock();
161+
142162
$filePathResolverMock->expects($this->once())
143163
->method('delete')
144164
->with($file);
145165

146-
$fileUploader = new FileUploader($filePathResolverMock);
166+
$fileUploader = new FileUploader($filePathResolverMock, $translatorMock);
147167

148168
$fileUploader->delete($file);
149169
}

‎app/bundles/CoreBundle/Translations/en_US/messages.ini‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -801,3 +801,6 @@ mautic.placeholder_tokens.introducing_tokens.title="Introducing tokens"
801801
mautic.placeholder_tokens.introducing_tokens.description="Tokens are placeholders used within emails, landing pages, and other communications, which automatically get replaced with personalized information (like a contact's name, email, or custom field data) when the message is sent."
802802
mautic.placeholder_tokens.fallback_text_info="The default value is specified after the | character. For example, if a contact doesn't have a First name while using this token <code>Hi {contactfield=firstname|there}</code>, the email will send the phrase as <code>Hi there</code>."
803803
mautic.core.theme.form.confirm.unhide="Show the theme, %theme%?"
804+
805+
mautic.core.fileuploader.upload_error="Could not upload file"
806+
mautic.core.fileuploader.unsupported_image="Unsupported image type or corrupt file. Allowed types are: %types%."

‎plugins/GrapesJsBuilderBundle/Assets/library/js/builder.service.js‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -88,6 +88,10 @@ export default class BuilderService {
8888
});
8989
});
9090

91+
this.editor.on('asset:upload:error', (error) => {
92+
Mautic.setFlashes(Mautic.addErrorFlashMessage(error));
93+
});
94+
9195
this.editor.on('asset:open', () => {
9296
const editor = this.editor;
9397
const assetsService = this.assetService;

‎plugins/GrapesJsBuilderBundle/Assets/library/js/dist/builder.js‎

Lines changed: 1 addition & 1 deletion
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎plugins/GrapesJsBuilderBundle/Controller/FileManagerController.php‎

Lines changed: 10 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -5,18 +5,26 @@
55
namespace MauticPlugin\GrapesJsBuilderBundle\Controller;
66

77
use Mautic\CoreBundle\Controller\AjaxController;
8+
use Mautic\CoreBundle\Exception\FileUploadException;
89
use MauticPlugin\GrapesJsBuilderBundle\Helper\FileManager;
910
use Symfony\Component\HttpFoundation\JsonResponse;
1011
use Symfony\Component\HttpFoundation\Request;
12+
use Symfony\Component\HttpFoundation\Response;
1113

1214
class FileManagerController extends AjaxController
1315
{
1416
private const DEFAULT_PAGE = 1;
1517
private const DEFAULT_LIMIT = 20;
1618

17-
public function uploadAction(Request $request, FileManager $fileManager): JsonResponse
19+
public function uploadAction(Request $request, FileManager $fileManager): Response
1820
{
19-
return $this->sendJsonResponse(['data'=> $fileManager->uploadFiles($request)]);
21+
try {
22+
$response = $this->sendJsonResponse(['data'=> $fileManager->uploadFiles($request)]);
23+
} catch (FileUploadException $error) {
24+
return new Response($error->getMessage(), Response::HTTP_BAD_REQUEST);
25+
}
26+
27+
return $response;
2028
}
2129

2230
public function deleteAction(Request $request, FileManager $fileManager): JsonResponse

‎plugins/GrapesJsBuilderBundle/Helper/FileManager.php‎

Lines changed: 8 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -20,12 +20,14 @@ class FileManager
2020
public function __construct(
2121
private FileUploader $fileUploader,
2222
private CoreParametersHelper $coreParametersHelper,
23-
private PathsHelper $pathsHelper
23+
private PathsHelper $pathsHelper,
2424
) {
2525
}
2626

2727
/**
2828
* @return array
29+
*
30+
* @throws FileUploadException
2931
*/
3032
public function uploadFiles($request)
3133
{
@@ -35,10 +37,11 @@ public function uploadFiles($request)
3537
$uploadedFiles = [];
3638

3739
foreach ($files as $file) {
38-
try {
39-
$uploadedFiles[] = $this->getFullUrl($this->fileUploader->upload($uploadDir, $file));
40-
} catch (FileUploadException) {
41-
}
40+
$this->fileUploader->validateImage($file);
41+
}
42+
43+
foreach ($files as $file) {
44+
$uploadedFiles[] = $this->getFullUrl($this->fileUploader->upload($uploadDir, $file));
4245
}
4346
}
4447

0 commit comments

Comments
 (0)