Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Already on GitHub? Sign in to your account

[2.1][File Uploads] Multi-File input filtering and FilePRG plugin update #3010

Merged
merged 4 commits into from Dec 10, 2012

Conversation

Projects
None yet
4 participants
Contributor

cgmartin commented Nov 19, 2012

  1. Solution for filtering and validating File elements with the HTML5 multiple attribute setting.

Example InputFilter Specification for a File form element:

array(
    'type'       => 'Zend\InputFilter\FileInput',
    'name'       => 'my-file-input',
    'filters'    => array('filelowercase'),
    'validators' => array('fileupload'),
);

If the multiple attribute is set, the validators and filters will be run against each individual value.

Also added a autoPrependUploadValidator flag to FileInput, which will automatically add a Upload validator. This is similar to how Input will automatically add a NotEmpty validator. Makes it easier for users and gives more secure defaults.

  1. Enhancement to the File Rename filter: Added a new option randomize which appends a unique id to the end of the target filename. This is useful as a filter for multiple file uploads, to temporarily store them in a directory with a unique name.

  2. Enhancement to the FilePostRedirectGet Controller Plugin: When using the File Rename filter with your File inputs, the FilePRG plugin is now smart enough to save off the uploaded files and restore the form state.

A working example can be found here: https://github.com/cgmartin/ZF2FileUploadExamples

Owner

weierophinney commented Nov 19, 2012

IMO, this looks very clean and straightforward; I like it.

Can you add a usage example to the PR descriptions as a record, so others looking at this understand how you'd use it?

@ghost ghost assigned weierophinney Nov 19, 2012

Contributor

cgmartin commented Nov 20, 2012

@weierophinney I edited the description above with some examples to hopefully paint a clearer picture. :) Thanks for the feedback

Owner

weierophinney commented Nov 20, 2012

If you could manage to make the validators work the same as the filters, would we need the "explode" functionality? Also, is that functionality on master already, or only in develop? If it's in master, we'd need to make sure that you could do either an explode validator or the new, simpler syntax.

Looks much simpler under the proposal -- would love to see this!

Contributor

cgmartin commented Nov 20, 2012

If you could manage to make the validators work the same as the filters, would we need the "explode" functionality?

Right, we would not need the explode functionality. This is not in master - it's strictly part of the 2.1 File Upload stuff, inside Zend\InputFilter\FileInput, only part of develop branch.

Sounds good. :) I'll work on this validator change and add it to this PR. Thanks again!

Owner

weierophinney commented Nov 20, 2012

@cgmartin BEST possible situation, then -- and thanks in advance!

Contributor

cgmartin commented Nov 21, 2012

@weierophinney I've made those validator changes. This should be ready for final review and merge, if all good.

@weierophinney weierophinney merged commit 164714b into zendframework:develop Dec 10, 2012

1 check passed

default The Travis build passed
Details

merge of _POST and _FILES leads to vulnerability. Because I can post data structure as in _FILE array and set tmp_name to file path I want to.
Also this type of merge do not support file uploads when file is under additional keywords for example:

$postOther = Array(
    [business] => Array(
        [name] => BigCompany
    )
);
$postFiles = Array(
    [business] => Array(
        [logo] => Array(
            [name] => 400.png
            [type] => image/png
            [tmp_name] => /tmp/php5Wx0aJ
            [error] => 0
            [size] => 15726
        )
    )
);

after array_merge we have same as in $postFiles and loosing 'name' value

Contributor

cgmartin replied Jan 7, 2013

That recursive merge issue is definitely a problem, thanks for catching that. I'll make a fix and add some unit tests.

Not sure I understand about the vulnerability you mention, since you can spoof $_FILES information in the request regardless. The Zend\Validator\File\Upload helps to validate file information by doing a is_uploaded_file. This validator gets added by default when adding file inputs to a form.

Sorry, didn't saw Upload Validator contents.

Member

Maks3w replied Jan 8, 2013

Please send any security observation to zf-security@zend.com

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment