Gin: Bind validation bug with int zero value

Created on 26 Nov 2015  路  11Comments  路  Source: gin-gonic/gin

The following test should pass, but actually it fails.

func TestValidationSuccess(t *testing.T) {
    type HogeStruct struct {
        Hoge int `json:"hoge" binding:"required"`
    }

    var obj HogeStruct
    req := requestWithBody("POST", "/", `{"hoge": 0}`)
    err := JSON.Bind(req, &obj)
    assert.NoError(t, err)
}

The reason is a validator problem. See more detail hasValue in gopkg.in/bluesuncorp/validator.v5/baked_in.go

In the function, the validation checks with the following line

        return field != nil && field != reflect.Zero(reflect.TypeOf(field)).Interface()

But this is wrong. Because whenever "field == 0", it always fails. Actually if "field != 0", it succeeds. (In my previous test, {"hoge":1} succeeds.)

There are a couple of approaches.

  1. Ask bluesuncorp/validator to fix the problem
  2. Not to use bluesuncorp/validator and fix the problem by ourselves.

What do you think? I wonder most of the people don't use Bind() function and parse JSON by themselves?

Most helpful comment

Hello @honteng,

this is not a problem with validator, I have addressed this before in this issue https://github.com/go-playground/validator/issues/142 with the "exists" tag.

To summarize Go is a static language so when the struct "HogeStruct" is initialized in order to set the JSON values the value of the "Hoge" field will get initialized to "0", the "required" tag does not ensure a field exists, just that it is not the default value.

The only way to do what you want is by using the "exists" tag like so:

func TestValidationSuccess(t *testing.T) {
    type HogeStruct struct {
        Hoge *int `json:"hoge" binding:"exists"`
    }

    var obj HogeStruct
    req := requestWithBody("POST", "/", `{"hoge": 0}`)
    err := JSON.Bind(req, &obj)
    assert.NoError(t, err)
}

So by changing the field value to *int when the field "Hoge" is not posted down it will be nil and will get caught by the "exists" tag. I wish there were a better way to do it, but because of Go's static nature it is the only way to catch if a value exists.

P.S. I really wish Gin would update to v8 of validator as it has many more validations and much better performance, but it would be a breaking change; you can Unmarshall the JSON and validate without using Bind() if you wish.

All 11 comments

Hello @honteng,

this is not a problem with validator, I have addressed this before in this issue https://github.com/go-playground/validator/issues/142 with the "exists" tag.

To summarize Go is a static language so when the struct "HogeStruct" is initialized in order to set the JSON values the value of the "Hoge" field will get initialized to "0", the "required" tag does not ensure a field exists, just that it is not the default value.

The only way to do what you want is by using the "exists" tag like so:

func TestValidationSuccess(t *testing.T) {
    type HogeStruct struct {
        Hoge *int `json:"hoge" binding:"exists"`
    }

    var obj HogeStruct
    req := requestWithBody("POST", "/", `{"hoge": 0}`)
    err := JSON.Bind(req, &obj)
    assert.NoError(t, err)
}

So by changing the field value to *int when the field "Hoge" is not posted down it will be nil and will get caught by the "exists" tag. I wish there were a better way to do it, but because of Go's static nature it is the only way to catch if a value exists.

P.S. I really wish Gin would update to v8 of validator as it has many more validations and much better performance, but it would be a breaking change; you can Unmarshall the JSON and validate without using Bind() if you wish.

Than you for response.

So far, I can use your workaround for our purpose. I didn't know the "exists" tag.

But it could be great to see some tests. Do you want me to write?

Sure, I'm all for any help

Ok, now I have some time to work on the tests.
https://github.com/gin-gonic/gin/pull/504
Please review it.

Thanks,

@honteng it LGTM but one of the repository admins will have to merge, sorry if I gave the impression I was one.

So just waiting for the admin person? Or should I do an extra action?

I'd just wait, it may be a while though as the maintainers of gin are a little busy with school

Hello @honteng,

Yes, as @joeybloggs pointed, we are busy with university, more even now that we have exams. I expect to have more time by the firsts days of February.

Thank you for understanding.

no problem :-)

closing

Since there is no exists tag anymore, we should use *int if we want to allow zero-value. See this issue.

Was this page helpful?
0 / 5 - 0 ratings