404 when using dynamic routing

[joewang1085]

• With issues:
  • Use the search tool before opening a new issue.
  • Please provide source code and commit sha if you found a bug.
  • Review existing issues and provide feedback or react to them.

Description

It will bring 404 problem when using dynamic routing(*,:) like below , by calling http://localhost:8080/all. It seemed can not match the path with prefix 'a', such as 'all'.

How to reproduce

package main

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

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

	r.GET("/aa/*xx", newHandler("/aa/*xx"))
	r.GET("/ab/*xx", newHandler("/ab/*xx"))
	r.GET("/:cc", newHandler("/:cc"))

	r.Run() // listen and serve on 0.0.0.0:8080 
}

func newHandler(t string) func(c *gin.Context) {
	return func(c *gin.Context) {
		c.JSON(200, gin.H{
			"message": t,
		})
	}
}

Expectations

$ curl http://localhost:8080/all
/:cc

Actual result

$ curl http://localhost:8080/all
404 page not found

Environment

• go version:
• gin version (or commit ref): 1.7.2
• operating system:

[appleboy]

duplicated of #2754 ?

cc @g1eny0ung @rw-access

[joewang1085]

I don't think it is really duplicated of #2754. It expects that getting '/:cc' when not matching others with using path like '/xx', but it will get 404 actually when we use a path with the common prefix 'a' of '/aa/*xx' and '/ab/*xx'. It seemed the '/:cc' can not work in such cases.

[rw-access]

Just attached a debugger locally to get a sense what's going on.
Looks like #2706 doesn't backtrack in all situations. This is the return that's hit before the backtrack is checked, which causes the 404. There is a valid skipped node to backtrack to, it's just not checked.

gin/tree.go

Lines 441 to 448 in 1d0f938

	// If there is no wildcard pattern, recommend a redirection
	if !n.wildChild {
	// Nothing found.
	// We can recommend to redirect to the same URL without a
	// trailing slash if a leaf exists for that path.
	value.tsr = (path == "/" && n.handlers != nil)
	return
	}

I think instead of a return in all of these places where there aren't matching handlers, we should conditionally do a goto backtrack and from there see if skipped should fallback. Definitely need some more tests, especially since this interacts with other features like TSR.

[g1eny0ung]

Just attached a debugger locally to get a sense what's going on.
Looks like #2706 doesn't backtrack in all situations. This is the return that's hit before the backtrack is checked, which causes the 404. There is a valid skipped node to backtrack to, it's just not checked.

gin/tree.go

Lines 441 to 448 in 1d0f938

	// If there is no wildcard pattern, recommend a redirection
	if !n.wildChild {
	// Nothing found.
	// We can recommend to redirect to the same URL without a
	// trailing slash if a leaf exists for that path.
	value.tsr = (path == "/" && n.handlers != nil)
	return
	}

I think instead of a return in all of these places where there aren't matching handlers, we should conditionally do a goto backtrack and from there see if skipped should fallback. Definitely need some more tests, especially since this interacts with other features like TSR.

Also found this problem, seems it's easy to resolve this problem by checking the skipped with the same way below:

gin/tree.go

Lines 567 to 572 in 1d0f938

	if path != "/" && skipped != nil && strings.HasSuffix(skipped.path, path) {
	path = skipped.path
	n = skipped.paramNode
	skipped = nil
	continue walk
	}

But as you said, we need more tests because of this change related to other features like TSR.

[qm012]

In fact, there are several situations

add router

r.GET("/:cc/cc", newHandler("/:cc/cc"))

call /all/cc -> 404
call /a/cc -> 404

prerequisite: a prefix router start

• level 1 router(all) length gt node.prefix,The following code will not execute

if path != "/" && skipped != nil && strings.HasSuffix(skipped.path, path) {
			path = skipped.path
			n = skipped.paramNode
			skipped = nil
			continue walk
		}

• When the route has only one a ,It will be equal to node. Indexes,The following code could not be matched

if value.handlers = n.handlers; value.handlers != nil {
				value.fullPath = n.fullPath
				return
			}