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

Null not possible #23

Closed
schonhoff opened this issue Sep 28, 2020 · 6 comments · Fixed by #24
Closed

Null not possible #23

schonhoff opened this issue Sep 28, 2020 · 6 comments · Fixed by #24
Labels
bug Something isn't working

Comments

@schonhoff
Copy link
Contributor

Hello,

I get the following error after trying to pass a 'null' to the postal code validation.

TypeError
Argument 2 passed to Axlon\PostalCodeValidation\PostalCodeValidator::passes() must be of the type string,
null given, called in /html/vendor/axlon/laravel-postal-code-validation/src/Extensions/PostalCode.php on line 85

I fixed it by adding a

'bail', 'required'

to my rule set but I think the package should check itself. Maybe you can add an additional test if the given postal code is null and return false. Here my possible workaround:

 /**
     * Determine if the given postal code(s) are valid for the given country.
     *
     * @param string $countryCode
     * @param string|null ...$postalCodes
     * @return bool
     */
    public function passes(string $countryCode, ?string ...$postalCodes): bool
    {
       // Added
        if(!isset($postalCodes)) {
           return false;
        }

        if (!$this->supports($countryCode)) {
            return false;
        }

        if (($pattern = $this->patternFor($countryCode)) === null) {
            return true;
        }

        foreach ($postalCodes as $postalCode) {
            if (preg_match($pattern, $postalCode) !== 1) {
                return false;
            }
        }

        return true;
    }

Thanks for the work and if I need to check in a pr, just ask :-)

@axlon axlon added the bug Something isn't working label Sep 29, 2020
@axlon
Copy link
Owner

axlon commented Sep 29, 2020

Thank you for the bug report @schonhoff. The correct behaviour for receiving null should be validation failing, bail should not be necessary (except if you want to suppress multiple validation errors on the same input) so this sounds like a bug.

Could you tell me you Laravel and Postal code validation versions?

@schonhoff
Copy link
Contributor Author

schonhoff commented Sep 29, 2020

Hello,

no problem. I hope I can help you with this information:

"name": "axlon/laravel-postal-code-validation", "version": "v3.1.0",

"name": "laravel/framework", "version": "v8.6.0",

My current validation rule for my postal code field:

['bail', 'required', 'postal_code:DE']

As you noticed correctly bail shouldn't be necessary, but currently it is for me.
If I return nothing (null) to the validation

$validator = Validator::make($data, $rules, $messages);
if($validator->fails())

throws the error message. With the fixed code on my first post I don't get an error and the validation works like you described.

@schonhoff
Copy link
Contributor Author

Just a hint:

on your src\Extensions\PostalCode Class you are using
public function validate(string $attribute, ?string $value, array $parameters)
on the validate function but in the function passes you are calling there needs to be a string.

public function passes(string $countryCode, string ...$postalCodes): bool

I guess that this is the error. Maybe it is because of php 7.4 that I'm currently using.

@axlon
Copy link
Owner

axlon commented Sep 29, 2020

@schonhoff Could you review the changes I made in PR linked above? I think they should fix your issue. Note that validation will still pass if the given country has no pattern and null is passed

@schonhoff
Copy link
Contributor Author

@axlon copied the changes to my local system and it seems like it fixes the issue. Thanks for the fast fix!

@axlon axlon closed this as completed in #24 Sep 30, 2020
@axlon
Copy link
Owner

axlon commented Sep 30, 2020

@schonhoff I've merged the changes and tagged it as v3.1.1

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants