From ca971cd5107ef2eec141ea510187bb80c721b752 Mon Sep 17 00:00:00 2001 From: smarcet Date: Tue, 22 Oct 2019 17:35:15 -0300 Subject: [PATCH] File Uploading Fixed transliterator for invalid file names Change-Id: I850c462989ca331b02640ca45c52227a7e2cbe85 --- app/Http/Utils/FileUploader.php | 60 +++++++++- .../Main/Repositories/IFolderRepository.php | 6 + .../Main/DoctrineFolderRepository.php | 9 ++ composer.json | 1 + composer.lock | 108 ++++++++++++------ tests/OAuth2SummitApiTest.php | 45 +++++++- 6 files changed, 191 insertions(+), 38 deletions(-) diff --git a/app/Http/Utils/FileUploader.php b/app/Http/Utils/FileUploader.php index 028446c8..e516df23 100644 --- a/app/Http/Utils/FileUploader.php +++ b/app/Http/Utils/FileUploader.php @@ -15,7 +15,11 @@ use App\Services\Model\IFolderService; use Illuminate\Http\UploadedFile; use Illuminate\Support\Facades\Log; use Illuminate\Support\Facades\Storage; +use models\exceptions\ValidationException; use models\main\File; +use Behat\Transliterator\Transliterator; +use models\main\IFolderRepository; + /** * Class FileUploader * @package App\Http\Utils @@ -32,16 +36,38 @@ final class FileUploader implements IFileUploader */ private $bucket; + /** + * @var IFolderRepository + */ + private $folder_repository; + /** * FileUploader constructor. * @param IFolderService $folder_service + * @param IFolderRepository $folder_repository * @param IBucket $bucket */ - public function __construct(IFolderService $folder_service, IBucket $bucket){ - $this->folder_service = $folder_service; - $this->bucket = $bucket; + public function __construct + ( + IFolderService $folder_service, + IFolderRepository $folder_repository, + IBucket $bucket + ) + { + $this->folder_service = $folder_service; + $this->folder_repository = $folder_repository; + $this->bucket = $bucket; } + + private static $default_replacements = [ + '/\s/' => '-', // remove whitespace + '/_/' => '-', // underscores to dashes + '/[^A-Za-z0-9+.\-]+/' => '', // remove non-ASCII chars, only allow alphanumeric plus dash and dot + '/[\-]{2,}/' => '-', // remove duplicate dashes + '/^[\.\-_]+/' => '', // Remove all leading dots, dashes or underscores + ]; + /** * @param UploadedFile $file * @param $folder_name @@ -54,6 +80,27 @@ final class FileUploader implements IFileUploader try { $client_original_name = $file->getClientOriginalName(); + $client_original_name = Transliterator::utf8ToAscii($client_original_name); + + foreach(self::$default_replacements as $regex => $replace) { + $client_original_name = preg_replace($regex, $replace, $client_original_name); + } + + $idx = 1; + $ext = pathinfo($client_original_name, PATHINFO_EXTENSION); + $name = pathinfo($client_original_name, PATHINFO_FILENAME); + + while($this->folder_repository->existByName($client_original_name)){ + $client_original_name = $name.$idx.'.'.$ext; + ++$idx; + } + + $title = pathinfo($client_original_name, PATHINFO_FILENAME); + + if(empty($title)){ + throw new ValidationException("empty file name is not valid"); + } + Log::debug(sprintf("FileUploader::build: folder_name %s client original name %s", $folder_name, $client_original_name)); $local_path = Storage::putFileAs(sprintf('/public/%s', $folder_name), $file, $client_original_name); @@ -67,8 +114,8 @@ final class FileUploader implements IFileUploader $attachment->setName($client_original_name); $file_name = sprintf("assets/%s/%s", $folder_name, $client_original_name); Log::debug(sprintf("FileUploader::build file_name %s", $file_name)); - $title = str_replace(array('-', '_'), ' ', preg_replace('/\.[^.]+$/', '', $file->getClientOriginalName())); $attachment->setFilename($file_name); + $title = str_replace(['-','_'],' ', preg_replace('/\.[^.]+$/', '', $title)); Log::debug(sprintf("FileUploader::build title %s", $title)); $attachment->setTitle($title); $attachment->setShowInSearch(true); @@ -79,7 +126,10 @@ final class FileUploader implements IFileUploader $attachment->setCloudMeta('LastPut', time()); $attachment->setCloudStatus('Live'); $attachment->setCloudSize(filesize($local_path)); - + } + catch(ValidationException $ex){ + Log::warning($ex); + throw $ex; } catch (\Exception $ex){ Log::error($ex); diff --git a/app/Models/Foundation/Main/Repositories/IFolderRepository.php b/app/Models/Foundation/Main/Repositories/IFolderRepository.php index 0c726fba..957ed957 100644 --- a/app/Models/Foundation/Main/Repositories/IFolderRepository.php +++ b/app/Models/Foundation/Main/Repositories/IFolderRepository.php @@ -24,6 +24,12 @@ interface IFolderRepository extends IBaseRepository */ public function getFolderByName($folder_name); + /** + * @param string $name + * @return bool + */ + public function existByName(string $name):bool; + /** * @param string $file_name * @return File diff --git a/app/Repositories/Main/DoctrineFolderRepository.php b/app/Repositories/Main/DoctrineFolderRepository.php index b77a17ca..f2fb43bc 100644 --- a/app/Repositories/Main/DoctrineFolderRepository.php +++ b/app/Repositories/Main/DoctrineFolderRepository.php @@ -95,4 +95,13 @@ SQL; return $native_query->getOneOrNullResult(); } + + /** + * @param string $name + * @return bool + */ + public function existByName(string $name): bool + { + return $this->count(['name'=> trim($name)]) > 0; + } } \ No newline at end of file diff --git a/composer.json b/composer.json index 5d13a255..dc0adaeb 100644 --- a/composer.json +++ b/composer.json @@ -17,6 +17,7 @@ "php": "^7.1.3", "ext-json": "*", "ext-pdo": "*", + "behat/transliterator": "^1.2", "cocur/slugify": "^2.3", "ezyang/htmlpurifier": "4.7.0", "fideloper/proxy": "^4.0", diff --git a/composer.lock b/composer.lock index 9792384d..af4a06ba 100644 --- a/composer.lock +++ b/composer.lock @@ -4,8 +4,52 @@ "Read more about it at https://getcomposer.org/doc/01-basic-usage.md#composer-lock-the-lock-file", "This file is @generated automatically" ], - "content-hash": "e8960c590d5b3ce6a88c1a2544a3751f", + "content-hash": "36e3d6f7cdd125988acca84a07b3bcf9", "packages": [ + { + "name": "behat/transliterator", + "version": "v1.2.0", + "source": { + "type": "git", + "url": "https://github.com/Behat/Transliterator.git", + "reference": "826ce7e9c2a6664c0d1f381cbb38b1fb80a7ee2c" + }, + "dist": { + "type": "zip", + "url": "https://api.github.com/repos/Behat/Transliterator/zipball/826ce7e9c2a6664c0d1f381cbb38b1fb80a7ee2c", + "reference": "826ce7e9c2a6664c0d1f381cbb38b1fb80a7ee2c", + "shasum": "" + }, + "require": { + "php": ">=5.3.3" + }, + "require-dev": { + "chuyskywalker/rolling-curl": "^3.1", + "php-yaoi/php-yaoi": "^1.0" + }, + "type": "library", + "extra": { + "branch-alias": { + "dev-master": "1.2-dev" + } + }, + "autoload": { + "psr-0": { + "Behat\\Transliterator": "src/" + } + }, + "notification-url": "https://packagist.org/downloads/", + "license": [ + "Artistic-1.0" + ], + "description": "String transliterator", + "keywords": [ + "i18n", + "slug", + "transliterator" + ], + "time": "2017-04-04T11:38:05+00:00" + }, { "name": "cocur/slugify", "version": "v2.5", @@ -1190,13 +1234,13 @@ "authors": [ { "name": "Maciej Łebkowski", - "email": "m.lebkowski@gmail.com", - "role": "Contributor" + "role": "Contributor", + "email": "m.lebkowski@gmail.com" }, { "name": "Markus Poerschke", - "email": "markus@eluceo.de", - "role": "Developer" + "role": "Developer", + "email": "markus@eluceo.de" } ], "description": "The eluceo/iCal package offers a abstraction layer for creating iCalendars. You can easily create iCal files by using PHP object instead of typing your *.ics file by hand. The output will follow RFC 2445 as best as possible.", @@ -1800,8 +1844,8 @@ "authors": [ { "name": "Bartosz Pachołek", - "email": "bartosz@idct.pl", - "role": "lead" + "role": "lead", + "email": "bartosz@idct.pl" } ], "description": "Library that provides wrapper methods around SSH2 and SFTP to simplify file download/upload over SSH/SCP/SFTP.", @@ -2506,14 +2550,14 @@ "authors": [ { "name": "Alex Bilbie", + "role": "Developer", "email": "hello@alexbilbie.com", - "homepage": "http://www.alexbilbie.com", - "role": "Developer" + "homepage": "http://www.alexbilbie.com" }, { "name": "Woody Gilk", - "homepage": "https://github.com/shadowhand", - "role": "Contributor" + "role": "Contributor", + "homepage": "https://github.com/shadowhand" } ], "description": "OAuth 2.0 Client Library", @@ -3618,9 +3662,9 @@ "authors": [ { "name": "Evert Pot", + "role": "Developer", "email": "me@evertpot.com", - "homepage": "http://evertpot.com/", - "role": "Developer" + "homepage": "http://evertpot.com/" } ], "description": "Functions for making sense out of URIs.", @@ -3675,14 +3719,14 @@ "authors": [ { "name": "Evert Pot", + "role": "Developer", "email": "me@evertpot.com", - "homepage": "http://evertpot.com/", - "role": "Developer" + "homepage": "http://evertpot.com/" }, { "name": "Markus Staab", - "email": "markus.staab@redaxo.de", - "role": "Developer" + "role": "Developer", + "email": "markus.staab@redaxo.de" } ], "description": "sabre/xml is an XML library that you may not hate.", @@ -4460,7 +4504,7 @@ }, { "name": "Gert de Pagter", - "email": "backendtea@gmail.com" + "email": "BackEndTea@gmail.com" } ], "description": "Symfony polyfill for ctype functions", @@ -5160,8 +5204,8 @@ "authors": [ { "name": "Tijs Verkoyen", - "email": "css_to_inline_styles@verkoyen.eu", - "role": "Developer" + "role": "Developer", + "email": "css_to_inline_styles@verkoyen.eu" } ], "description": "CssToInlineStyles is a class that enables you to convert HTML-pages/files into HTML-pages/files with inline styles. This is very useful when you're sending emails.", @@ -5753,18 +5797,18 @@ "authors": [ { "name": "Arne Blankerts", - "email": "arne@blankerts.de", - "role": "Developer" + "role": "Developer", + "email": "arne@blankerts.de" }, { "name": "Sebastian Heuer", - "email": "sebastian@phpeople.de", - "role": "Developer" + "role": "Developer", + "email": "sebastian@phpeople.de" }, { "name": "Sebastian Bergmann", - "email": "sebastian@phpunit.de", - "role": "Developer" + "role": "Developer", + "email": "sebastian@phpunit.de" } ], "description": "Component for reading phar.io manifest information from a PHP Archive (PHAR)", @@ -6133,8 +6177,8 @@ "authors": [ { "name": "Sebastian Bergmann", - "email": "sebastian@phpunit.de", - "role": "lead" + "role": "lead", + "email": "sebastian@phpunit.de" } ], "description": "FilterIterator implementation that filters files based on a list of suffixes.", @@ -6175,8 +6219,8 @@ "authors": [ { "name": "Sebastian Bergmann", - "email": "sebastian@phpunit.de", - "role": "lead" + "role": "lead", + "email": "sebastian@phpunit.de" } ], "description": "Simple template engine.", @@ -6926,8 +6970,8 @@ "authors": [ { "name": "Sebastian Bergmann", - "email": "sebastian@phpunit.de", - "role": "lead" + "role": "lead", + "email": "sebastian@phpunit.de" } ], "description": "Library that helps with managing the version number of Git-hosted PHP projects", diff --git a/tests/OAuth2SummitApiTest.php b/tests/OAuth2SummitApiTest.php index 61b3f7bc..56c18c40 100644 --- a/tests/OAuth2SummitApiTest.php +++ b/tests/OAuth2SummitApiTest.php @@ -37,7 +37,6 @@ final class OAuth2SummitApiTest extends ProtectedApiTest $app->instance(\App\Http\Utils\IFileUploader::class, $fileUploaderMock); - return $app; } @@ -1451,4 +1450,48 @@ final class OAuth2SummitApiTest extends ProtectedApiTest return intval($video_id); } + + public function testAddPresentationSlideInvalidName($summit_id=25){ + + $repo = EntityManager::getRepository(\models\summit\Summit::class); + $summit = $repo->getById($summit_id); + $presentation = $summit->getPublishedPresentations()[0]; + $params = array + ( + 'id' => $summit_id, + 'presentation_id' => $presentation->getId(), + ); + + $headers = array + ( + "HTTP_Authorization" => " Bearer " . $this->access_token, + "CONTENT_TYPE" => "application/json" + ); + + $video_data = array + ( + 'name' => 'test slide', + 'description' => 'test slide', + 'display_on_site' => true, + ); + + $response = $this->action + ( + "POST", + "OAuth2PresentationApiController@addPresentationSlide", + $params, + array(), + array(), + [ + 'file' => UploadedFile::fake()->image('invalid image.jpeg') + ], + $headers, + json_encode($video_data) + ); + + $video_id = $response->getContent(); + $this->assertResponseStatus(201); + return intval($video_id); + } + } \ No newline at end of file