gofiber / fiber

⚡️ Express inspired web framework written in Go
https://gofiber.io
MIT License
33.54k stars 1.65k forks source link

🐛 [Bug]: client.SetValWithStruct set zero value if the field is slice #3167

Open ksw2000 opened 4 hours ago

ksw2000 commented 4 hours ago

Bug Description

The function SetValWithStruct() sets values using structs. It skips zero values, such as 0 and false. However, if the fields are slices (excluding bool slices), all the values in the slices will be set.

How to Reproduce

To reproduce the bugs, we can simply add a test in client/request_test.go:

func TestSetValWithStructForSlices(t *testing.T) {
    p := &QueryParam{
        Args: fasthttp.AcquireArgs(),
    }
    SetValWithStruct(p, "param", struct {
        TZeroInt   int
        TInt       int
        TIntSlice  []int
        TBoolSlice []bool
    }{
        TZeroInt:   0,
        TInt:       1,
        TIntSlice:  []int{0, 1, 2},
        TBoolSlice: []bool{true, false},
    })

    fmt.Println(string(p.Peek("TZeroInt")))
    fmt.Println(string(p.Peek("TInt")))
    for _, v := range p.PeekMulti("TIntSlice") {
        fmt.Print(string(v), ", ")
    }
    fmt.Println()
    for _, v := range p.PeekMulti("TBoolSlice") {
        fmt.Print(string(v), ", ")
    }
    fmt.Println()
}
cd client
go test --run TestSetValWithStructForSlices -v

result:


1
0, 1, 2, 
true,

Expected Behavior


1
1, 2, 
true,

The SetValWithStruct() should skip 0 in the int slice too.

Fiber Version

latest

Code Snippet (optional)

No response

Checklist:

welcome[bot] commented 4 hours ago

Thanks for opening your first issue here! 🎉 Be sure to follow the issue template! If you need help or want to chat with us, join us on Discord https://gofiber.io/discord

JIeJaitt commented 1 hour ago

@ksw2000 Hi, are you sure SetValWithStruct() should also skip the 0's in the int slice, I don't think [1,2] and [0,1,2] are the same slice.

ksw2000 commented 46 minutes ago

@JIeJaitt I'm not entirely sure, since there are two different logics for processing int slice and bool slice in the SetValWithStruct() function. In my opinion, there are absolutely bugs.

If the 0 in [0, 1, 2] should be stored. I think false in [true, false] should be stored too. That is to say, the expected behavior is changed to:


1
0, 1, 2, 
true, false,

SetValWithStruct does not consider zero values when processing slices. However, since it only stores true when processing bool slices, zero values are considered in bool slices.

https://github.com/gofiber/fiber/blob/7b3a36f22fc1166ceb9cb78cf69b3a2f95d077da/client/request.go#L948-L951

JIeJaitt commented 31 minutes ago

@ksw2000 I think what you said makes sense. There is indeed a problem here, which creates ambiguity for users. I think if possible, two functions are provided here. One is the normal SetValWithStruct() function that will store 0 and false, and the other is the SetValWithStructSpecial() function that will skip 0 and false values.