Description

Setting HandleMethodNotAllowed on the engine and configuring the routes in a intricate way, gin panics when a certain request is sent. See details below on how to reproduce.

How to reproduce

package main

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

func main() {
    r := gin.Default()
    r.HandleMethodNotAllowed = true

    r.GET("/base/metrics", handler)
    r.GET("/base/v1/:id/devices", handler)
    r.GET("/base/v1/user/:id/groups", handler)
    r.GET("/base/v1/organizations/:id", handler)
    r.DELETE("/base/v1/organizations/:id", handler)
    r.Run() 
}

Now, make a GET on "localhost:8080/base/v1/user/groups", in which case gin panics.

I must admit that the above setup is a bit off, with how ".../:id/devices" and ".../user/:id/groups" is overlapping somewhat, but I still do not expect a panic. If the setup of the routes truly is wrong, then I would expect a panic to happen earlier, either at r.GET() or r.Run().

I've also tried to delete some of the routes to minimize the example, but it seems that all lines are needed for this to fail, which further emphasize that this is a corner case.

Expectations

$ curl http://localhost:8080/base/v1/user/groups
404 page not found

Actual result

$ curl -i http://localhost:8080/base/v1/user/groups
curl: (52) Empty reply from server

With the following log message from the gin app:

2023/01/03 10:16:50 http: panic serving 127.0.0.1:56181: runtime error: index out of range [0] with length 0
goroutine 34 [running]:
net/http.(*conn).serve.func1()
        /usr/local/go/src/net/http/server.go:1850 +0xb0
panic({0x1053aefe0, 0x140000260f0})
        /usr/local/go/src/runtime/panic.go:890 +0x258
github.com/gin-gonic/gin.(*node).getValue(0x1400012d938?, {0x14000520244?, 0x1400012d9d8?}, 0x0, 0x1400000e030, 0x0)
        /Users/kristiansvalland/go/pkg/mod/github.com/gin-gonic/gin@v1.8.2/tree.go:637 +0x378
github.com/gin-gonic/gin.(*Engine).handleHTTPRequest(0x1400054c4e0, 0x140000ae000)
        /Users/kristiansvalland/go/pkg/mod/github.com/gin-gonic/gin@v1.8.2/gin.go:637 +0x474
github.com/gin-gonic/gin.(*Engine).ServeHTTP(0x1400054c4e0, {0x1053d9df8?, 0x140000a8000}, 0x14000512100)
        /Users/kristiansvalland/go/pkg/mod/github.com/gin-gonic/gin@v1.8.2/gin.go:572 +0x1d4
net/http.serverHandler.ServeHTTP({0x14000502e70?}, {0x1053d9df8, 0x140000a8000}, 0x14000512100)
        /usr/local/go/src/net/http/server.go:2947 +0x2c4
net/http.(*conn).serve(0x1400051ab40, {0x1053da5e0, 0x14000502d80})
        /usr/local/go/src/net/http/server.go:1991 +0x560
created by net/http.(*Server).Serve
        /usr/local/go/src/net/http/server.go:3102 +0x444

Environment

  • go version: go1.19.4
  • gin version (or commit ref): v1.8.2 and master
  • operating system: MacOS

Suggested fix

As hinted by in the backtrace, the panic occurs in tree.go#L637, specifically here (with commentary and suggested fix commented):

if !value.tsr && path != "/" {
    for l := len(*skippedNodes); l > 0; {
        skippedNode := (*skippedNodes)[l-1] // <-- Panic occurs here in the second round of the for loop
        *skippedNodes = (*skippedNodes)[:l-1]
        l-- // suggested fix
        if strings.HasSuffix(skippedNode.path, path) {
            path = skippedNode.path
            n = skippedNode.node
            if value.params != nil {
                *value.params = (*value.params)[:skippedNode.paramsCount]
            }
            globalParamsCount = skippedNode.paramsCount
            continue walk
        }
    }
}

Note: The above logic happens three places in tree.go, so they should probably also be fixed at the same time.

Capacity for working on this

I have already created a test and fix for this issue, which I will create a PR for promptly.