Can't bind list of struct pointer in query

[claneo]

Description

Sorry I'm not familiar with golang, so the description below maybe is not accurate.

Can't bind list of struct pointer in query

It seem like the pointer support was lost in this commit

0d50ce8?diff=split#diff-b0f2c06474cd98fa718e657b7037a516ada93c3de8f51fd795bfdf0ab3e2ade5L133-L138

I try to add these lines into setWithProperType,then the problem is resolved

	case reflect.Ptr:
		if !value.Elem().IsValid() {
			value.Set(reflect.New(value.Type().Elem()))
		}
		return setWithProperType(val, value.Elem(), field)

How to reproduce

package main

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

type Struct struct {
	Id int64 `json:"id"`
}

type Req struct {
	Items []*Struct `json:"items" form:"items"`
}

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

	r.GET("/api", func(c *gin.Context) {
		req := new(Req)
		err := c.Bind(req)
		if err != nil {
			c.JSON(500, err.Error())
			return
		}
		c.JSON(200, req)
	})

	_ = r.Run("127.0.0.1:8080")
}

Expectations

$ curl -i 'http://localhost:8080/api?items=\{"id":1\}&items=\{"id":2\}'
{"items":[{"id":1},{"id":2}]}

Actual result

$ curl -i 'http://localhost:8080/api?items=\{"id":1\}&items=\{"id":2\}'
"unknown type"

Environment

• go version: v1.20.4
• gin version (or commit ref): v1.9.1
• operating system: macos 13.4.1 arm64

[omkar-foss]

Hey @claneo, I was able to successfully reproduce this issue from your example code snippet above and was able to get the "unknown type" error. My findings as below for your reference.

Root Cause:

As you suggested, the Ptr case in setWithProperType() has been removed in #1749, which is why the default case here gets evaluated in setWithProperType() - leading to the unknown type error.

Solution:

In your example, change []*Struct to []Struct for Items, and then your query request will start working properly without any changes to Gin code. This is because []Struct will be detected as an Array type, which is supported and working well.

I've updated your example as below for reference with comment:

package main

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

type Struct struct {
	Id int64 `json:"id"`
}

type Req struct {
	Items []Struct `json:"items" form:"items"` // Use []Struct instead of []*Struct here
}

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

	r.GET("/api", func(c *gin.Context) {
		req := new(Req)
		err := c.Bind(req)
		if err != nil {
			c.JSON(500, err.Error())
			return
		}
		c.JSON(200, req)
	})

	_ = r.Run("127.0.0.1:8080")
}

I've tested the above code with Gin v1.9.1 and it works well. Hope it works for you too.

[claneo]

Hi, @omkar-foss , Thanks for your reply.

I know this problem can been fixed by change []*Struct to []Struct. But in my real code, these type is generated from api defination with cli tool. I'm not sure whether this is best practice or not.

So I think maybe gin can support this feature, to cover more use case.

[omkar-foss]

Hey @claneo, indeed, it might be a good idea to keep support for []*Struct to cover relevant use cases. Since there are Go developers who prefer the []*Struct over []Struct in certain cases for performance or other reasons. Although on the other hand, there are articles and developers who strongly suggest that using []*Struct is a bad practice (refer this article for instance).

If we plan on re-adding support for []*Struct, we may need get some background about why the support was removed in #1749, in order to avoid re-introducing any issues caused due to this feature in the past.

@vkd @thinkerou It would be great if you could help by giving us some background about why the Ptr support was removed in #1749. Thanks.

[vkd]

Thanks @omkar-foss for bringing this up.

As far as I remember, there wasn't a test case for such a scenario, and that case block was removed as a redundant.
I don't see any potential issues if we implement such a case to process a pointer.

[omkar-foss]

Hey @vkd, thanks a lot for your reply.

I've raised this PR which consists of the re-addition of pointer support (with tests), the code change is an adaptation of the change in this commit where the support was removed as well as suggested by @claneo above.