[5.8.12] In-correct validation message for gte:value

[ankurk91]

• Laravel Version: 5.8.12
• PHP Version: 7.2
• Database Driver & Version: na

Description:

Looks like 5.8.12 introduce this issue.
The PR #28174 fixed the long running issue of gte validation throwing type mismatch exceptions but introduce this one.

Steps To Reproduce:

       $canBeNull = null; // assume it is coming from client
        $validator = \Validator::make([
            'min_rate' => null,
            'max_rate' => 100,
        ], [
            'min_rate' => ['required', 'numeric',],
            'max_rate' => ['required', 'numeric', 'gte:'.$canBeNull],
        ]);

        dd($validator->errors()->toArray());

Result

array:2 [
  "min_rate" => array:1 [
    0 => "The min rate field is required."
  ]
  "max_rate" => array:1 [
    0 => "The max rate must be greater than or equal 2."
  ]
]

Notice the 2 in error message.

[driesvints]

Hmm, annoying. When we fix this we should add a regression test that makes sure this isn't re-introduced again.

[MattStrauss]

It looks like the issue is in ValidatesAttributes File. In the validateGte method the below line returns an array of the user supplied data.

$comparedToValue = $this->getValue($parameters[0]);

So in the original post example above this would return the below:
array:2 [▼
"min_rate" => null
"max_rate" => 100
]

The reason it returns the above array is that above getValue method is in the Validator class file, see below:

protected function getValue($attribute)
    {
        return Arr::get($this->data, $attribute);
    }

Notice that it passes in the $attribute $var, which in the original posters case is equal to null. But it also passes in $this->data which is the user supplied data array from above to the Arr:get helper method, which is shown below:

public static function get($array, $key, $default = null)
    {
        if (! static::accessible($array)) {
            return value($default);
        }

        if (is_null($key)) {
            return $array;
        }

        if (static::exists($array, $key)) {
            return $array[$key];
        }

        if (strpos($key, '.') === false) {
            return $array[$key] ?? value($default);
        }

        foreach (explode('.', $key) as $segment) {
            if (static::accessible($array) && static::exists($array, $segment)) {
                $array = $array[$segment];
            } else {
                return value($default);
            }
        }

        return $array;
    }

Since the $key argument is null, the below case is used, which just returns the original user supplied data,

if (is_null($key)) {
            return $array;
        }

Then when we get to the below if statement in the validateGte method, it's going to return false since it is comparing an array and integer.

if (! $this->isSameType($value, $comparedToValue)) {
            return false;
        }

And since it just returns false without supplying any logic that it was a type mismatch, it is sent to the Validator class's addFailure method. And eventually returns the failure message from ReplacesAttributes.php, see below:

 protected function replaceGte($message, $attribute, $rule, $parameters)
    {

    	if (is_null($value = $this->getValue($parameters[0]))) {
            return str_replace(':value', $parameters[0], $message);
        }

        return str_replace(':value', $this->getSize($attribute, $value), $message);
    }

Here, basically the same thing happens as above. The $value variable set above ends up being the user supplied data again because the $parameters[0] sent through as the $key is null (again).

That takes us to the second return statement in the above method, where getSize is called, see that method below:

protected function getSize($attribute, $value)
    {
        $hasNumeric = $this->hasRule($attribute, $this->numericRules);

        // This method will determine if the attribute is a number, string, or file and
        // return the proper size accordingly. If it is a number, then number itself
        // is the size. If it is a file, we take kilobytes, and for a string the
        // entire length of the string will be considered the attribute size.
        if (is_numeric($value) && $hasNumeric) {
            return $value;
        } elseif (is_array($value)) {
            return count($value);
        } elseif ($value instanceof File) {
            return $value->getSize() / 1024;
        }

        return mb_strlen($value);
    }

The below case is used since the $value argument is an array (of the user supplied data). And it is counted and hence the 2 is returned which is why the user sees the 2 in the error message output. If you would have three or four user supplied pieces of data, then the message would include 3 or 4, respectively.

elseif (is_array($value)) {
            return count($value);

TLDR
All that being said... It might make sense to stop the Validation process on gte and other validations like it (i.e. lte, gt, etc.) once there is a type mismatch. Although, using null as the expected data type/size seems like it may cause other problems due to how the Arr:get helper method currently functions,

Perhaps something like the below would make more sense. If you think so, let me know and I can start a PR.

in ValidatesAttributes.php:

 /**
     * Validate that an attribute is greater than or equal another attribute.
     *
     * @param  string  $attribute
     * @param  mixed   $value
     * @param  array   $parameters
     * @return bool
     */
    public function validateGte($attribute, $value, $parameters)
    {
	    $this->requireParameterCount(1, $parameters, 'gte');

	    $comparedToValue = $this->getValue($parameters[0]);

	    $this->shouldBeNumeric($attribute, 'Gte');

	    **return $this->sameTypeCheck($attribute, $value, $parameters[0]);**

	    if (is_null($comparedToValue) && (is_numeric($value) && is_numeric($parameters[0]))) {

		    return $this->getSize($attribute, $value) >= $parameters[0];

	    }

//	    if (! $this->isSameType($value, $comparedToValue)) {
//
//		    return false;
//
//	    }

	    return $this->getSize($attribute, $value) >= $this->getSize($attribute, $comparedToValue);
	    
    }

	/**
	 * Check if the parameters are of the same type.
	 *
	 * @param  string $attribute
	 * @param  mixed  $userValue
	 * @param  mixed  $comparedToValue
	 * @return bool
	 */
	protected function sameTypeCheck($attribute, $userValue, $comparedToValue)
	{
		$requiredType = gettype($comparedToValue);

		if (gettype($userValue) != $requiredType) {
			$this->addFailure($attribute, 'type_mismatch', ['type' => $requiredType]);
			return true; // bail
		}
	}

And then adding the below to Validations.php:
'type_mismatch' => 'The :attribute field must be of type :type.',

Finally, adding the below to ReplaceAttributes.php:

/**
     * Replace all place-holders for the type-mismatch rule.
     *
     * @param  string  $message
     * @param  string  $attribute
     * @param  string  $rule
     * @param  array   $parameters
     * @return string
     */
protected function replaceType_mismatch($message, $attribute, $rule, $parameters)
	{

		return str_replace(':type', $parameters['type'], $message);
	}

[MattStrauss]

Per comment on closed PR, seems like this issue can be closed: #28474 (comment)