search

zendframework / zend-filter

Filter component from Zend Framework
BSD 3-Clause "New" or "Revised" License
68 stars 35 forks source link

Zend\Filter\File\RenameUpload behaves differently when folder exists and not #4

Open svycka opened 9 years ago

svycka commented 9 years ago

code example:

$filter = new \Zend\Filter\File\RenameUpload([
    'target' => './public/img/logotypes/logo',
    'use_upload_extension' => true,
    'randomize' => true,
]);
$file = $filter->filter($file_data);
echo $file['tmp_name'];

when logo folder exists returned value would be:

./public/img/logotypes/logo/phpj7GsVi_55f03311ed828.jpg

when logo folder doesn't exists returned value would be:

./public/img/logotypes/logo_55f034e642bf3.jpg

I can't beleave this is desired behavior because this can lead to bugs. Guess I used this incorrectly because I wanted logo prefix this worked for me until someone created logo folder then everything gone bad :(

michalbundyra commented 5 years ago

@svycka I've checked it and I believe it is desired behaviour. As target we can provide the directory or the file name and in script we can't recognise what you have on mind. So:

svycka commented 5 years ago

@webimpress I do not agree with you

if provided string is a directory (so must exist) then the file will be moved into that directory,

sounds ok.

otherwise, path does not exist, so script assumes that it should be a file

this is not documented so I am not sure how you decided this is the desired behavior. Don't understand how having two hidden behaviors from one option is good. maybe new file_prefix or something would be better.

michalbundyra commented 5 years ago

@svycka Ok, reopening. Feel free to submit PR - either: documentation update of propose a fix.

svycka commented 5 years ago

fixing would mean BC break? if this is the desired behavior

michalbundyra commented 5 years ago

@svycka If you propose fix which will be BC Break we will plan it for v3.

I've described what is the current behaviour, and, yes, I can see it can be a bit tricky, but I think it is logical. There is not anything like 'file prefix' right now, the provided target is either directory "where file should be moved" or the new file name. As you are using randomize=true there is randomized string added to the name.

weierophinney commented 4 years ago

This repository has been closed and moved to laminas/laminas-filter; a new issue has been opened at https://github.com/laminas/laminas-filter/issues/13.