gin icon indicating copy to clipboard operation
gin copied to clipboard

binding:fix empty value error

Open guonaihong opened this issue 4 years ago • 6 comments

Here is the code that can report an error

package main

import (
	"fmt"
	"github.com/gin-gonic/gin"
	"io"
	"net/http"
	"os"
	"time"
)

type header struct {
	Duration   time.Duration `header:"duration"`
	CreateTime time.Time     `header:"createTime" time_format:"unix"`
}

func needFix1() {
	g := gin.Default()
	g.GET("/", func(c *gin.Context) {
		h := header{}
		err := c.ShouldBindHeader(&h)
		if err != nil {
			c.JSON(500, fmt.Sprintf("fail:%s\n", err))
			return
		}

		c.JSON(200, h)
	})

	g.Run(":8081")
}

func needFix2() {
	g := gin.Default()
	g.GET("/", func(c *gin.Context) {
		h := header{}
		err := c.ShouldBindHeader(&h)
		if err != nil {
			c.JSON(500, fmt.Sprintf("fail:%s\n", err))
			return
		}

		c.JSON(200, h)
	})

	g.Run(":8082")
}

func sendNeedFix1() {
	// send to needFix1
	sendBadData("http://127.0.0.1:8081", "duration")
}

func sendNeedFix2() {
	// send to needFix2
	sendBadData("http://127.0.0.1:8082", "createTime")
}

func sendBadData(url, key string) {
	req, err := http.NewRequest("GET", "http://127.0.0.1:8081", nil)
	if err != nil {
		fmt.Printf("err:%s\n", err)
		return
	}

	// Only the key and no value can cause an error
	req.Header.Add(key, "")
	rsp, err := http.DefaultClient.Do(req)
	if err != nil {
		return
	}
	io.Copy(os.Stdout, rsp.Body)
	rsp.Body.Close()
}

func main() {
	go needFix1()
	go needFix2()

	time.Sleep(time.Second / 1000 * 200) // 200ms
	sendNeedFix1()
	sendNeedFix2()
}

guonaihong avatar Dec 06 '19 07:12 guonaihong

Codecov Report

Merging #2169 into master will increase coverage by 0.00%. The diff coverage is 100.00%.

Impacted file tree graph

@@           Coverage Diff           @@
##           master    #2169   +/-   ##
=======================================
  Coverage   98.47%   98.47%           
=======================================
  Files          41       41           
  Lines        2298     2302    +4     
=======================================
+ Hits         2263     2267    +4     
  Misses         20       20           
  Partials       15       15           
Impacted Files Coverage Δ
binding/form_mapping.go 100.00% <100.00%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more Δ = absolute <relative> (impact), ø = not affected, ? = missing data Powered by Codecov. Last update 73ccfea...6655232. Read the comment docs.

codecov[bot] avatar Dec 06 '19 08:12 codecov[bot]

@vkd @thinkerou Any feedbacks?

appleboy avatar Jan 16 '20 06:01 appleboy

Vote for this pr dispute. @appleboy @vkd @thinkerou When dealing with int, float and other types, I recommend adding the string.TrimSpace function. The reason is that go's strconv.ParseUint and other functions will report an error when encountering data beginning with a space.

package main

import (
	"fmt"
	"strconv"
)

func main() {
	fmt.Println(strconv.ParseInt("  5", 10, 0))
	fmt.Println(strconv.ParseInt("5", 10, 0))
}

/*
0 strconv.ParseInt: parsing "  5": invalid syntax
5 <nil>

*/

strings.TrimSpace + 1

guonaihong avatar Jan 18 '20 06:01 guonaihong

By my perspective, adding strings.TrimSpace will make the gin bindings is not unified. It looks not usual for gin - because, as example, json binding doesn't do that kind of conversion. I think keeping the same behavior for all kind of bindings is better. @guonaihong thanks for your PR, but I vote for the not using the strings.TrimSpace in headers/form bindings.

strings.TrimSpace - 1

vkd avatar Jan 19 '20 13:01 vkd

@thinkerou move to 1.7 milestones?

appleboy avatar Mar 20 '20 15:03 appleboy

@thinkerou move to 1.7 milestones?

OK, thanks!

thinkerou avatar Mar 21 '20 01:03 thinkerou