gin-gonic / gin

Gin is a HTTP web framework written in Go (Golang). It features a Martini-like API with much better performance -- up to 40 times faster. If you need smashing performance, get yourself some Gin.
https://gin-gonic.com/
MIT License
79.11k stars 8.03k forks source link

parse `.1` in query param form to decimal.Decimal #4089

Open zenyusy opened 2 weeks ago

zenyusy commented 2 weeks ago

Description

Gin receives HTTP GET call like 127.0.0.1:8000/foo?a=.1, where a is expected to be decimal number. The handling part has defined a struct with a decimal.Decimal field to receive this a. (URL being "a=0.1" vs "a=.1", the result will be different.)

the actual part to look into is BindQuery, the calling chain is a bit long, i think the important functions are

  1. https://github.com/gin-gonic/gin/blob/c8a3adc65703d8958265c07689662e54f037038c/binding/query.go (Parse URL to {"a":[".1"]} map)

  2. https://github.com/gin-gonic/gin/blob/c8a3adc65703d8958265c07689662e54f037038c/binding/form_mapping.go#L225 (Try to parse ".1" and assign to the destination field)

In the setByForm fn, it goes to default branch, and attempt of trySetCustom is not effective, so finally go to setWithProperType:

the receiving/destination variable is decimal.Decimal, which is a struct, so go to case reflect.Struct in setWithProperType, then json.Unmarshal

THE BAD THING is .1 is not a valid JSONstring. (0.1 or ".1" are valid JSONstring)

I note inside case reflect.Struct, 2 struct types are specially handled. so regarding my use case, is it a typical one where trySetCustom is supposed to be used? Or, passing query a=.1 is bad practice?

How to reproduce

Gin code:

package main

import (
    "net/http"

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

func main() {
    router := gin.Default()
    router.GET("/foo", func(c *gin.Context) {
        var req struct {
            A decimal.Decimal `form:"a"`
        }
        if err := c.BindQuery(&req); err != nil {
            c.JSON(http.StatusBadRequest, gin.H{"error": err.Error()})
            return
        }
    })
    router.Run(":8000")
}

127.0.0.1:8000/foo?a=.1

Expectations

no error

Actual result

error

Environment

ksw2000 commented 1 week ago

The setByForm function calls trySetCustom to check if the binding fields implement the BindUnmarshaler interface. However, decimal.Decimal does not implement this interface by default.

Fortunately, you can still implement the interface BindUnmarshaler on your own.

// BindUnmarshaler is the interface used to wrap the UnmarshalParam method.
type BindUnmarshaler interface {
    // UnmarshalParam decodes and assigns a value from an form or query param.
    UnmarshalParam(param string) error
}

Example:

package main

import (
    "fmt"
    "net/http"

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

type myDecimal decimal.Decimal

func (d *myDecimal) UnmarshalParam(val string) error {
    return (*decimal.Decimal)(d).UnmarshalJSON([]byte(val))
}

func main() {
    router := gin.Default()
    router.GET("/foo", func(c *gin.Context) {
        var req struct {
            A myDecimal `form:"a"`
        }
        if err := c.BindQuery(&req); err != nil {
            c.JSON(http.StatusBadRequest, gin.H{"error": err.Error()})
            return
        } else {
            fmt.Println((decimal.Decimal)(req.A))
        }
    })
    router.Run(":8000")
}
[GIN-debug] Listening and serving HTTP on :8000
0.1
[GIN] 2024/11/11 - 14:52:00 | 200 |     156.327µs |       127.0.0.1 | GET      "/foo?a=.1"