Panic Bad field type *int64, when using required_if and omitempty at the same time

[chrisdibartolo]

• [X] I have looked at the documentation here first?
• [X] I have looked at the examples provided that may showcase my question here?

Package version eg. v9, v10:

v10.10.0

Issue, Question or Enhancement:

When validating an int64 pointer with required_if and omitempty at the same time, calling Struct() leads to panic.

Using omitempty merely is ok. Validating an int64 instead of an int64 pointer is ok.

Similar to: Panic Bad field type *string, when using required_without and omitempty at the same time #654

Code sample, to showcase or reproduce:

package main

import (
	"github.com/go-playground/validator/v10"
	"log"
)

type Sample struct {
	Type     string `json:"type" validate:"required"`
	Quantity int64  `json:"quantity" validate:"required_if=Type special,gte=2"`
}

type SampleWithPointer struct {
	Type     string `json:"type" validate:"required"`
	Quantity *int64 `json:"quantity,omitempty" validate:"required_if=Type special,gte=2"`
}

func main() {

	_type := "notspecial"
	s := &Sample{Type: _type}
	swp := &SampleWithPointer{Type: _type}

	validate := validator.New()
	if err := validate.Struct(s); err != nil {
		log.Printf("Validate sample with error, %v", err)
	}
	if err := validate.Struct(swp); err != nil {
		log.Printf("Validate SampeWithPointer with error, %v", err)
	}
}

stack trace:

2009/11/10 23:00:00 Validate sample with error, Key: 'Sample.Quantity' Error:Field validation for 'Quantity' failed on the 'gte' tag
panic: Bad field type *int64

goroutine 1 [running]:
github.com/go-playground/validator/v10.isGte({0x59b798, 0xc0000d1c20})
	/tmp/gopath931800745/pkg/mod/github.com/go-playground/validator/[email protected]/baked_in.go:1789 +0x4dd
github.com/go-playground/validator/v10.wrapFunc.func1({0x534940, 0xc0000ac208}, {0x59b798, 0xc0000d1c20})
	/tmp/gopath931800745/pkg/mod/github.com/go-playground/validator/[email protected]/baked_in.go:40 +0x2d
github.com/go-playground/validator/v10.(*validate).traverseField(0xc0000d1c20, {0x599a70, 0xc0000ba000}, {0x5471a0, 0xc0000ac1f8, 0x13}, {0x534940, 0xc0000ac208, 0x85}, {0xc000350300, ...}, ...)
	/tmp/gopath931800745/pkg/mod/github.com/go-playground/validator/[email protected]/validator.go:445 +0x16a7
github.com/go-playground/validator/v10.(*validate).validateStruct(0xc0000d1c20, {0x599a70, 0xc0000ba000}, {0x534e00, 0xc0000ac1f8, 0x1}, {0x5471a0, 0xc0000ac1f8, 0xc0000a31e0}, {0x59bd60, ...}, ...)
	/tmp/gopath931800745/pkg/mod/github.com/go-playground/validator/[email protected]/validator.go:77 +0x765
github.com/go-playground/validator/v10.(*Validate).StructCtx(0xc0003492c0, {0x599a70, 0xc0000ba000}, {0x534e00, 0xc0000ac1f8})
	/tmp/gopath931800745/pkg/mod/github.com/go-playground/validator/[email protected]/validator_instance.go:344 +0x430
github.com/go-playground/validator/v10.(*Validate).Struct(...)
	/tmp/gopath931800745/pkg/mod/github.com/go-playground/validator/[email protected]/validator_instance.go:317
main.main()
	/tmp/sandbox1464673145/prog.go:29 +0x13d

Program exited.

[berabulut]

This is happening because validator is trying to run gte=2 validation on a nil value. Adding omitempty before gte=2 should fix it.

type SampleWithPointer struct {
	Type     string `json:"type" validate:"required"`
	Quantity *int64 `json:"quantity,omitempty" validate:"required_if=Type special,omitempty,gte=2"`
}

Adding omitempty to json tag doesn't affect validator's behaviour.

[zemzale]

This workaround seems to be fixing the issue, but we should probably fix this, so it doesn't panic.

[berabulut]

@zemzale I think adding this code block fixes the validation. But we need to add this block to every validation that panics with nil values.

func isGte(fl FieldLevel) bool {
	field := fl.Field()
	param := fl.Param()

	switch field.Kind() {

	case reflect.Pointer:
		if field.IsNil() {
			return false
		}

		field = field.Elem()

	case reflect.String:
        ...

https://github.com/go-playground/validator/blob/c7e0172e0fd176bdc521afb5186818a7db6b77ac/baked_in.go#L1815

[sandeep-adireddi]

I don't think the omitempty tag is relevant in this case. It's actually the required_if tags that set the pointer to nil if fields aren't assigned a value during struct initialization.

package main

import (
	"log"

	"github.com/go-playground/validator/v10"
)

type SamplPointerWithoutRequiredTag struct {
	Type     string `json:"type" validate:"required"`
	Quantity *int64 `json:"quantity" validate:"gte=2"`
}

type SamplPointerWithRequiredTag struct {
	Type     string `json:"type" validate:"required"`
	Quantity *int64 `json:"quantity" validate:"required_if=Type notspecial,eq=3"`
}

func main() {

	_type := "special"
	var q int64 = 3
	s1 := &SamplPointerWithoutRequiredTag{Type: _type, Quantity: &q}
	s2 := &SamplPointerWithoutRequiredTag{Type: _type}
	s3 := &SamplPointerWithRequiredTag{Type: _type, Quantity: &q}
	s4 := &SamplPointerWithRequiredTag{Type: _type}

	validate := validator.New()
	if err := validate.Struct(s1); err != nil {
		log.Printf("Validate SamplPointerWithoutRequiredTag  and given input for field, %v", err)
	}
	if err := validate.Struct(s2); err != nil {

		log.Printf("Validate SamplPointerWithRequiredTag and without input for field, %v", err)
	}
	if err := validate.Struct(s3); err != nil {

		log.Printf("Validate SamplPointerWithRequiredTag and  given input for field, %v", err)
	}
	if err := validate.Struct(s4); err != nil {

		log.Printf("Validate SamplPointerWithRequiredTag and without input for field, %v", err)
	}
}

Stack Trace:

2009/11/10 23:00:00 Validate SamplPointerWithRequiredTag and without input for field, Key: 'SamplPointerWithoutRequiredTag.Quantity' Error:Field validation for 'Quantity' failed on the 'gte' tag
panic: Bad field type *int64

goroutine 1 [running]:
github.com/go-playground/validator/v10.isEq({0x60b078, 0xc0003e0120})
	/tmp/gopath1756549367/pkg/mod/github.com/go-playground/validator/[email protected]/baked_in.go:1308 +0x3b9
github.com/go-playground/validator/v10.wrapFunc.func1({0xc0003e0120?, 0xc000398600?}, {0x60b078?, 0xc0003e0120?})
	/tmp/gopath1756549367/pkg/mod/github.com/go-playground/validator/[email protected]/baked_in.go:43 +0x2d
github.com/go-playground/validator/v10.(*validate).traverseField(0xc0003e0120, {0x609f90, 0xc0000b4000}, {0x59e020?, 0xc0000a8768?, 0xc0001f6680?}, {0x586de0?, 0xc0000a8778?, 0x1?}, {0xc000398600, ...}, ...)
	/tmp/gopath1756549367/pkg/mod/github.com/go-playground/validator/[email protected]/validator.go:454 +0x1727
github.com/go-playground/validator/v10.(*validate).validateStruct(0xc0003e0120, {0x609f90, 0xc0000b4000}, {0x5872e0?, 0xc0000a8768?, 0x1?}, {0x59e020?, 0xc0000a8768?, 0xc0000aa680?}, {0x60c2c0, ...}, ...)
	/tmp/gopath1756549367/pkg/mod/github.com/go-playground/validator/[email protected]/validator.go:77 +0x750
github.com/go-playground/validator/v10.(*Validate).StructCtx(0xc0003dc5b0, {0x609f90, 0xc0000b4000}, {0x5872e0, 0xc0000a8768?})
	/tmp/gopath1756549367/pkg/mod/github.com/go-playground/validator/[email protected]/validator_instance.go:389 +0x479
github.com/go-playground/validator/v10.(*Validate).Struct(...)
	/tmp/gopath1756549367/pkg/mod/github.com/go-playground/validator/[email protected]/validator_instance.go:362
main.main()
	/tmp/sandbox3121368493/prog.go:40 +0x2c5

Program exited.

[sandeep-adireddi]

@zemzale The code snippet https://github.com/go-playground/validator/blob/bd1113d5c1901c5205fc0a7af031a11268ff13ee/validator.go#L454 executes with the 'runValidationWhenNil' value set to false, and the field value is nil for the GTE tag. I believe this is the reason for the panic in the code mentioned above.

[deankarn]

Sorry for the delay in responding, stealing some time before the kids are up to comment.

I don't believe there is an issue here and that the validations defined is missing accounting for the possibility of the nil value as mentioned by @berabulut. Adding of the omitempty is not a workaround but the correct way to define the validation.

using this below example, lets thinking about it step by step:

type SampleWithPointer struct {
	Type     string `json:"type" validate:"required"`
	Quantity *int64 `json:"quantity,omitempty" validate:"required_if=Type special,gte=2"`
}

1. Check if Quantity should be required conditionally and so have accepted the fact that it may not be required.
2. Validate its gte to 2 and the gte validation does not accept pointers and so returns panics as validation are not correctly defined.

or to put this into pure Go code this is what's being done:

        type SampleWithPointer struct {
		Type     string `json:"type" validate:"required"`
		Quantity *int64 `json:"quantity,omitempty" validate:"required_if=Type special,gte=2"`
	}

	swp := &SampleWithPointer{Type: "notspecial"}

	if swp.Type == "special" && swp.Quantity == nil {
		fmt.Println("returning error required_if")
	} else if swp.Quantity >= 2 { // doesn't even compile because pointer and integer cannot be compared in this way and why the validation panics as defined
                fmt.Println("returning error gte")
	}

What's really desired is this, which is equivalent to required_if=Type special,omitempty,gte=2:

        type SampleWithPointer struct {
		Type     string `json:"type" validate:"required"`
		Quantity *int64 `json:"quantity,omitempty" validate:"required_if=Type special,omitempty,gte=2"`
	}

	swp := &SampleWithPointer{Type: "notspecial"}

	if swp.Type == "special" && swp.Quantity == nil {
		fmt.Println("returning error required_if")
	} else if swp.Quantity != nil && *swp.Quantity >= 2 {
		fmt.Println("returning error gte")
	} else {
		fmt.Println("all good, wasn't required and ok if not set")
	}

Now arguably maybe gte and a bunch of other validations should handle nil values on their own without having to add another tag, agreed and in the next major version this will likely be the case that all validations will have to handle nil values.

But for the time being making such a change would be breaking and cause all sorts of undefined behaviour.

I understand that this may seem suboptimal and that the design of the validation functions & signatures may seem inadequate but also please try to remember a lot has been added to this library over the years and things were designed before there were cross-field functions like required_if, before runValidationWhenNil existed and so forth.

I have been compiling a list with such things for the next major version to improve upon, given how widely used this current version is I'm waiting until there are a significant amount of these changes to warrant such a major version bump.