[WIP] Zend\Filter harmonization (Issue 5119) #5436

Closed
wants to merge 16 commits into
from

6 participants

@ThaDafinser

Filters have currently different behaviours, like:

  • return on wrong input value
  • trigger errors
  • throw exceptions

See issue here: #5119 (comment)

this PR should harmoize to one behaviour.

Todo list

  • Zend\Filter\* - except the list bellow
    • Zend\Filter\Callback - not possible to check? as a user defined function is awaited (could be everything)
    • Zend\Filter\Compress - commit missing + travis check
    • Zend\Filter\DateTimeFormatter - commit missing + travis check
    • Zend\Filter\Decompress - commit missing + travis check
    • Zend\Filter\Decrypt - commit missing + travis check
    • Zend\Filter\Encrypt - commit missing + travis check
    • Zend\Filter\Inflector - not possible to check?
  • Zend\Filter\File\*
  • Zend\Filter\Word\*
  • Zend\I18n\Filter\* - commit missing + travis check
@bakura10

I think most of those changes are BC. I've done already a lot of work to clean filters for ZF3 (#5097), maybe you could directly do PR to my branch so that those changes are also included in the refactor branch? What do you think?

@ThaDafinser

@bakura10 most of the changes are definetly no BC breaks.

The only difference is, that there is now always a check of the filtered values, before filtering and no error is triggered.
If a Filter has been used currently right, there will be no BC break.

For me both ways are okay. ZF2 or ZF3....someone else shall decide 😄

@texdc

Speaking of the behavior of filters, keep this in mind.

@bakura10

That's an interesting thing @texdc . In fact, hydrator filters are... well... filters. They don't hydrate/extract unwanting filter. In fact, what you would want would be to rename Filter component to Sanitizer component? I'm not against this idea, however I'm wondering if it would be a good thing to do. People are accustomed to using the Zend\Filter component since... many many many years. Changing this now would indeed confuse a lot of people. Again, the tradeoff between correctness, usage and simplicity is not an easy task.

