gin icon indicating copy to clipboard operation
gin copied to clipboard

[v1.7.0+] BindJSON panics when JSON body is "null"

Open yns01 opened this issue 3 years ago • 7 comments

Description

bindJSON will panic since version 1.7.0 if JSON body is set to "null" for a POST request.

How to reproduce

package main

import (
	"fmt"

	"github.com/gin-gonic/gin"
)

type Example struct {
	ID string `json:"id"`
}

func main() {
	fmt.Println(gin.Version)
	r := gin.New()

	r.POST("/v1/jobs", func(ctx *gin.Context) {
		var ex *Example

		err := ctx.BindJSON(&ex)
		fmt.Println(err)
	})

	r.Run()
}

Expectations

$ curl --location --request POST 'localhost:8080/v1/jobs' \
--header 'Content-Type: application/json' \
--data-raw 'null'
400 Bad Request

Actual result

$ curl --location --request POST 'localhost:8080/v1/jobs' \
--header 'Content-Type: application/json' \
--data-raw 'null'

ct.Value.Interface on zero Value
goroutine 36 [running]:
net/http.(*conn).serve.func1()
        /usr/local/opt/go/libexec/src/net/http/server.go:1802 +0xb9
panic({0x15cd100, 0xc000308600})
        /usr/local/opt/go/libexec/src/runtime/panic.go:1047 +0x266
reflect.valueInterface({0x0, 0x0, 0x105b9a5}, 0x8)
        /usr/local/opt/go/libexec/src/reflect/value.go:1356 +0x10e
reflect.Value.Interface(...)
        /usr/local/opt/go/libexec/src/reflect/value.go:1351
github.com/gin-gonic/gin/binding.(*defaultValidator).ValidateStruct(0x15ea7a0, {0x15ea7a0, 0x0})
        /Users/yanis/go/pkg/mod/github.com/gin-gonic/[email protected]/binding/default_validator.go:45 +0xf2
github.com/gin-gonic/gin/binding.(*defaultValidator).ValidateStruct(0xc00013f2c0, {0x15927c0, 0xc0000100b0})
        /Users/yanis/go/pkg/mod/github.com/gin-gonic/[email protected]/binding/default_validator.go:45 +0x105
github.com/gin-gonic/gin/binding.validate(...)
        /Users/yanis/go/pkg/mod/github.com/gin-gonic/[email protected]/binding/binding.go:117
github.com/gin-gonic/gin/binding.decodeJSON({0x1f7d398, 0xc000452fc0}, {0x15927c0, 0xc0000100b0})
        /Users/yanis/go/pkg/mod/github.com/gin-gonic/[email protected]/binding/json.go:55 +0x9b
github.com/gin-gonic/gin/binding.jsonBinding.Bind({}, 0x1e005b8, {0x15927c0, 0xc0000100b0})
        /Users/yanis/go/pkg/mod/github.com/gin-gonic/[email protected]/binding/json.go:37 +0x5f
github.com/gin-gonic/gin.(*Context).ShouldBindWith(...)
        /Users/yanis/go/pkg/mod/github.com/gin-gonic/[email protected]/context.go:703
github.com/gin-gonic/gin.(*Context).MustBindWith(0xc000438100, {0x15927c0, 0xc0000100b0}, {0x17963a0, 0x1bc6208})
        /Users/yanis/go/pkg/mod/github.com/gin-gonic/[email protected]/context.go:646 +0x47
github.com/gin-gonic/gin.(*Context).BindJSON(...)
        /Users/yanis/go/pkg/mod/github.com/gin-gonic/[email protected]/context.go:609
main.main.func1(0x40)
        /Users/yanis/Qonto/test/main.go:18 +0x4c
github.com/gin-gonic/gin.(*Context).Next(...)
        /Users/yanis/go/pkg/mod/github.com/gin-gonic/[email protected]/context.go:165
github.com/gin-gonic/gin.(*Engine).handleHTTPRequest(0xc000464680, 0xc000438100)
        /Users/yanis/go/pkg/mod/github.com/gin-gonic/[email protected]/gin.go:489 +0x63e
github.com/gin-gonic/gin.(*Engine).ServeHTTP(0xc000464680, {0x17970d0, 0xc0000cb880}, 0xc000438000)
        /Users/yanis/go/pkg/mod/github.com/gin-gonic/[email protected]/gin.go:445 +0x1c5
net/http.serverHandler.ServeHTTP({0x1795b58}, {0x17970d0, 0xc0000cb880}, 0xc000438000)
        /usr/local/opt/go/libexec/src/net/http/server.go:2879 +0x43b
net/http.(*conn).serve(0xc0001faa00, {0x179bb40, 0xc0003c1a40})
        /usr/local/opt/go/libexec/src/net/http/server.go:1930 +0xb08
created by net/http.(*Server).Serve
        /usr/local/opt/go/libexec/src/net/http/server.go:3034 +0x4e8

Environment

  • go version: 1.17.5
  • gin version (or commit ref): Working version 1.6.3, not working since 1.7.0
  • operating system: MacOS Big Sur

More:

Looks like there's an issue with https://github.com/gin-gonic/gin/commit/4bfae4c8c8f7764cc587022ba6d9d2fd18e6c47d, and especially the ValidateStruct function. From my understanding, the nil check evaluates to false when obj is a nil interface.

I quickly tried to check for nil interfaces which seem to solve the issue. I'm not sure about side effects though:

if (reflect.ValueOf(c).Kind() == reflect.Ptr && reflect.ValueOf(c).IsNil()) {
return nil
}

yns01 avatar Jan 25 '22 22:01 yns01

Use BindJSON will panic, you should use ShouldBindJSON instead.

Bisstocuz avatar Jan 26 '22 12:01 Bisstocuz

Thanks @Bisstocuz, but the result is the same, it panics.

yns01 avatar Jan 26 '22 13:01 yns01

var ex *Example

This is a pointer, it has no memory allocation. Use var ex Example instead.

Bisstocuz avatar Jan 26 '22 13:01 Bisstocuz

I know, however this used to work prior to 1.7.0. I don't think the code should panic.

yns01 avatar Jan 26 '22 14:01 yns01

Also, this does not seem to cause any issues to the json decoder. The panics occurs after the decoding, in the struct validation fn.

		var ex *Example

		dec := json.NewDecoder(ctx.Request.Body)
		err := dec.Decode(&ex)
		fmt.Println(err) <nil>
		fmt.Println(ex) &{hello} OR nil if I pass "null" as a body

yns01 avatar Jan 26 '22 14:01 yns01

Also, this does not seem to cause any issues to the json decoder. The panics occurs after the decoding, in the struct validation fn.

		var ex *Example

		dec := json.NewDecoder(ctx.Request.Body)
		err := dec.Decode(&ex)
		fmt.Println(err) <nil>
		fmt.Println(ex) &{hello} OR nil if I pass "null" as a body
var ex Example

err := ctx.BindJSON(&ex)

I also encountered this problem. In these two days, it can be modified to solve it.

wickcy avatar Jan 27 '22 04:01 wickcy

I tried this. It worked.

ex := new(Example)
err := ctx.BindJSON(ex)

sdlyingyong avatar Nov 02 '22 07:11 sdlyingyong