Skip to content
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

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

Open
weierophinney opened this issue Dec 31, 2019 · 5 comments

Comments

@weierophinney
Copy link
Member

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 :(


Originally posted by @svycka at zendframework/zend-filter#4

@weierophinney
Copy link
Member Author

@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:

  • if provided string is a directory (so must exist) then the file will be moved into that directory,
  • otherwise, path does not exist, so script assumes that it should be a file

Originally posted by @michalbundyra at zendframework/zend-filter#4 (comment)

@weierophinney
Copy link
Member Author

@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.


Originally posted by @svycka at zendframework/zend-filter#4 (comment)

@weierophinney
Copy link
Member Author

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


Originally posted by @michalbundyra at zendframework/zend-filter#4 (comment)

@weierophinney
Copy link
Member Author

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


Originally posted by @svycka at zendframework/zend-filter#4 (comment)

@weierophinney
Copy link
Member Author

@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.


Originally posted by @michalbundyra at zendframework/zend-filter#4 (comment)

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

No branches or pull requests

1 participant