我居然发现了golang的gin里NegotiateFormat的一个bug

最新推荐文章于 2024-06-13 20:50:09 发布

夏日白云

最新推荐文章于 2024-06-13 20:50:09 发布

阅读量287

点赞数

分类专栏： Golang 文章标签： golang 开发语言后端 gin web

本文链接：https://blog.csdn.net/lin_strong/article/details/122038017

版权

Golang 专栏收录该内容

17 篇文章 1 订阅

订阅专栏

这两天服务器运行了万年的老代码突然panic了。感到异常的诧异。

经过追查，更诧异的是发现居然是gin的bug。原代码是这样的：

func render(g *gin.Context, resp。。。。) {
……
	// content-type only supports application/json and application/x-protobuf
	pb := NegotiateFormat(g, binding.MIMEPROTOBUF)  // 就是这里报出的panic
	if pb == binding.MIMEPROTOBUF {
		g.ProtoBuf(http.StatusOK, resp)
		g.Abort()
		return
	}
	//default
	g.AbortWithStatusJSON(http.StatusOK, resp)
}

由于panic的信息不好查看实际的Context参数。于是往内深挖代码：

// NegotiateFormat returns an acceptable Accept format.
func (c *Context) NegotiateFormat(offered ...string) string {
	assert1(len(offered) > 0, "you must provide at least one offer")

	if c.Accepted == nil {
		c.Accepted = parseAccept(c.requestHeader("Accept"))
	}
	if len(c.Accepted) == 0 {
		return offered[0]
	}
	for _, accepted := range c.Accepted {
		for _, offer := range offered {
			// According to RFC 2616 and RFC 2396, non-ASCII characters are not allowed in headers,
			// therefore we can just iterate over the string without casting it into []rune
			i := 0
			for ; i < len(accepted); i++ {
				if accepted[i] == '*' || offer[i] == '*' {
					return offer
				}
				if accepted[i] != offer[i] {
					break
				}
			}
			if i == len(accepted) {
				return offer
			}
		}
	}
	return ""
}

看出端倪了么？

这是一个C语言里头就经常出现经典bug。在

			i := 0
			for ; i < len(accepted); i++ {
				if accepted[i] == '*' || offer[i] == '*' {
					return offer
				}
				if accepted[i] != offer[i] {
					break
				}
			}

里头。其实几乎就是一个判断两个字符串匹配的函数，这个for循环的i长度上限判断只针对accepted，但是却没有考虑offer的长度。这就导致了，如果offer的所有字符和accepted的前面字符完全匹配，但是accepted更长的时候，offer[i]会越界。

我们来写个单测复现一下：

func TestNegotiateFormat(t *testing.T) {
	w := httptest.NewRecorder()
	req, _ := http.NewRequest("POST", "/test", nil)
	req.Header["Accept"] = []string{binding.MIMEPROTOBUF, binding.MIMEJSON}
	r := gin.Default()

	// data api
	r.POST("/test", func(g *gin.Context) {
		format := g.NegotiateFormat(binding.MIMEPROTOBUF)
		fmt.Println(format)
		if format == binding.MIMEPROTOBUF {
			g.AbortWithStatus(201)
			return
		}
		//default
		g.AbortWithStatus(200)
	})

	r.ServeHTTP(w, req)

	assert.Equal(t, 201, w.Code)
}

输出为：

$ go test -v
=== RUN   TestNegotiateFormat
[GIN-debug] [WARNING] Creating an Engine instance with the Logger and Recovery middleware already attached.

[GIN-debug] [WARNING] Running in "debug" mode. Switch to "release" mode in production.
 - using env:   export GIN_MODE=release
 - using code:  gin.SetMode(gin.ReleaseMode)

[GIN-debug] POST   /test                     --> ……/handler/test.TestNegotiateFormat.func1 (3 handlers)


2021/12/20 11:44:15 [Recovery] 2021/12/20 - 11:44:15 panic recovered:
POST /test HTTP/1.1
Accept: application/x-protobuf1


runtime error: index out of range [22] with length 22
/usr/local/go/src/runtime/panic.go:88 (0x1036144)
        goPanicIndex: panic(boundsError{x: int64(x), signed: true, y: y, code: boundsIndex})
