gin icon indicating copy to clipboard operation
gin copied to clipboard

[Discussion] abortIndex is too little?

Open wangyuheng opened this issue 1 year ago • 1 comments

How to reproduce

package main

import (
	"fmt"
	"log"
	"math"
	"net/http"

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

func main() {
	r := gin.Default()
	r.Use(customMws()...)
	r.GET("/ping", func(c *gin.Context) {
		c.JSON(http.StatusOK, gin.H{
			"message": "pong",
		})
	})
	r.Run() // listen and serve on 0.0.0.0:8080 (for windows "localhost:8080")
}

func customMws() []gin.HandlerFunc {
	l := math.MaxInt8>>2 - 10
	res := make([]gin.HandlerFunc, 0)
	for i := 0; i < l; i++ {
		j := i
		res = append(res, func(c *gin.Context) {
			log.Println(fmt.Sprintf("req wrapper mw:%d", j))
			c.Next()
			log.Println(fmt.Sprintf("resp wrapper mw:%d aborted: %t", j, c.IsAborted()))
		})
		res = append(res, func(c *gin.Context) {
			log.Println(fmt.Sprintf("req mw:%d", j))
			c.Next()
			log.Println(fmt.Sprintf("resp mw:%d aborted: %t", j, c.IsAborted()))
		})
	}
	return res
}

See the source

func (c *Context) Next() {
	c.index++
	for c.index < int8(len(c.handlers)) {
		c.handlers[c.index](c)
		c.index++
	}
}

and

func (group *RouterGroup) combineHandlers(handlers HandlersChain) HandlersChain {
	finalSize := len(group.Handlers) + len(handlers)
	assert1(finalSize < int(abortIndex), "too many handlers")
	mergedHandlers := make(HandlersChain, finalSize)
	copy(mergedHandlers, group.Handlers)
	copy(mergedHandlers[len(group.Handlers):], handlers)
	return mergedHandlers
}

Expectations

$ curl localhost:8080/ping

c.IsAborted() is always false because i do not call Abort()

Actual result

resp mw:13 aborted: false
resp wrapper mw:13 aborted: false
resp mw:12 aborted: false
resp wrapper mw:12 aborted: false
resp mw:11 aborted: true
resp wrapper mw:11 aborted: true
resp mw:10 aborted: true
resp wrapper mw:10 aborted: true

Discussion

Maybe abortIndex is too little? The Boundary Index should not touch by index++. Or when handles over 63 should make some impact I do not know? Thank u

Environment

  • go version: 1.17.2
  • gin version (or commit ref): 1.8.1
  • operating system: MacOS

wangyuheng avatar Aug 22 '22 09:08 wangyuheng

func (c *Context) Next() {
	c.index++
	for c.index < int8(len(c.handlers)) {
		c.handlers[c.index](c)
		c.index++
	}
}

Mainly here to make index increase. When calling the c.Next method to exec c.index++, when calling the c.Next method again in the middleware, c.index is increased again.

Then when the middleware exits it is called again

c.handlers[c.index](c)
// exit and call again
c.index++

If c.IsAborted() is not checked after c.Next(), it will not affect the execution of the registration middleware.

ywanbing avatar Sep 13 '22 07:09 ywanbing