@samsonasik samsonasik commented on an outdated diff Nov 11, 2013
library/Zend/Filter/Decrypt.php
@@ -24,6 +24,10 @@ class Decrypt extends Encrypt
*/
public function filter($value)
{
+ if(!is_string($value)){
@samsonasik
samsonasik added a line comment Nov 11, 2013

add space after if before open parenthesis and after close parenthesis before open bracket.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@samsonasik samsonasik commented on an outdated diff Nov 11, 2013
library/Zend/Filter/Encrypt.php
@@ -117,6 +120,10 @@ public function __call($method, $options)
*/
public function filter($value)
{
+ if(!is_string($value)){
@samsonasik
samsonasik added a line comment Nov 11, 2013

add space after if before open parenthesis and after close parenthesis before open bracket.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@samsonasik samsonasik commented on an outdated diff Nov 11, 2013
library/Zend/Filter/File/Decrypt.php
// An uploaded file? Retrieve the 'tmp_name'
- $isFileUpload = (is_array($value) && isset($value['tmp_name']));
- if ($isFileUpload) {
+ $isFileUpload = false;
+ if(is_array($value)){
@samsonasik
samsonasik added a line comment Nov 11, 2013

add space after if before open parenthesis and after close parenthesis before open bracket.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@samsonasik samsonasik commented on an outdated diff Nov 11, 2013
library/Zend/Filter/File/Decrypt.php
// An uploaded file? Retrieve the 'tmp_name'
- $isFileUpload = (is_array($value) && isset($value['tmp_name']));
- if ($isFileUpload) {
+ $isFileUpload = false;
+ if(is_array($value)){
+ if(!isset($value['tmp_name'])){
@samsonasik
samsonasik added a line comment Nov 11, 2013

add space after if before open parenthesis and after close parenthesis before open bracket.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@samsonasik samsonasik commented on an outdated diff Nov 11, 2013
library/Zend/Filter/File/Encrypt.php
// An uploaded file? Retrieve the 'tmp_name'
- $isFileUpload = (is_array($value) && isset($value['tmp_name']));
- if ($isFileUpload) {
+ $isFileUpload = false;
+ if(is_array($value)){
@samsonasik
samsonasik added a line comment Nov 11, 2013

add space after if before open parenthesis and after close parenthesis before open bracket.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@samsonasik samsonasik commented on an outdated diff Nov 11, 2013
library/Zend/Filter/File/Encrypt.php
// An uploaded file? Retrieve the 'tmp_name'
- $isFileUpload = (is_array($value) && isset($value['tmp_name']));
- if ($isFileUpload) {
+ $isFileUpload = false;
+ if(is_array($value)){
+ if(!isset($value['tmp_name'])){
@samsonasik
samsonasik added a line comment Nov 11, 2013

add space after if before open parenthesis and after close parenthesis before open bracket.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@samsonasik samsonasik commented on an outdated diff Nov 11, 2013
library/Zend/Filter/File/LowerCase.php
// An uploaded file? Retrieve the 'tmp_name'
- $isFileUpload = (is_array($value) && isset($value['tmp_name']));
- if ($isFileUpload) {
+ $isFileUpload = false;
+ if(is_array($value)){
@samsonasik
samsonasik added a line comment Nov 11, 2013

add space after if before open parenthesis and after close parenthesis before open bracket.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@samsonasik samsonasik commented on an outdated diff Nov 11, 2013
library/Zend/Filter/File/LowerCase.php
// An uploaded file? Retrieve the 'tmp_name'
- $isFileUpload = (is_array($value) && isset($value['tmp_name']));
- if ($isFileUpload) {
+ $isFileUpload = false;
+ if(is_array($value)){
+ if(!isset($value['tmp_name'])){
@samsonasik
samsonasik added a line comment Nov 11, 2013

add space after if before open parenthesis and after close parenthesis before open bracket.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@samsonasik samsonasik commented on an outdated diff Nov 11, 2013
library/Zend/Filter/File/Rename.php
// An uploaded file? Retrieve the 'tmp_name'
- $isFileUpload = (is_array($value) && isset($value['tmp_name']));
- if ($isFileUpload) {
+ $isFileUpload = false;
+ if(is_array($value)){
+ if(!isset($value['tmp_name'])){
@samsonasik
samsonasik added a line comment Nov 11, 2013

and here..

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@samsonasik samsonasik commented on an outdated diff Nov 11, 2013
library/Zend/Filter/File/RenameUpload.php
// An uploaded file? Retrieve the 'tmp_name'
- $isFileUpload = (is_array($value) && isset($value['tmp_name']));
- if ($isFileUpload) {
+ $isFileUpload = false;
+ if(is_array($value)){
+ if(!isset($value['tmp_name'])){
@samsonasik
samsonasik added a line comment Nov 11, 2013

and here..

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@samsonasik samsonasik commented on an outdated diff Nov 11, 2013
library/Zend/Filter/File/UpperCase.php
// An uploaded file? Retrieve the 'tmp_name'
- $isFileUpload = (is_array($value) && isset($value['tmp_name']));
- if ($isFileUpload) {
+ $isFileUpload = false;
+ if(is_array($value)){
+ if(!isset($value['tmp_name'])){
@samsonasik
samsonasik added a line comment Nov 11, 2013

and here..

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@samsonasik samsonasik commented on an outdated diff Nov 11, 2013
library/Zend/Filter/PregReplace.php
@@ -129,6 +129,10 @@ public function getReplacement()
*/
public function filter($value)
{
+ if(!is_scalar($value) && !is_array($value)){
@samsonasik
samsonasik added a line comment Nov 11, 2013

and here..

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@samsonasik samsonasik commented on an outdated diff Nov 11, 2013
library/Zend/Filter/StripNewlines.php
*/
public function filter($value)
{
+ if(!is_scalar($value) && !is_array($value)){
@samsonasik
samsonasik added a line comment Nov 11, 2013

and here..

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@samsonasik samsonasik commented on an outdated diff Nov 11, 2013
library/Zend/I18n/Filter/Alnum.php
*/
public function filter($value)
{
+ if(!is_scalar($value) && !is_array($value)){
@samsonasik
samsonasik added a line comment Nov 11, 2013

and here..

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@samsonasik samsonasik commented on an outdated diff Nov 11, 2013
library/Zend/I18n/Filter/Alpha.php
*/
public function filter($value)
{
+ if(!is_scalar($value) && !is_array($value)){
@samsonasik
samsonasik added a line comment Nov 11, 2013

and here..

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@samsonasik samsonasik commented on an outdated diff Nov 11, 2013
library/Zend/I18n/Filter/NumberFormat.php
@@ -140,6 +140,10 @@ public function getFormatter()
*/
public function filter($value)
{
+ if(!is_scalar($value)){
@samsonasik
samsonasik added a line comment Nov 11, 2013

and here..

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@weierophinney weierophinney commented on the diff Jan 3, 2014
library/Zend/Filter/BaseName.php
*
* @param string $value
* @return string|mixed
*/
public function filter($value)
{
- if (null === $value) {
- return null;
- }
-
if (!is_scalar($value)) {
@weierophinney
Zend Framework member
weierophinney added a line comment Jan 3, 2014

I'd update this to:

if (!is_scalar($value) || null === $value) {

in order to retain the original behavior.

@ThaDafinser
ThaDafinser added a line comment Jan 9, 2014

It is the original behaviour. null is not a scalar.

So if null is entered null (the value) is returned -> so it's the same :-)

@weierophinney
Zend Framework member
weierophinney added a line comment Feb 4, 2014

Thanks for schooling me on this -- I'm forgetting my basic types! (Verified: is_scalar(null) returns false.)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@weierophinney weierophinney commented on an outdated diff Jan 3, 2014
library/Zend/Filter/DateTimeFormatter.php
@@ -73,7 +77,14 @@ protected function normalizeDateTime($value)
{
if ($value === '' || $value === null) {
return $value;
- } elseif (is_int($value)) {
+ }
+
+ if(!is_string($value) && !is_int($value) && !$value instanceof DateTime){
@weierophinney
Zend Framework member
weierophinney added a line comment Jan 3, 2014

add spaces around the conditional.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@weierophinney weierophinney commented on the diff Jan 3, 2014
library/Zend/Filter/Digits.php
*
* @param string $value
* @return string|mixed
*/
public function filter($value)
{
- if (null === $value) {
- return null;
- }
-
if (!is_scalar($value)) {
@weierophinney
Zend Framework member
weierophinney added a line comment Jan 3, 2014

Update this to:

if (!is_scalar($value) || null === $value) {

to preserve the original behavior.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@weierophinney weierophinney commented on the diff Jan 3, 2014
library/Zend/Filter/HtmlEntities.php
@@ -182,29 +181,18 @@ public function setDoubleQuote($doubleQuote)
*/
public function filter($value)
{
- if (null === $value) {
- return null;
- }
-
if (!is_scalar($value)) {
@weierophinney
Zend Framework member
weierophinney added a line comment Jan 3, 2014

Same comment as on previous classes -- incorporate the null check into this condition to retain previous behavior.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@weierophinney weierophinney commented on the diff Jan 3, 2014
library/Zend/Filter/Int.php
*
* @param string $value
* @return int|mixed
*/
public function filter($value)
{
- if (null === $value) {
- return null;
- }
-
if (!is_scalar($value)) {
@weierophinney
Zend Framework member
weierophinney added a line comment Jan 3, 2014

Same comment as on previous classes -- incorporate the null check into this condition to retain previous behavior.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@weierophinney weierophinney commented on the diff Jan 3, 2014
library/Zend/Filter/RealPath.php
*
* @param string $value
* @return string|mixed
*/
public function filter($value)
{
- if (null === $value) {
- return null;
- }
-
- if (!is_scalar($value)) {
- trigger_error(
- sprintf(
- '%s expects parameter to be scalar, "%s" given; cannot filter',
- __METHOD__,
- (is_object($value) ? get_class($value) : gettype($value))
- ),
- E_USER_WARNING
- );
+ if(!is_string($value)){
@weierophinney
Zend Framework member
weierophinney added a line comment Jan 3, 2014

Same comment as on previous classes -- incorporate the null check into this condition to retain previous behavior.

@weierophinney
Zend Framework member
weierophinney added a line comment Jan 3, 2014

Ignore -- null will be caught here anyways.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@weierophinney weierophinney commented on the diff Jan 3, 2014
library/Zend/Filter/StringToLower.php
*
* @param string $value
* @return string|mixed
*/
public function filter($value)
{
- if (null === $value) {
- return null;
- }
-
if (!is_scalar($value)) {
@weierophinney
Zend Framework member
weierophinney added a line comment Jan 3, 2014

Same comment as on previous classes -- incorporate the null check into this condition to retain previous behavior.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@weierophinney weierophinney commented on the diff Jan 3, 2014
library/Zend/Filter/StringToUpper.php
*
* @param string $value
* @return string|mixed
*/
public function filter($value)
{
- if (null === $value) {
- return null;
- }
-
if (!is_scalar($value)) {
@weierophinney
Zend Framework member
weierophinney added a line comment Jan 3, 2014

Same comment as on previous classes -- incorporate the null check into this condition to retain previous behavior.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@weierophinney weierophinney commented on the diff Jan 3, 2014
library/Zend/Filter/StripTags.php
@@ -175,22 +174,9 @@ public function setAttributesAllowed($attributesAllowed)
*/
public function filter($value)
{
- if (null === $value) {
- return null;
- }
-
if (!is_scalar($value)) {
@weierophinney
Zend Framework member
weierophinney added a line comment Jan 3, 2014

Same comment as on previous classes -- incorporate the null check into this condition to retain previous behavior.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@weierophinney weierophinney commented on an outdated diff Jan 3, 2014
tests/ZendTest/Filter/BaseNameTest.php
- /**
- * @return void
- */
- public function testReturnsNullIfNullIsUsed()
- {
- $filter = new BaseNameFilter();
- $filtered = $filter->filter(null);
- $this->assertNull($filtered);
+ $valuesExpected = array(
+ null,
+ new \stdClass(),
+ array(
+ '/path/to/filename',
+ '/path/to/filename.ext'
+ )
+ );
@weierophinney
Zend Framework member
weierophinney added a line comment Jan 3, 2014

Use a data provider instead of a foreach loop.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@weierophinney weierophinney commented on an outdated diff Jan 3, 2014
tests/ZendTest/Filter/CompressTest.php
+ /**
+ * @return void
+ */
+ public function testReturnUnfiltered()
+ {
+ $filter = new CompressFilter('bz2');
+
+ $valuesExpected = array(
+ null,
+ new \stdClass(),
+ array(
+ 'compress me',
+ 'compress me too, please'
+ )
+ );
+ foreach ($valuesExpected as $input) {
@weierophinney
Zend Framework member
weierophinney added a line comment Jan 3, 2014

Use a data provider instead of a foreach loop.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@weierophinney weierophinney commented on an outdated diff Jan 3, 2014
tests/ZendTest/Filter/DateTimeFormatterTest.php
- public function testFormatterDoesNotFormatNull()
- {
- $filter = new DateTimeFormatter();
- $result = $filter->filter(null);
- $this->assertEquals(null, $result);
+ $valuesExpected = array(
+ null,
+ '',
+ new \stdClass(),
+ array(
+ '1',
+ -1
+ ),
+ 0.53
+ );
+ foreach ($valuesExpected as $input) {
@weierophinney
Zend Framework member
weierophinney added a line comment Jan 3, 2014

Use a data provider instead of a foreach loop.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@weierophinney weierophinney commented on an outdated diff Jan 3, 2014
tests/ZendTest/Filter/DecompressTest.php
+ /**
+ * @return void
+ */
+ public function testReturnUnfiltered()
+ {
+ $filter = new DecompressFilter('bz2');
+
+ $valuesExpected = array(
+ null,
+ new \stdClass(),
+ array(
+ 'decompress me',
+ 'decompress me too, please'
+ )
+ );
+ foreach ($valuesExpected as $input) {
@weierophinney
Zend Framework member
weierophinney added a line comment Jan 3, 2014

Use a data provider instead of a foreach loop.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@weierophinney weierophinney commented on an outdated diff Jan 3, 2014
tests/ZendTest/Filter/DecryptTest.php
+ if (!extension_loaded('mcrypt')) {
+ $this->markTestSkipped('Mcrypt extension not installed');
+ }
+
+ $decrypt = new DecryptFilter(array('adapter' => 'BlockCipher', 'key' => 'testkey'));
+ $decrypt->setVector('1234567890123456890');
+
+ $valuesExpected = array(
+ null,
+ new \stdClass(),
+ array(
+ 'ec133eb7460682b0020b736ad6d2ef14c35de0f1e5976330ae1dd096ef3b4cb7MTIzNDU2Nzg5MDEyMzQ1NoZvxY1JkeL6TnQP3ug5F0k=',
+ 'decrypt me too, please'
+ )
+ );
+ foreach ($valuesExpected as $input) {
@weierophinney
Zend Framework member
weierophinney added a line comment Jan 3, 2014

Use a data provider instead of a foreach loop.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@weierophinney weierophinney commented on an outdated diff Jan 3, 2014
tests/ZendTest/Filter/DigitsTest.php
- * @return void
- */
- public function testReturnsNullIfNullIsUsed()
- {
- $filter = new DigitsFilter();
- $filtered = $filter->filter(null);
- $this->assertNull($filtered);
+ $valuesExpected = array(
+ null,
+ new \stdClass(),
+ array(
+ 'abc123',
+ 'abc 123'
+ )
+ );
+ foreach ($valuesExpected as $input) {
@weierophinney
Zend Framework member
weierophinney added a line comment Jan 3, 2014

Use a data provider instead of a foreach loop.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@weierophinney weierophinney commented on an outdated diff Jan 3, 2014
tests/ZendTest/Filter/DirTest.php
+ /**
+ * @return void
+ */
+ public function testReturnUnfiltered()
+ {
+ $filter = new DirFilter();
+
+ $valuesExpected = array(
+ null,
+ new \stdClass(),
+ array(
+ '/path/to/filename',
+ '/path/to/filename.ext'
+ )
+ );
+ foreach ($valuesExpected as $input) {
@weierophinney
Zend Framework member
weierophinney added a line comment Jan 3, 2014

Use a data provider instead of a foreach loop.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@weierophinney weierophinney commented on an outdated diff Jan 3, 2014
tests/ZendTest/Filter/EncryptTest.php
+ if (!extension_loaded('mcrypt')) {
+ $this->markTestSkipped('Mcrypt extension not installed');
+ }
+
+ $encrypt = new EncryptFilter(array('adapter' => 'BlockCipher', 'key' => 'testkey'));
+ $encrypt->setVector('1234567890123456890');
+
+ $valuesExpected = array(
+ null,
+ new \stdClass(),
+ array(
+ 'encrypt me',
+ 'encrypt me too, please'
+ )
+ );
+ foreach ($valuesExpected as $input) {
@weierophinney
Zend Framework member
weierophinney added a line comment Jan 3, 2014

Use a data provider instead of a foreach loop.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@weierophinney weierophinney commented on an outdated diff Jan 3, 2014
tests/ZendTest/Filter/File/DecryptTest.php
+ * @return void
+ */
+ public function testReturnUnfiltered()
+ {
+ $filter = new FileDecrypt();
+ $filter->setKey('1234567890123456');
+
+ $valuesExpected = array(
+ null,
+ new \stdClass(),
+ array(
+ dirname(__DIR__).'/_files/nofile.txt',
+ dirname(__DIR__).'/_files/nofile2.txt'
+ )
+ );
+ foreach ($valuesExpected as $input) {
@weierophinney
Zend Framework member
weierophinney added a line comment Jan 3, 2014

Use a data provider instead of a foreach loop.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@weierophinney weierophinney commented on an outdated diff Jan 3, 2014
tests/ZendTest/Filter/File/EncryptTest.php
+ * @return void
+ */
+ public function testReturnUnfiltered()
+ {
+ $filter = new FileEncrypt();
+ $filter->setKey('1234567890123456');
+
+ $valuesExpected = array(
+ null,
+ new \stdClass(),
+ array(
+ dirname(__DIR__) . '/_files/nofile.txt',
+ dirname(__DIR__) . '/_files/nofile2.txt'
+ )
+ );
+ foreach ($valuesExpected as $input) {
@weierophinney
Zend Framework member
weierophinney added a line comment Jan 3, 2014

Use a data provider instead of a foreach loop.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@weierophinney weierophinney commented on an outdated diff Jan 3, 2014
tests/ZendTest/Filter/File/LowerCaseTest.php
+ /**
+ * @return void
+ */
+ public function testReturnUnfiltered()
+ {
+ $filter = new FileLowerCase();
+
+ $valuesExpected = array(
+ null,
+ new \stdClass(),
+ array(
+ dirname(__DIR__).'/_files/nofile.txt',
+ dirname(__DIR__).'/_files/nofile2.txt'
+ )
+ );
+ foreach ($valuesExpected as $input) {
@weierophinney
Zend Framework member
weierophinney added a line comment Jan 3, 2014

Use a data provider instead of a foreach loop.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@weierophinney weierophinney commented on an outdated diff Jan 3, 2014
tests/ZendTest/Filter/File/RenameTest.php
+ /**
+ * @return void
+ */
+ public function testReturnUnfiltered()
+ {
+ $filter = new FileRename($this->_newFile);
+
+ $valuesExpected = array(
+ null,
+ new \stdClass(),
+ array(
+ $this->_oldFile,
+ $this->_origFile
+ )
+ );
+ foreach ($valuesExpected as $input) {
@weierophinney
Zend Framework member
weierophinney added a line comment Jan 3, 2014

Use a data provider instead of a foreach loop.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@weierophinney weierophinney commented on an outdated diff Jan 3, 2014
tests/ZendTest/Filter/File/RenameUploadTest.php
+ public function testReturnUnfiltered()
+ {
+ $filter = new RenameUploadMock(array(
+ 'target' => $this->_newFile,
+ 'randomize' => true,
+ ));
+
+ $valuesExpected = array(
+ null,
+ new \stdClass(),
+ array(
+ $this->_oldFile,
+ 'something invalid'
+ )
+ );
+ foreach ($valuesExpected as $input) {
@weierophinney
Zend Framework member
weierophinney added a line comment Jan 3, 2014

Use a data provider instead of a foreach loop.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@weierophinney weierophinney commented on an outdated diff Jan 3, 2014
tests/ZendTest/Filter/File/UpperCaseTest.php
+ * @return void
+ */
+ public function testReturnUnfiltered()
+ {
+ $filter = new FileUpperCase();
+ $filter->setEncoding('ISO-8859-1');
+
+ $valuesExpected = array(
+ null,
+ new \stdClass(),
+ array(
+ $this->_newFile,
+ 'something invalid'
+ )
+ );
+ foreach ($valuesExpected as $input) {
@weierophinney
Zend Framework member
weierophinney added a line comment Jan 3, 2014

Use a data provider instead of a foreach loop.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@weierophinney weierophinney commented on an outdated diff Jan 3, 2014
tests/ZendTest/Filter/HtmlEntitiesTest.php
{
- $filtered = $this->_filter->filter(null);
- $this->assertNull($filtered);
+ $valuesExpected = array(
+ null,
+ new \stdClass(),
+ array(
+ '<',
+ '>'
+ )
+ );
+ foreach ($valuesExpected as $input) {
@weierophinney
Zend Framework member
weierophinney added a line comment Jan 3, 2014

Use a data provider instead of a foreach loop.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@weierophinney weierophinney commented on an outdated diff Jan 3, 2014
tests/ZendTest/Filter/IntTest.php
- * @return void
- */
- public function testReturnsNullIfNullIsUsed()
- {
- $filter = new IntFilter();
- $filtered = $filter->filter(null);
- $this->assertNull($filtered);
+ $valuesExpected = array(
+ null,
+ new \stdClass(),
+ array(
+ '1',
+ -1
+ )
+ );
+ foreach ($valuesExpected as $input) {
@weierophinney
Zend Framework member
weierophinney added a line comment Jan 3, 2014

Use a data provider instead of a foreach loop.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@weierophinney weierophinney commented on an outdated diff Jan 3, 2014
tests/ZendTest/Filter/PregReplaceTest.php
@@ -100,4 +118,22 @@ public function testPassingPatternWithExecModifierRaisesException()
$this->setExpectedException('Zend\Filter\Exception\InvalidArgumentException', '"e" pattern modifier');
$filter->setPattern('/foo/e');
}
+
+ /**
+ *
+ * @return void
+ */
+ public function testReturnUnfiltered()
+ {
+ $filter = $this->filter;
+ $filter->setPattern('#^controller/(?P<action>[a-z_-]+)#')->setReplacement('foo/bar');
+
+ $valuesExpected = array(
+ null,
+ new \stdClass()
+ );
+ foreach ($valuesExpected as $input) {
@weierophinney
Zend Framework member
weierophinney added a line comment Jan 3, 2014

Use a data provider instead of a foreach loop.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@weierophinney weierophinney commented on an outdated diff Jan 3, 2014
tests/ZendTest/Filter/RealPathTest.php
- $filtered = $filter->filter($input);
- $err = ErrorHandler::stop();
-
- $this->assertEquals($input, $filtered);
- $this->assertInstanceOf('ErrorException', $err);
- $this->assertContains('cannot filter', $err->getMessage());
- }
-
- /**
- * @return void
- */
- public function testReturnsNullIfNullIsUsed()
- {
- $filtered = $this->_filter->filter(null);
- $this->assertNull($filtered);
+ foreach ($valuesExpected as $input) {
@weierophinney
Zend Framework member
weierophinney added a line comment Jan 3, 2014

Use a data provider instead of a foreach loop.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@weierophinney weierophinney commented on an outdated diff Jan 3, 2014
tests/ZendTest/Filter/StringToLowerTest.php
{
- $filtered = $this->_filter->filter(null);
- $this->assertNull($filtered);
+ $valuesExpected = array(
+ null,
+ new \stdClass(),
+ array(
+ 'UPPER CASE WRITTEN',
+ 'This should stay the same'
+ )
+ );
+ foreach ($valuesExpected as $input) {
@weierophinney
Zend Framework member
weierophinney added a line comment Jan 3, 2014

Use a data provider instead of a foreach loop.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@weierophinney weierophinney commented on an outdated diff Jan 3, 2014
tests/ZendTest/Filter/StringToUpperTest.php
{
- $filtered = $this->_filter->filter(null);
- $this->assertNull($filtered);
+ $valuesExpected = array(
+ null,
+ new \stdClass(),
+ array(
+ 'lower case written',
+ 'This should stay the same'
+ )
+ );
+ foreach ($valuesExpected as $input) {
@weierophinney
Zend Framework member
weierophinney added a line comment Jan 3, 2014

Use a data provider instead of a foreach loop.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@weierophinney weierophinney commented on an outdated diff Jan 3, 2014
tests/ZendTest/Filter/StripNewlinesTest.php
+ $this->assertEquals(array_values($expected), $filter(array_keys($expected)));
+ }
+
+ /**
+ *
+ * @return void
+ */
+ public function testReturnUnfiltered()
+ {
+ $filter = new StripNewLinesFilter();
+
+ $valuesExpected = array(
+ null,
+ new \stdClass()
+ );
+ foreach ($valuesExpected as $input) {
@weierophinney
Zend Framework member
weierophinney added a line comment Jan 3, 2014

Use a data provider instead of a foreach loop.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@weierophinney weierophinney commented on an outdated diff Jan 3, 2014
tests/ZendTest/Filter/StripTagsTest.php
- /**
- * @return void
- */
- public function testReturnsNullIfNullIsUsed()
- {
- $filtered = $this->_filter->filter(null);
- $this->assertNull($filtered);
+ $valuesExpected = array(
+ null,
+ new \stdClass(),
+ array(
+ '<li data-name="Test User" data-id="11223"></li>',
+ '<li data-name="Test User 2" data-id="456789"></li>'
+ )
+ );
+ foreach ($valuesExpected as $input) {
@weierophinney
Zend Framework member
weierophinney added a line comment Jan 3, 2014

Use a data provider instead of a foreach loop.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@weierophinney weierophinney commented on an outdated diff Jan 3, 2014
tests/ZendTest/Filter/UriNormalizeTest.php
+ /**
+ * @return void
+ */
+ public function testReturnUnfiltered()
+ {
+ $filter = new UriNormalize();
+
+ $valuesExpected = array(
+ null,
+ new \stdClass(),
+ array(
+ 'http://www.example.com',
+ 'file:///home/shahar/secret/../../otherguy/secret'
+ )
+ );
+ foreach ($valuesExpected as $input) {
@weierophinney
Zend Framework member
weierophinney added a line comment Jan 3, 2014

Use a data provider instead of a foreach loop.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@weierophinney weierophinney commented on an outdated diff Jan 3, 2014
tests/ZendTest/Filter/Word/CamelCaseToSeparatorTest.php
+ $this->assertNotEquals($input, $filtered);
+ $this->assertEquals(array('Camel Cased Words', 'something Different'), $filtered);
+ }
+
+ /**
+ * @return void
+ */
+ public function testReturnUnfiltered()
+ {
+ $filter = new CamelCaseToSeparatorFilter();
+
+ $valuesExpected = array(
+ null,
+ new \stdClass()
+ );
+ foreach ($valuesExpected as $input) {
@weierophinney
Zend Framework member
weierophinney added a line comment Jan 3, 2014

Use a data provider instead of a foreach loop.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@weierophinney weierophinney commented on an outdated diff Jan 3, 2014
tests/ZendTest/Filter/Word/DashToSeparatorTest.php
+ $this->assertEquals(array('dash separated words', 'something different'), $filtered);
+ }
+
+
+ /**
+ * @return void
+ */
+ public function testReturnUnfiltered()
+ {
+ $filter = new DashToSeparatorFilter();
+
+ $valuesExpected = array(
+ null,
+ new \stdClass()
+ );
+ foreach ($valuesExpected as $input) {
@weierophinney
Zend Framework member
weierophinney added a line comment Jan 3, 2014

Use a data provider instead of a foreach loop.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@weierophinney weierophinney commented on an outdated diff Jan 3, 2014
tests/ZendTest/Filter/Word/SeparatorToCamelCaseTest.php
+ $this->assertNotEquals($input, $filtered);
+ $this->assertEquals(array('CamelCasedWords', 'SomethingDifferent'), $filtered);
+ }
+
+ /**
+ * @return void
+ */
+ public function testReturnUnfiltered()
+ {
+ $filter = new SeparatorToCamelCaseFilter();
+
+ $valuesExpected = array(
+ null,
+ new \stdClass()
+ );
+ foreach ($valuesExpected as $input) {
@weierophinney
Zend Framework member
weierophinney added a line comment Jan 3, 2014

Use a data provider instead of a foreach loop.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@weierophinney weierophinney commented on an outdated diff Jan 3, 2014
tests/ZendTest/Filter/Word/SeparatorToSeparatorTest.php
@@ -48,4 +65,19 @@ public function testFilterSeparatesWordsWithSearchAndReplacementSpecified()
$this->assertEquals('dash?separated?words', $filtered);
}
+ /**
+ * @return void
+ */
+ public function testReturnUnfiltered()
+ {
+ $filter = new SeparatorToSeparatorFilter('=', '?');
+
+ $valuesExpected = array(
+ null,
+ new \stdClass()
+ );
+ foreach ($valuesExpected as $input) {
@weierophinney
Zend Framework member
weierophinney added a line comment Jan 3, 2014

Use a data provider instead of a foreach loop.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@weierophinney weierophinney commented on an outdated diff Jan 3, 2014
tests/ZendTest/I18n/Filter/AlnumTest.php
+ );
+
+ $actual = $filter->filter(array_keys($values));
+
+ $this->assertEquals(array_values($values), $actual);
+ }
+
+ public function testReturnUnfiltered()
+ {
+ $filter = new AlnumFilter();
+
+ $valuesExpected = array(
+ null,
+ new \stdClass()
+ );
+ foreach ($valuesExpected as $input) {
@weierophinney
Zend Framework member
weierophinney added a line comment Jan 3, 2014

Use a data provider instead of a foreach loop.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@weierophinney weierophinney commented on an outdated diff Jan 3, 2014
tests/ZendTest/I18n/Filter/AlphaTest.php
+ );
+
+ $actual = $filter->filter(array_keys($values));
+
+ $this->assertEquals(array_values($values), $actual);
+ }
+
+ public function testReturnUnfiltered()
+ {
+ $filter = new AlphaFilter();
+
+ $valuesExpected = array(
+ null,
+ new \stdClass()
+ );
+ foreach ($valuesExpected as $input) {
@weierophinney
Zend Framework member
weierophinney added a line comment Jan 3, 2014

Use a data provider instead of a foreach loop.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@weierophinney weierophinney commented on an outdated diff Jan 3, 2014
tests/ZendTest/I18n/Filter/NumberFormatTest.php
+ public function testReturnUnfiltered()
+ {
+ $filter = new NumberFormatFilter('de_AT', NumberFormatter::DEFAULT_STYLE, NumberFormatter::TYPE_DOUBLE);
+ $this->assertEquals($expected, $filter->filter($value));
+
+ $filter = new AlphaFilter();
+
+ $valuesExpected = array(
+ null,
+ new \stdClass(),
+ array(
+ '1.234.567,891',
+ '1.567,891'
+ )
+ );
+ foreach ($valuesExpected as $input) {
@weierophinney
Zend Framework member
weierophinney added a line comment Jan 3, 2014

Use a data provider instead of a foreach loop.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@weierophinney weierophinney commented on an outdated diff Jan 3, 2014
tests/phpunit.xml.dist
@@ -1,4 +1,4 @@
-<phpunit bootstrap="./Bootstrap.php" colors="true">
+<phpunit bootstrap="./Bootstrap.php" colors="false">
@weierophinney
Zend Framework member
weierophinney added a line comment Jan 3, 2014

Revert this change.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@weierophinney
Zend Framework member

@ThaDafinser Looking good! Do you have an estimate on when you can incorporate feedback and get the i18n filters completed?

@ThaDafinser

@samsonasik coding standards should be fine now. (i hope i didn't missed a spot...is there also a automatically fixer?)
@weierophinney the phunit.xml which is reverted

About the notes for the "null" check. It's the same behaviour....null is not a scalar, so it return itself (which is null)

/* original */
if (null === $value) {
    return null;
}
if (!is_scalar($value)) {
    return $value;
}

/* new */
//the same, if the value is null -> it will return null...
if (!is_scalar($value)) {
    return $value;
}

Missing
foreach / data provider for unittests....don't know when i have time....:-/

@ThaDafinser

In my second last commit, i fixed the call in invoke.

I don't understand why null have to be passed, but i let now pass null again in my last commit.

namespace ZendTest\Filter;

use Zend\Filter\Decompress as DecompressFilter;

/**
 * @group      Zend_Filter
 */
class DecompressTest extends \PHPUnit_Framework_TestCase
    public function testCompressToFile()
    {
        $filter   = new DecompressFilter('bz2');
        $archive = __DIR__ . '/../_files/compressed.bz2';
        $filter->setArchive($archive);

        $content = $filter->compress('compress me');
        $this->assertTrue($content);

        $filter2  = new DecompressFilter('bz2');
        $content2 = $filter2($archive);
        $this->assertEquals('compress me', $content2);

        $filter3 = new DecompressFilter('bz2');
        $filter3->setArchive($archive);
        $content3 = $filter3(null);
        $this->assertEquals('compress me', $content3);
    }
@ThaDafinser

@weierophinney everything fine or not?

@weierophinney
Zend Framework member

@ThaDafinser Yes, I think so -- need to review again. (Been busy getting Apigility ready for beta...)

@dstockto

This would be good to have since currently adding filters in apigility causes breakage if the inputs come in as something the filters cannot deal with.

@weierophinney weierophinney added a commit that referenced this pull request Feb 4, 2014
@weierophinney weierophinney [#5436] Added information to README
- Detailing what the PR accomplishes, as it's a slight change in behavior.
1ed030b
@weierophinney weierophinney added a commit that referenced this pull request Feb 4, 2014
@weierophinney weierophinney Merge branch 'feature/5436' into develop
Close #5436
18e2f00
@weierophinney weierophinney added this to the 2.3.0 milestone Feb 4, 2014
@weierophinney weierophinney self-assigned this Feb 4, 2014
@weierophinney
Zend Framework member

Merged to develop for release with 2.3.0.

@ThaDafinser

Yiiiha :-) finally

@dstockto

@ThaDafinser Thanks for doing this, it solves a big problem I was having

@weierophinney weierophinney added a commit to zendframework/zend-i18n that referenced this pull request May 15, 2015
@weierophinney weierophinney Merge pull request zendframework/zendframework#5436 from ThaDafinser/…
…ZendFilter

[WIP] Zend\Filter harmonization (Issue 5119)

Conflicts:
	library/Zend/I18n/Filter/NumberFormat.php
69bd82b
@weierophinney weierophinney added a commit to zendframework/zend-i18n that referenced this pull request May 15, 2015
@weierophinney weierophinney Merge branch 'feature/5436' into develop 9f9c3cd
@weierophinney weierophinney added a commit to zendframework/zend-filter that referenced this pull request May 15, 2015
@weierophinney weierophinney Merge pull request zendframework/zendframework#5436 from ThaDafinser/…
…ZendFilter

[WIP] Zend\Filter harmonization (Issue 5119)

Conflicts:
	library/Zend/I18n/Filter/NumberFormat.php
1069251
@weierophinney weierophinney added a commit to zendframework/zend-filter that referenced this pull request May 15, 2015
@weierophinney weierophinney Merge branch 'feature/5436' into develop 77f0d14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment