filterus icon indicating copy to clipboard operation
filterus copied to clipboard

Possible bug? Invalid outcome of validation

Open pquerner opened this issue 9 years ago • 8 comments

Having an array like

array(19) {
  'sku' =>
  string(2) "SW"
  '_type' =>
  string(6) "simple"
  '_attribute_set' =>
  string(18) "xxxxx"
  '_product_websites' =>
  string(10) "xxxxx"
  'name' =>
  NULL
  'description' =>
  string(8) "TESTTEST"
  'short_description' =>
  string(8) "TESTTEST"
  'price' =>
  int(0)
  'url_key' =>
  NULL
  'cost' =>
  NULL
  'status' =>
  int(1)
  'visibility' =>
  int(4)
  'manufacturer' =>
  string(9) "xxxx"
  'tax_class_id' =>
  int(2)
  'base_price_unit' =>
  string(3) "PCS"
  'base_price_base_unit' =>
  string(3) "PCS"
  'country_of_manufacture' =>
  string(11) "xxxxx"
  'qty' =>
  int(0)
  'is_in_stock' =>
  int(1)
}

And with this filter:

$filter = Filter::map(array(
                'name'              => 'string,min:1', //anything above 1 should do
                'sku'               => 'string,min:3', //prefix is hardcoded, so anything above strlen 2 will do
                '_attribute_set'    => 'string,min:1', //TODO Should be hardcoded to whatever is in $conf
                '_product_websites' => 'string,min:1',
                '_type'             => 'string,min:1', //TODO Either configurable or simple
            ));

I would suspect that $filter->validate($product) would throw back false since "name" is clearly a) not a string and b) is not having a stringlengh of atleast 1.

But I do get true as a result, which is false.

pquerner avatar Nov 06 '14 09:11 pquerner

Or even this simple example:

$array = array(
                    'foo' => 2,
                    'bar' => 'test',
                    'name' => NULL,
                );

                var_dump($array);

/*

array(3) {
  'foo' =>
  int(2)
  'bar' =>
  string(4) "test"
  'name' =>
  NULL
}

*/

                $filter = Filter::map(array(
                    'foo' => 'int',
                    'bar' => 'string,min:4',
                    'name' => 'string,min:1',
                ));

                var_dump($filter->validate($array)); //bool(true)

pquerner avatar Nov 06 '14 09:11 pquerner

Min value doesn't have effect on empty strings.

NiLon avatar Jan 08 '15 09:01 NiLon

But it should've, because like in this example the result is not correct. I expect a string, get a NULL and its still valid.

pquerner avatar Jan 09 '15 07:01 pquerner

Min value doesn't have effect on empty strings.

@NiLon - I have just discovered this. Are you saying this is considered correct?

I will put in a regex for now, but this seems to be a bug to me.

halfer avatar Mar 11 '18 12:03 halfer

Ah, I found the bug, it is indeed wrong. A minimum length of one is correctly enforced in an ordinary non-map filter, but there is a bug in the Map class:

public function validate($var) {
    if (!is_object($var) && !is_array($var)) {
        return false;
    }
    if (is_object($var)) {
        return $var == $this->filter($var);
    }
    return $var == $this->filter($var);
}

Just before the return I do this:

var_dump($this->filter($var));
var_dump($var);

On an empty string with a min length validation of 1, I get this (filtered version first, raw version second):

array(2) {
  ["foo"]=>
  int(2)
  ["bar"]=>
  NULL
}
array(2) {
  ["foo"]=>
  int(2)
  ["bar"]=>
  string(0) ""
}

However these are treated as equal by $var == $this->filter($var) because === is not used.

halfer avatar Mar 11 '18 13:03 halfer

I've fixed this using the following subclass:

<?php

/**
 * Repairs a bug in the Filterus library
 *
 * @see https://github.com/ircmaxell/filterus/issues/5
 */

namespace Missive\Filter;

use Filterus\Filters\Map as FMap;

class Map extends FMap
{
    public function validate($var)
    {
        if (!is_object($var) && !is_array($var))
        {
            return false;
        }

        return $var === $this->filter($var);
    }

    public static function map($filters)
    {
        return new self(array('filters' => $filters));
    }
}

If anyone bumps into this, feel free to use this code (same license as Filterus itself) and swap the namespace. Maintainers, please ask me to raise a PR, if this lib is still maintained.

halfer avatar Mar 11 '18 13:03 halfer

Urgh. In fact, that won't catch a type failure where a null is provided as a string, since the string will be filtered down to a null anyway.

It would be better to loop through the filters in the validate() method and apply them manually.

halfer avatar Mar 11 '18 13:03 halfer

I've modified my version of validate() above, which passes more of my own tests:

    public function validate($var)
    {
        if (!is_object($var) && !is_array($var))
        {
            return false;
        }

        $ok = true;
        foreach ($this->options['filters'] as $key => $filterStr)
        {
            $filter = self::factory($filterStr);
            $value = isset($var[$key]) ? $var[$key] : null;
            $ok = $filter->validate($value);
            if (!$ok)
            {
                break;
            }
        }

        return $ok;
    }

halfer avatar Mar 11 '18 17:03 halfer