/Users/admin/go/pkg/mod/github.com/gin-gonic/gin@v1.7.1/context.go:1124 (0x1587d1d)
        (*Context).NegotiateFormat: if accepted[i] == '*' || offer[i] == '*' {
/Users/admin/go/src/……/handler/test/base_test.go:21 (0x15e516a)
        TestNegotiateFormat.func1: format := g.NegotiateFormat(binding.MIMEPROTOBUF)
/Users/admin/go/pkg/mod/github.com/gin-gonic/gin@v1.7.1/context.go:165 (0x15822da)
        (*Context).Next: c.handlers[c.index](c)
/Users/admin/go/pkg/mod/github.com/gin-gonic/gin@v1.7.1/recovery.go:99 (0x15977c8)
        CustomRecoveryWithWriter.func1: c.Next()
/Users/admin/go/pkg/mod/github.com/gin-gonic/gin@v1.7.1/context.go:165 (0x15822da)
        (*Context).Next: c.handlers[c.index](c)
/Users/admin/go/pkg/mod/github.com/gin-gonic/gin@v1.7.1/logger.go:241 (0x1596904)
        LoggerWithConfig.func1: c.Next()
/Users/admin/go/pkg/mod/github.com/gin-gonic/gin@v1.7.1/context.go:165 (0x15822da)
        (*Context).Next: c.handlers[c.index](c)
/Users/admin/go/pkg/mod/github.com/gin-gonic/gin@v1.7.1/gin.go:489 (0x158d277)
        (*Engine).handleHTTPRequest: c.Next()
/Users/admin/go/pkg/mod/github.com/gin-gonic/gin@v1.7.1/gin.go:445 (0x158c9db)
        (*Engine).ServeHTTP: engine.handleHTTPRequest(c)
/Users/admin/go/src/……/handler/test/base_test.go:31 (0x15e5032)
        TestNegotiateFormat: r.ServeHTTP(w, req)
/usr/local/go/src/testing/testing.go:1123 (0x111a2ce)
        tRunner: fn(t)
/usr/local/go/src/runtime/asm_amd64.s:1374 (0x1070f00)
        goexit: BYTE    $0x90   // NOP

[GIN] 2021/12/20 - 11:44:15 | 500 |    1.014915ms |                 | POST     "/test"
    base_test.go:33: 
                Error Trace:    base_test.go:33
                Error:          Not equal: 
                                expected: 201
                                actual  : 500
                Test:           TestNegotiateFormat
--- FAIL: TestNegotiateFormat (0.00s)
FAIL
exit status 1
FAIL    ……/handler/test  1.865s

好吧，只要Accept这个header中的字符串的前半部分和offered的一样，然后随便长一个字符就panic了。虽然现实中这个的概率满低的。所以好久也没panic过。

升了下版本，发现这个bug在1.7.7版本中仍然没有得到修复。那就只好自己动手风衣猪食了。

其实修复起来也很简单。i增加下对offer长度的校验就行，其他不好搞就拷贝下：

func NegotiateFormat(c *gin.Context, offered ...string) string {
……
	// {&& i < len(offer)}   fix panic
	for ; i < len(accepted) && i < len(offer); i++ {
……
}

然后再来波单测：

func TestNegotiateFormat(t *testing.T) {
	w := httptest.NewRecorder()
	req, _ := http.NewRequest("POST", "/test", nil)
	req.Header["Accept"] = []string{binding.MIMEPROTOBUF + "1"}
	r := gin.Default()

	// data api
	r.POST("/test", func(g *gin.Context) {
		format := NegotiateFormat(g, binding.MIMEPROTOBUF)
		fmt.Println(format)
		if format == binding.MIMEPROTOBUF {
			g.AbortWithStatus(201)
			return
		}
		//default
		g.AbortWithStatus(200)
	})

	r.ServeHTTP(w, req)

	assert.Equal(t, 201, w.Code)
}

$ go test -v
=== RUN   TestNegotiateFormat
[GIN-debug] [WARNING] Creating an Engine instance with the Logger and Recovery middleware already attached.

[GIN-debug] [WARNING] Running in "debug" mode. Switch to "release" mode in production.
 - using env:   export GIN_MODE=release
 - using code:  gin.SetMode(gin.ReleaseMode)

[GIN-debug] POST   /test                     --> code.byted.org/……/handler/test.TestNegotiateFormat.func1 (3 handlers)

[GIN] 2021/12/20 - 11:59:28 | 200 |      11.188µs |                 | POST     "/test"
    base_test.go:34: 
                Error Trace:    base_test.go:34
                Error:          Not equal: 
                                expected: 201
                                actual  : 200
                Test:           TestNegotiateFormat
--- FAIL: TestNegotiateFormat (0.00s)
FAIL
exit status 1
FAIL    code.byted.org/……/handler/test  1.767s

搞定，没有panic。下班