You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
I have looked at the examples provided that may showcase my question here?
Package version eg. v9, v10: v10
Issue, Question or Enhancement:
In #615 the country_code alias is introduced which calls 3 validators iso3166_1_alpha2|iso3166_1_alpha3|iso3166_1_alpha_numeric.
If you use the country_code to validate a string which has an invalid value you will receive a panic with the message Bad field type string. This is caused by the iso3166_1_alpha_numeric validation here
I would expect that the alias should not panic here but instead return false.
Fix
I was planning to make a PR for this issue but the fix may require some big changes and I thought it may be good to discuss them before making any kind of changes.
A simple solution would be to catch any panic for an alias, this makes sense for country_code which can have multiple valid types.
An easier solution would be to just remove the panic in the iso3166_1_alpha_numeric function but then it would differ from the rest of the package. That raises another question of whether the package should be throwing panics in the first place?
I am not sure what the best way to solve this would be and would look for input from maintainers or perhaps @krhubert who added this functionality.
The text was updated successfully, but these errors were encountered:
It looks like when the country code validations were added support for string types wasn't added. I think all that needs to be done is to add a new case statement for string field data from a string to an int or uint and then do the same validation that exists today here
I'd rather keep the panic than returning false because string fields were NOT supported by this validation, returning false would give the indication it was working but...it's not.
It would be greatly appreciated if you could create a PR to add those other cases statements.
Package version eg. v9, v10: v10
Issue, Question or Enhancement:
In #615 the
country_code
alias is introduced which calls 3 validatorsiso3166_1_alpha2|iso3166_1_alpha3|iso3166_1_alpha_numeric
.If you use the
country_code
to validate a string which has an invalid value you will receive a panic with the messageBad field type string
. This is caused by theiso3166_1_alpha_numeric
validation herevalidator/baked_in.go
Line 2359 in ce34f36
Code sample, to showcase or reproduce:
This is a test I wrote to confirm the issue, you will receive a panic on
"1"
.Expected behaviour
I would expect that the alias should not panic here but instead return false.
Fix
I was planning to make a PR for this issue but the fix may require some big changes and I thought it may be good to discuss them before making any kind of changes.
A simple solution would be to catch any panic for an alias, this makes sense for
country_code
which can have multiple valid types.An easier solution would be to just remove the panic in the
iso3166_1_alpha_numeric
function but then it would differ from the rest of the package. That raises another question of whether the package should be throwing panics in the first place?I am not sure what the best way to solve this would be and would look for input from maintainers or perhaps @krhubert who added this functionality.
The text was updated successfully, but these errors were encountered: