ZF-8693: BUG! Incorrect use switch(true)
Description
Bug in Zend/File/Transfer/Adapter/Abstract.php
method addValidators;
$validators = array(
array('MimeType', true, array('image/jpeg', 'image/gif', 'image/png')),
array('FilesSize', true, array('max' => '1MB', 'messages' => 'файл больше 1MБ')),
);
$this->_upload->addValidators($validators, 'img');
set validators for all files, because error in block
switch (true) {
case (0 == $argc):
break;
case (1 <= $argc):
$validator = array_shift($validatorInfo);
case (2 <= $argc):
$breakChainOnFailure = array_shift($validatorInfo);
case (3 <= $argc):
$options = array_shift($validatorInfo);
case (4 <= $argc):
$files = array_shift($validatorInfo);
default:
$this->addValidator($validator, $breakChainOnFailure, $options, $files);
break;
Must be changed to:
if ($argc > 0) {
$validator = array_shift($validatorInfo);
if (2 <= $argc)
$breakChainOnFailure = array_shift($validatorInfo);
if (3 <= $argc)
$options = array_shift($validatorInfo);
if (4 <= $argc)
$files = array_shift($validatorInfo);
$this->addValidator($validator, $breakChainOnFailure, $options, $files);
}
Invalid use switch See on oficial site php.net
<?php switch ($i) { case 0: echo "i equals 0"; case 1: echo "i equals 1"; case 2: echo "i equals 2"; } ?>Here, if $i is equal to 0, PHP would execute all of the echo statements! If $i is equal to 1, PHP would execute the last two echo statements. You would get the expected behavior ('i equals 2' would be displayed) only if $i is equal to 2. Thus, it is important not to forget break statements (even though you may want to avoid supplying them on purpose under certain circumstances).
Comments
Posted by Sergey (lifinsky) on 2010-01-03T03:14:25.000+0000
Code is not correct. Test again, because this code always run case 3 and 4 - set files to null always.
$validators = array( array('MimeType', true, array('image/jpeg', 'image/gif', 'image/png')), array('FilesSize', true, array('max' => '1MB', 'messages' => 'файл больше 1MБ')), );
$this->_upload->addValidators($validators, 'img');
Test this!!! Case 1 - true always, and not has break - run cases 2-4. This is error!!!
Posted by Matthew Weier O'Phinney (matthew) on 2010-01-03T12:20:25.000+0000
We don't need to remove the switch statement; we need to merely change the conditions. switch(true) is a perfectly valid use case.
Please provide a reproduce case and expected and actual results, please.
Posted by Sergey Boroday (simpliest) on 2010-01-03T23:41:05.000+0000
I think code below should work correctly
same code as diff
Posted by Thomas Weidner (thomas) on 2010-01-04T01:47:44.000+0000
Sergey: Again... please provide a reproducable use.
The above code works in about 5 components. When it has to be changed it would mean that 5 components do not work properly. Therefor please add a usecase which shows where you have problems in your application.
Just changing the core without knowing why is not a good practice.
Posted by Sergey Boroday (simpliest) on 2010-01-04T02:31:59.000+0000
Thomas Weidner: Sorry, but reporter of issue Sergey and I am (Sergey Boroday) are two different "Sergey". :)
Author of issue http://framework.zend.com/issues/secure/…
But you right. Shame me. Proposed code didn't fix anything and will not work correctly.
Posted by Sergey (lifinsky) on 2010-01-04T04:47:07.000+0000
If I use validators in this format
$validators = array( array('MimeType', true, array('image/jpeg', 'image/gif', 'image/png')), array('FilesSize', true, array('max' => '1MB', 'messages' => 'файл больше 1MБ')), );
I set 3 parameters ($argc)
4 parameter - null, I add validator to file key, for example 'img'
$upload->addValidators($validators, 'img');
But bug in case set validators to all files!!!! What do you do not understand????
case (4 <= $argc): $files = array_shift($validatorInfo); ------ null!!!
Case must be changed to if, or inverse case order from max $argc = 4 to 1
I change code to
Posted by Sergey Boroday (simpliest) on 2010-01-04T06:57:43.000+0000
успокойся. Им нужен детальный кейс для воспроизведения ситуации. А еще лучше тест.
I tried to write test but here some questions
Posted by Thomas Weidner (thomas) on 2010-01-11T13:18:44.000+0000
Fixed with r20206.
Some notes: * The provided code snippets where not used (they added other problems) * The provided unittests are partitially wrong * Actually it is not possible to set multiple instances of the same class... one MimeType instance overwrites the other... this is also mentioned within the manual (will be fixed by another issue)
Posted by Sergey (lifinsky) on 2010-01-13T06:06:01.000+0000
I want use format 2
equivalent to format 1
Because format 1 has always $breakChainOnFailure = false!!!
Posted by Thomas Weidner (thomas) on 2010-01-13T06:53:54.000+0000
It would be a pleasure if you also read what you get responded by people:
{quote} •Actually it is not possible to set multiple instances of the same class... one MimeType instance overwrites the other... this is also mentioned within the manual (will be fixed by another issue) {quote}
Posted by Thomas Weidner (thomas) on 2010-01-13T07:20:55.000+0000
Additionally the syntax of your format2 works and is tested within our unittests.
Posted by Sergey (lifinsky) on 2010-01-13T12:25:32.000+0000
HOW IT WORK?????
Your code always set $files to null!!!!
switch (true) { case (0 == $argc): break; case (1 <= $argc): $validator = array_shift($validatorInfo); case (2 <= $argc): $breakChainOnFailure = array_shift($validatorInfo); case (3 <= $argc): $options = array_shift($validatorInfo); case (4 <= $argc): $files = array_shift($validatorInfo); default: $this->addValidator($validator, $breakChainOnFailure, $options, $files); break;
ALWAYS case 4! always $files = null in my example
Posted by Sergey Boroday (simpliest) on 2010-01-13T13:44:33.000+0000
Sergey. Please read comments carefully. Issue fixed. Get the new version from trunk.