Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Added null protection on validation #61

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

rcerljenko
Copy link

We use this package in our company and we had a situation when frontend sends a null value to the validated field causing an TypeError exception to be thrown like so:

/.../vendor/sunspikes/clamav-validator/src/ClamavValidator/ClamavValidator.php:140

Sunspikes\ClamavValidator\ClamavValidator::getFilePath(): Return value must be of type string, null returned

This PR resolves that by skipping a validation if the validated field is null.

@rcerljenko
Copy link
Author

@sunspikes could you take a look please.. thanks!

@sunspikes
Copy link
Owner

@rcerljenko sorry that i missed this PR, If you just want the field to accept null values, did you try: https://laravel.com/docs/9.x/validation#rule-nullable

You could basically do

$rules = [
    'file' => 'nullable|clamav',
];

@marcovo-peercode
Copy link

Adding nullable could be a work-around, but not a solution to this problem. In line with standard Laravel rules, a validation rule should always return a valid conclusion (true/false) rather than throwing an exception.

In particular: I think the best approach, in line with standard Laravel rules, would be to let the rule return false whenever an empty value is received. This in contrast with the here proposed change.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants