ShouldBindQuery panic if query string is empty

[dfarr]

Description

I have a simple web server with a search api that accepts three string query parameters: a, b, and c. I can use ShouldBindQuery to bind these parameters to a struct, but if I call the endpoint without any query in the path the handler panics (and recovers).

How to reproduce

package main

import (
	"net/http"

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

type Params struct {
	A string `form:"a"`
	B string `form:"b"`
	C string `form:"c"`
}

func main() {
	g := gin.Default()

	g.GET("/search", func(c *gin.Context) {
		var params *Params
		if err := c.ShouldBindQuery(&params); err != nil {
			c.JSON(http.StatusBadRequest, gin.H{
				"error": err.Error(),
			})
			return
		}

		c.JSON(200, params)
	})

	g.Run(":9000")
}

Expectations

$ curl http://localhost:9000/search?a=a&b=b&c=c
{"A":"a","B":"b","C":"c"}

$ curl http://localhost:9000/search
{"A":"","B":"","C":""}

Actual result

$ curl http://localhost:9000/search?a=a&b=b&c=c
{"A":"a","B":"b","C":"c"}

$ curl http://localhost:9000/search
(handler panics and recovers)

2023/09/21 10:13:53 [Recovery] 2023/09/21 - 10:13:53 panic recovered:
GET /search HTTP/1.1
Host: localhost:9000
Accept: */*
User-Agent: curl/8.1.2


reflect: call of reflect.Value.Interface on zero Value
/usr/local/go/src/reflect/value.go:1495 (0x102812f3b)
	valueInterface: panic(&ValueError{"reflect.Value.Interface", Invalid})
/usr/local/go/src/reflect/value.go:1490 (0x102a4b357)
	Value.Interface: return valueInterface(v, true)
/Users/dfarr/go/pkg/mod/github.com/gin-gonic/[email protected]/binding/default_validator.go:57 (0x102a4b340)
	(*defaultValidator).ValidateStruct: return v.ValidateStruct(value.Elem().Interface())
/Users/dfarr/go/pkg/mod/github.com/gin-gonic/[email protected]/binding/default_validator.go:57 (0x102a4b367)
	(*defaultValidator).ValidateStruct: return v.ValidateStruct(value.Elem().Interface())
/Users/dfarr/go/pkg/mod/github.com/gin-gonic/[email protected]/binding/binding.go:120 (0x102a4dd97)
	validate: return Validator.ValidateStruct(obj)
/Users/dfarr/go/pkg/mod/github.com/gin-gonic/[email protected]/binding/query.go:20 (0x102a4dd7c)
	queryBinding.Bind: return validate(obj)
/Users/dfarr/go/pkg/mod/github.com/gin-gonic/[email protected]/context.go:741 (0x102a7f9ff)
	(*Context).ShouldBindWith: return b.Bind(c.Request, obj)
/Users/dfarr/go/pkg/mod/github.com/gin-gonic/[email protected]/context.go:711 (0x102a7f9e4)
	(*Context).ShouldBindQuery: return c.ShouldBindWith(obj, binding.Query)
/Users/dfarr/Desktop/gin-bug/main.go:20 (0x102a7fa00)
	main.func1: if err := c.ShouldBindQuery(&params); err != nil {
/Users/dfarr/go/pkg/mod/github.com/gin-gonic/[email protected]/context.go:174 (0x102a7a6ef)
	(*Context).Next: c.handlers[c.index](c)
/Users/dfarr/go/pkg/mod/github.com/gin-gonic/[email protected]/recovery.go:102 (0x102a7a6d4)
	CustomRecoveryWithWriter.func1: c.Next()
/Users/dfarr/go/pkg/mod/github.com/gin-gonic/[email protected]/context.go:174 (0x102a79a8f)
	(*Context).Next: c.handlers[c.index](c)
/Users/dfarr/go/pkg/mod/github.com/gin-gonic/[email protected]/logger.go:240 (0x102a79a60)
	LoggerWithConfig.func1: c.Next()
/Users/dfarr/go/pkg/mod/github.com/gin-gonic/[email protected]/context.go:174 (0x102a78bc3)
	(*Context).Next: c.handlers[c.index](c)
/Users/dfarr/go/pkg/mod/github.com/gin-gonic/[email protected]/gin.go:620 (0x102a788ec)
	(*Engine).handleHTTPRequest: c.Next()
/Users/dfarr/go/pkg/mod/github.com/gin-gonic/[email protected]/gin.go:576 (0x102a7863f)
	(*Engine).ServeHTTP: engine.handleHTTPRequest(c)
/usr/local/go/src/net/http/server.go:2938 (0x10294a40b)
	serverHandler.ServeHTTP: handler.ServeHTTP(rw, req)
/usr/local/go/src/net/http/server.go:2009 (0x102947587)
	(*conn).serve: serverHandler{c.server}.ServeHTTP(w, w.req)
/usr/local/go/src/runtime/asm_arm64.s:1197 (0x1027cc953)
	goexit: MOVD	R0, R0	// NOP

Environment

• go version: go version go1.21.0 darwin/arm64
• gin version (or commit ref): v1.9.1
• operating system: mac m2

[dfarr]

I can work around this by instantiating params, or by passing in a struct instead of a pointer to a struct.

// works
params := &Params{}
c.ShouldBindQuery(&params)

// works
var params Params
c.ShouldBindQuery(params)

// does not work
var params *Params
c.ShouldBindQuery(&params)

I primarily use ShouldBindJSON for my other endpoints and calls to this function work fine when passing a pointer to struct, even if the underlying struct is nil.

[VarusHsu]

It's a error about improper use of pointers. Modify var params *Params to var params Params.
In your previous code, the type of params is *Param, then you pass &params, so ShouldBindQuery accept a varaible type of **Param. So there is a null pointer exception.

[VarusHsu]

Make sure your ShouldBindQuery accepted type is *Param, So if you define params like this

var params *Params

Then you call should like this:

if err := c.ShouldBindQuery(params); err != nil {

It will work fine :)

[dfarr]

Thanks @VarusHsu, that works!

I think the confusion stems from the discrepancy between ShouldBindJSON and ShouldBindQuery, my bad.

// valid
var params *Params
c.ShouldBindJSON(&params)

// invalid
var params *Params
c.ShouldBindQuery(&params)

[dfarr]

Make sure your ShouldBindQuery accepted type is *Param, So if you define params like this

var params *Params

Then you call should like this:

if err := c.ShouldBindQuery(params); err != nil {

It will work fine :)

Actually this results in a panic as well