Url validation may cause an error if an object was submitted

[TheKeymaster]

How to reproduce

When you try to validate an URL containing an object like this example:

src/bestFileName.php

$validator = new Validator(['someUrl' => new \stdClass()]);

$validator->rule('url', 'someUrl');

if (!$validator->validate()) {
    die('You are a bad person!');
}

Expected Behavior

You should get the error message: You are a bad person!.

Actual Behavior

You will get an exception message: strpos() expects parameter 1 to be string, object given.

Stacktrace:

/project/vendor/vlucas/valitron/src/Valitron/Validator.php:612
/project/vendor/vlucas/valitron/src/Valitron/Validator.php:1081
/project/src/bestFileName.php:5

[TheKeymaster]

I have created this issue over a week ago. Had anyone time to look at this? I can also create a PR if its being merged soonish then.

[willemwollebrants]

The problem is that valitron right now will, as you point out, throw an exception if the wrong kind of input is given. See also #182

There are a few options (also outlined there):

• have valitron try to cast the value to a string. That way an object could pass (by implementing __toString for example), but you're not testing the exact input.
• fail validation (an url must be a string, after all)
• throw an exception, this is what happens now.

I think the original use-case for valitron was checking submitted data from forms, so most rules assume that the input will be a string. If it's not, it's usually (in my experience) a programming error, not an input error by the user. I think that for programming errors, exceptions are the best way to notify the program that something unexpected happened, but I'm open to have you (or anyone else) change my mind on this :)

[TheKeymaster]

Thank you for your answer, sorry that I am a bit late.

I thought a bit about this and also discussed with other people on their opinion about this behavior.
When I use a library for validation of URLs and I submit an object where actually a string is required, I do expect an exception.

It would be great if it was possible to check if it is a string and may throw an InvalidArgumentException if it is not, to indicate that something is wrong here. Not only does this tell us, the developers, that something in our code is wrong, but also tells us what is wrong and that this is expected behavior.

Also I do not think that it is good to trust strpos() will always throw an exception later on, because this may change in the future.

I'm curious for your answer and opinion on this :). If you also agree, I am up to create a PR with all the changes. Probably a big change in my eyes since we might need a Class where we can tell what type of variable we accept for each available rule.

EDIT:
After discussing with even more people they may think of this as prevention of workflow. Because if you are throwing an exception here, when actually we only want to validate some values, we need to create a try {} catch (){} block only for this case. Maybe just returning false when validating in this case, may be sufficient.

[willemwollebrants]

I did a bit of thinking and I think the fastest way to have maximum flexibility without changing the current behavior might be something like this

It's just a concept for now. There are 3 new options you could set:
First of all, should we check the input type?

$validator->guardInputTypes();

If we do, there are 2 options for dealing with invalid input:

//can we try to cast invalid input to the expected type?
$validator->castInputTypes();
//should we throw an exception if the type is wrong (and casting failed)? 
$validator->throwInvalidArgumentException();

So to get the behavior you'd like to see, you could do this:

$validator = new Validator(['someUrl' => new \stdClass()]);
$validator->guardInputTypes(true);
$validator->throwInvalidArgumentException(false);
$validator->rule('url', 'someUrl');

if (!$validator->validate()) {
    die('You are a bad person!');
}

What do you think?

[TheKeymaster]

I think this concept is a good idea.
With this change users can decide if they want to ignore exceptions and later handle the validation without any exceptions. I would rename Validator::throwInvalidArgumentException() to Validator::throwExceptionsForInvalidInputTypes() because I think its more understandable in that way.

Other than that it looks very promising. Nice 👍!