-
Notifications
You must be signed in to change notification settings - Fork 255
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
add base64 upload #495
base: master
Are you sure you want to change the base?
add base64 upload #495
Changes from 10 commits
c910542
bf8499d
6c83c5c
a0573c0
af00b22
7777cb5
0919441
4fafa7a
8a6e439
889215f
5468717
8ceb373
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,31 @@ | ||
<?php | ||
namespace Josegonzalez\Upload\File\Path; | ||
|
||
use Cake\Utility\Hash; | ||
use Cake\Utility\Text; | ||
|
||
class Base64Processor extends DefaultProcessor | ||
{ | ||
/** | ||
* Returns the filename for the current field/data combination. | ||
* If a `nameCallback` is specified in settings, then that callable | ||
* will be invoked with the current upload data. | ||
* | ||
* @return string | ||
*/ | ||
public function filename() | ||
{ | ||
$processor = Hash::get($this->settings, 'nameCallback', null); | ||
$extension = Hash::get($this->settings, 'base64_extension', '.png'); | ||
if (is_callable($processor)) { | ||
$numberOfParameters = (new \ReflectionFunction($processor))->getNumberOfParameters(); | ||
if ($numberOfParameters == 2) { | ||
return $processor($this->data, $this->settings); | ||
} | ||
|
||
return $processor($this->table, $this->entity, $this->data, $this->field, $this->settings); | ||
} | ||
|
||
return Text::uuid() . "$extension"; | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,59 @@ | ||
<?php | ||
namespace Josegonzalez\Upload\File\Transformer; | ||
|
||
class Base64Transformer extends DefaultTransformer | ||
{ | ||
/** | ||
* Path where the file will be writen | ||
* | ||
* @var string | ||
*/ | ||
private $path; | ||
|
||
/** | ||
* Creates a set of files from the initial data and returns them as key/value | ||
* pairs, where the path on disk maps to name which each file should have. | ||
* Example: | ||
* | ||
* [ | ||
* '/tmp/path/to/file/on/disk' => 'file.pdf', | ||
* '/tmp/path/to/file/on/disk-2' => 'file-preview.png', | ||
* ] | ||
* | ||
* @return array key/value pairs of temp files mapping to their names | ||
*/ | ||
public function transform() | ||
{ | ||
$decoded = base64_decode($this->data['data']); | ||
file_put_contents($this->getPath(), $decoded); | ||
|
||
return [ | ||
$this->getPath() => $this->data['name'], | ||
]; | ||
} | ||
|
||
/** | ||
* Sets the path for the file to be written | ||
* | ||
* @param string $path Path to write the file | ||
* @return void | ||
*/ | ||
public function setPath($path) | ||
{ | ||
$this->path = $path; | ||
} | ||
|
||
/** | ||
* Returns the path where the file will be written | ||
* | ||
* @return string|empty | ||
*/ | ||
public function getPath() | ||
{ | ||
if (empty($this->path)) { | ||
return $this->path = tempnam(sys_get_temp_dir(), 'upload'); | ||
} | ||
|
||
return $this->path; | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -87,7 +87,8 @@ public function beforeSave(Event $event, Entity $entity, ArrayObject $options) | |
continue; | ||
} | ||
|
||
if (Hash::get((array)$entity->get($field), 'error') !== UPLOAD_ERR_OK) { | ||
$uploadValidator = $this->getUploadValidator($entity, $settings, $field); | ||
if ($uploadValidator->hasUploadFailed()) { | ||
if (Hash::get($settings, 'restoreValueOnFailure', true)) { | ||
$entity->set($field, $entity->getOriginal($field)); | ||
$entity->setDirty($field, false); | ||
|
@@ -99,7 +100,14 @@ public function beforeSave(Event $event, Entity $entity, ArrayObject $options) | |
$path = $this->getPathProcessor($entity, $data, $field, $settings); | ||
$basepath = $path->basepath(); | ||
$filename = $path->filename(); | ||
$data['name'] = $filename; | ||
if (is_string($data)) { | ||
$temp = []; | ||
$temp['name'] = $filename; | ||
$temp['data'] = $data; | ||
$data = $temp; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. What is the purpose of this? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. When upload base64 $data is just a string f.ex There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Also because of the https://github.com/FriendsOfCake/cakephp-upload/pull/495/files#diff-84c66ea1e7013949dabde1157612da57R122 There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Can you show how this would be used in practice? It's not clear whats going on here. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. You mean with the 2 extra interfaces? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Something like this.Maybe with some better names 😄 There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Maybe you could come with something better?I tried to think of a design pattern to apply, but I could only think of template or strategy but is pretty much like what we already have with the interfaces in place There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I meant in terms of an end user. How are you planning on using this/why should this go into the core? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'm using https://foliotek.github.io/Croppie/ so that the user can select his profile picture and the result is in base64. |
||
} else { | ||
$data['name'] = $filename; | ||
} | ||
$files = $this->constructFiles($entity, $data, $field, $settings, $basepath); | ||
|
||
$writer = $this->getWriter($entity, $data, $field, $settings); | ||
|
@@ -111,8 +119,10 @@ public function beforeSave(Event $event, Entity $entity, ArrayObject $options) | |
|
||
$entity->set($field, $filename); | ||
$entity->set(Hash::get($settings, 'fields.dir', 'dir'), $basepath); | ||
$entity->set(Hash::get($settings, 'fields.size', 'size'), $data['size']); | ||
$entity->set(Hash::get($settings, 'fields.type', 'type'), $data['type']); | ||
if (!isset($temp)) { | ||
$entity->set(Hash::get($settings, 'fields.size', 'size'), $data['size']); | ||
$entity->set(Hash::get($settings, 'fields.type', 'type'), $data['type']); | ||
} | ||
} | ||
} | ||
|
||
|
@@ -205,6 +215,27 @@ public function getWriter(Entity $entity, $data, $field, $settings) | |
)); | ||
} | ||
|
||
/** | ||
* Retrieves an instance of a validator that validates that the current upload has succeded | ||
* | ||
* @param \Cake\ORM\Entity $entity an entity | ||
* @param array $data the data being submitted for a save | ||
* @return \Josegonzalez\Upload\UploadValidator\UploadValidatorInterface | ||
*/ | ||
public function getUploadValidator(Entity $entity, $settings, $field) | ||
{ | ||
$default = 'Josegonzalez\Upload\UploadValidator\DefaultUploadValidator'; | ||
$uploadValidatorClass = Hash::get($settings, 'uploadValidator', $default); | ||
if (is_subclass_of($uploadValidatorClass, 'Josegonzalez\Upload\UploadValidator\UploadValidatorInterface')) { | ||
return new $uploadValidatorClass($entity, $field); | ||
} | ||
|
||
throw new UnexpectedValueException(sprintf( | ||
"'uploadValidator' not set to instance of UploadValidatorInterface: %s", | ||
$uploadValidatorClass | ||
)); | ||
} | ||
|
||
/** | ||
* Creates a set of files from the initial data and returns them as key/value | ||
* pairs, where the path on disk maps to name which each file should have. | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,19 @@ | ||
<?php | ||
namespace Josegonzalez\Upload\UploadValidator; | ||
|
||
use Josegonzalez\Upload\UploadValidator\DefaultUploadValidator; | ||
|
||
class Base64UploadValidator extends DefaultUploadValidator | ||
{ | ||
|
||
/** | ||
* Check's data for any upload errors. | ||
* pairs, where the path on disk maps to name which each file should have. | ||
* | ||
* @return bool `true` if upload failed | ||
*/ | ||
public function hasUploadFailed() | ||
{ | ||
return !base64_decode($this->entity->get($this->field), true); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. How can one do other validations like for e.g checking for particular mimetype / file type? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Ah I now see the comments regarding adding methods/interfaces for checking size and mime type. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I don't think is possible to do check on mimetype / file type without having the file already written on the disk There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Wouldn't it be possible having validation methods using the example code provided in this comment #495 (comment) ? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I just tried and it works. I wasn't aware of such method to determine the mimetype. |
||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,46 @@ | ||
<?php | ||
namespace Josegonzalez\Upload\UploadValidator; | ||
|
||
use Cake\ORM\Entity; | ||
use Cake\Utility\Hash; | ||
use Josegonzalez\Upload\UploadValidator\UploadValidatorInterface; | ||
|
||
class DefaultUploadValidator implements UploadValidatorInterface | ||
{ | ||
/** | ||
* Entity instance. | ||
* | ||
* @var \Cake\ORM\Entity | ||
*/ | ||
protected $entity; | ||
|
||
/** | ||
* Name of field | ||
* | ||
* @var string | ||
*/ | ||
protected $field; | ||
|
||
/** | ||
* Constructor | ||
* | ||
* @param \Cake\ORM\Entity $entity the entity to construct a path for. | ||
* @param string $field the field for which data will be saved | ||
*/ | ||
public function __construct(Entity $entity, $field) | ||
{ | ||
$this->entity = $entity; | ||
$this->field = $field; | ||
} | ||
|
||
/** | ||
* Check's data for any upload errors. | ||
* pairs, where the path on disk maps to name which each file should have. | ||
* | ||
* @return bool `true` if upload failed | ||
*/ | ||
public function hasUploadFailed() | ||
{ | ||
return Hash::get((array)$this->entity->get($this->field), 'error') !== UPLOAD_ERR_OK; | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,23 @@ | ||
<?php | ||
namespace Josegonzalez\Upload\UploadValidator; | ||
|
||
use Cake\ORM\Entity; | ||
|
||
interface UploadValidatorInterface | ||
{ | ||
/** | ||
* Constructor. | ||
* | ||
* @param \Cake\ORM\Entity $entity the entity to construct a path for. | ||
* @param string $field the field for which data will be saved | ||
*/ | ||
public function __construct(Entity $entity, $field); | ||
|
||
/** | ||
* Check's data for any upload errors. | ||
* pairs, where the path on disk maps to name which each file should have. | ||
* | ||
* @return bool `true` if upload failed | ||
*/ | ||
public function hasUploadFailed(); | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,45 @@ | ||
<?php | ||
namespace Josegonzalez\Upload\Test\TestCase\File\Path; | ||
|
||
use Cake\TestSuite\TestCase; | ||
use Josegonzalez\Upload\File\Path\Base64Processor; | ||
|
||
class Base64ProcessorTest extends TestCase | ||
{ | ||
public function testIsProcessorInterface() | ||
{ | ||
$entity = $this->getMockBuilder('Cake\ORM\Entity')->getMock(); | ||
$table = $this->getMockBuilder('Cake\ORM\Table')->getMock(); | ||
$data = ['name' => 'filename']; | ||
$field = 'field'; | ||
$settings = []; | ||
$processor = new Base64Processor($table, $entity, $data, $field, $settings); | ||
$this->assertInstanceOf('Josegonzalez\Upload\File\Path\ProcessorInterface', $processor); | ||
} | ||
|
||
public function testRandomFileNameDefaultExtension() | ||
{ | ||
$entity = $this->getMockBuilder('Cake\ORM\Entity')->getMock(); | ||
$table = $this->getMockBuilder('Cake\ORM\Table')->getMock(); | ||
$data = ['name' => 'filename']; | ||
$field = 'field'; | ||
$settings = []; | ||
$processor = new Base64Processor($table, $entity, $data, $field, $settings); | ||
$fileName = $processor->filename(); | ||
$found = strpos($fileName, '.png'); | ||
$this->assertNotFalse($found); | ||
} | ||
|
||
public function testRandomFileNameCustomExtension() | ||
{ | ||
$entity = $this->getMockBuilder('Cake\ORM\Entity')->getMock(); | ||
$table = $this->getMockBuilder('Cake\ORM\Table')->getMock(); | ||
$data = ['name' => 'filename']; | ||
$field = 'field'; | ||
$settings = ['base64_extension' => '.cake']; | ||
$processor = new Base64Processor($table, $entity, $data, $field, $settings); | ||
$fileName = $processor->filename(); | ||
$found = strpos($fileName, '.cake'); | ||
$this->assertNotFalse($found); | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,48 @@ | ||
<?php | ||
namespace Josegonzalez\Upload\Test\TestCase\File\Transformer; | ||
|
||
use Cake\ORM\Entity; | ||
use Cake\ORM\Table; | ||
use Cake\TestSuite\TestCase; | ||
use Josegonzalez\Upload\File\Transformer\Base64Transformer; | ||
use VirtualFileSystem\FileSystem as Vfs; | ||
|
||
class Base64TransformerTest extends TestCase | ||
{ | ||
public function setup() | ||
{ | ||
$this->entity = $this->getMockBuilder('Cake\ORM\Entity')->getMock(); | ||
$this->table = $this->getMockBuilder('Cake\ORM\Table')->getMock(); | ||
$this->data = ['data' => 'Y2FrZXBocA==', 'name' => '5a2e69ff-c2c0-44c1-94a7-d791202f0067.txt']; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We should keep in mind that using a custom array format like this makes validation tricker. Most validation methods for uploaded files won't be usable. I haven't reviewed the full PR but you need to ensure proper validation is done for this array format and someone can't sneak in whatever they want using this feature. |
||
$this->field = 'field'; | ||
$this->settings = []; | ||
$this->transformer = new Base64Transformer( | ||
$this->table, | ||
$this->entity, | ||
$this->data, | ||
$this->field, | ||
$this->settings | ||
); | ||
|
||
$this->vfs = new Vfs; | ||
mkdir($this->vfs->path('/tmp')); | ||
file_put_contents($this->vfs->path('/tmp/tempfile'), $this->data['data']); | ||
} | ||
|
||
public function teardown() | ||
{ | ||
unset($this->transformer); | ||
} | ||
|
||
public function testTransform() | ||
{ | ||
$this->transformer->setPath($this->vfs->path('/tmp/tempfile')); | ||
$expected = [$this->vfs->path('/tmp/tempfile') => '5a2e69ff-c2c0-44c1-94a7-d791202f0067.txt']; | ||
$this->assertEquals($expected, $this->transformer->transform()); | ||
} | ||
|
||
public function testIsTransformerInterface() | ||
{ | ||
$this->assertInstanceOf('Josegonzalez\Upload\File\Transformer\TransformerInterface', $this->transformer); | ||
} | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice