gin: gin crashes when binding JSON array of objects because of forced validation

My app receives requests which are JSON arrays of objects. I’m representing these data using slices and map[string]interface{}, since the objects can have unknown fields.

I’ve noticed that latest changes to Gin cause are assuming the binding is using structs only and performing validation on them. This is causing gin to crash if trying to bind on something other than struct, like a slice of maps.

type Object map[string]interface{}
type MyObjects []Object

func MyHandler(context *gin.Context) {
    var objects MyObjects
    context.Bind(&objects)  // Request is `[{"key1":"value1"},{"key2":"value2"},..]`
    ...
}

2015/05/27 17:16:51 Panic recovery -> interface passed for validation is not a struct
/home/ory/.gvm/gos/go1.4.1/src/runtime/panic.go:387 (0x4168a8)
        gopanic: reflectcall(unsafe.Pointer(d.fn), deferArgs(d), uint32(d.siz), uint32(d.siz))
/home/ory/.gvm/pkgsets/go1.4.1/global/src/gopkg.in/bluesuncorp/validator.v5/validator.go:177 (0x6d3c49)
        (*Validate).structRecursive: panic("interface passed for validation is not a struct")
/home/ory/.gvm/pkgsets/go1.4.1/global/src/gopkg.in/bluesuncorp/validator.v5/validator.go:173 (0x6d3b89)
        (*Validate).structRecursive: return v.structRecursive(top, current, structValue.Elem().Interface())
/home/ory/.gvm/pkgsets/go1.4.1/global/src/gopkg.in/bluesuncorp/validator.v5/validator.go:162 (0x6d3960)
        (*Validate).Struct: return v.structRecursive(s, s, s)
/home/ory/.gvm/pkgsets/go1.4.1/global/src/github.com/gin-gonic/gin/binding/binding.go:59 (0x5e4680)
        Validate: if err := validate.Struct(obj); err != nil {
/home/ory/.gvm/pkgsets/go1.4.1/global/src/github.com/gin-gonic/gin/binding/json.go:24 (0x5e5aae)
        jsonBinding.Bind: return Validate(obj)
<autogenerated>:6 (0x5e5ff7)
/home/ory/.gvm/pkgsets/go1.4.1/global/src/github.com/gin-gonic/gin/context.go:262 (0x4f05b2)
        (*Context).BindWith: if err := b.Bind(c.Request, obj); err != nil {
/home/ory/.gvm/pkgsets/go1.4.1/global/src/github.com/gin-gonic/gin/context.go:254 (0x4f046e)
        (*Context).Bind: return c.BindWith(obj, b)
...

The question arises: Why is gin forcing me to validate my objects? More so, validate using https://github.com/bluesuncorp/validator . What if I want to validate using another preferred validator package, like https://github.com/asaskevich/govalidator ? What if I don’t want to validate at all?

I feel like this type of control should be given to the developer, and not forced upon him. Maybe validation is better off as an optional (or default) middleware instead?

About this issue

Original URL
State: closed
Created 9 years ago
Comments: 18 (7 by maintainers)

Commits related to this issue

Fix for #310 — committed to gin-gonic/gin by manucorporat 9 years ago
Better bind error messages — committed to gin-gonic/gin by manucorporat 9 years ago

Most upvoted comments

@oryband well, it is not really that bad:

The validator will not validate anything unless there is a binding:“required” tag.
Even though it is a non-nil by default, the validator is lazy initialized so it does not allocate a lot of memory.

I know this is not great for you, but I am sure many people will disagree if I disable it by default. Gin is a very fast framework, right now even faster than httpRouter. But it is a framework, it includes more features than a muxer. So built-in validation still makes sense.

As I said other times, Gin is about a well designed balance between correctness, features and performance.

Check out the framework overhead: https://www.techempower.com/benchmarks/#section=data-r10&hw=ec2&test=plaintext

and that benchmarks used Gin 0.6, right now the overhead is even lower.

func init() {
    // shortcut for binding.Validator = nil, so you do not have to import the package.
    gin.DisableBindValidation()
    // gin.SetMode(gin.ReleaseMode)
}

manucorporat on May 31, 2015

@manucorporat this does fix the problem, but I feel the concept is wrong: Binding should just perform an unmarshal of the content based on Content-Type.

It should not perform implicit validations. In addition, this causes some unnecessary reflections to be executed. This shouldn’t affect performance that much, but the key point we chose to use Gin here at Rounds was because Gin is fast - so let’s go all the way with that and not force implicit reflections where it’s not strictly necessary. Also, if I decide to validate using my own validator, that Validate() call inside Bind() is unnecessary, because I call my own Validate().

If I wish to validate I can just do so manually after binding, i.e.:

var v MyObject
if err := context.Bind(&v); err != nil {
    // Handle binding error.
}
if err := validate.Struct(&v); err != nil {  // Choose the validation library of my preference.
    // Handle validation error.
}
// Continue

At the very least, validation could be performed implicitly by setting an off-by-default flag on initialization, similar to ginmode. But I still prefer it wouldn’t be there entirely.

wdyt?

oryband on May 31, 2015