Skip to content

[Bug] Mixed params and exact paths can cause a 404  #2682

Closed
@rw-access

Description

@rw-access
Router1 "/get/test/abc/"
Router2 "/get/:param/abc/"

I tests these two routers with some requests:
/get/test/abc/ -> Router1
/get/xyz/abc/ -> Router2
/get/tt/abc/ -> 404(Expect Router2)
So, if there are some param starts with the prefix 't' in this example, it will not get the right router.

Originally posted by @Tevic in #2663 (comment)

Activity

rw-access

rw-access commented on Apr 9, 2021

@rw-access
ContributorAuthor

I don't have the ability to self-assign this, but feel free to assign me.
I've got an idea for a fix.

g1eny0ung

g1eny0ung commented on Apr 22, 2021

@g1eny0ung
Contributor

@rw-access I'm also facing this problem, here are the example routes I defined:

r.GET("/", xxx)
r.GET("/:foo", xxx)
r.Group("/api", xxx)
r.GET("/static/*any", xxx)
r.GET("/favicon.ico", xxx)

For the realworld project needed, I try to solve this problem. After debugging, I found that it's related to https://github.com/gin-gonic/gin/pull/2663/files#diff-050830ac4334170e8d335e1b95519052a3461c5b57a4ca38a0531e35a8f3d388R410:

// Try all the non-wildcard children first by matching the indices
idxc := path[0]
for i, c := range []byte(n.indices) {
	if c == idxc {
		n = n.children[i]
		continue walk
	}
}

Since the node indices will build as asf to make a fast deep search. When accessing the route like /archives, the above code will jump into /api route group to find the matching route, then it failed (return 404).

Before #2663, the above code wrapped with an if condition if !n.wildChild so it can be executed normally without any other side effect.

I wanted to propose a PR for repair, but I think it’s best to discuss how to do it. Seems to solve this problem, I think it's needed to add an extra condition like if children have a :param node after c == idxc?Here is an example of linking the above code:

// Try all the non-wildcard children first by matching the indices
idxc := path[0]
for i, c := range []byte(n.indices) {
	if c == idxc {
		childWillWalkTo := n.children[i]

		if strings.Split(childWillWalkTo.path, "/")[0] != strings.Split(path, "/")[0] && strings.HasPrefix(n.children[len(n.children)-1].path, ":") {
			// "api" != "archives" && :foo has :
			continue
		} else {
			n = childWillWalkTo
			continue walk
		}
	}
}

I’m not sure if there are other things to pay attention to. Hope to get your feedback.

appleboy

appleboy commented on Apr 28, 2021

@appleboy
Member

@rw-access Can you take a look at the PR #2706?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

Labels

No labels
No labels

Type

No type

Projects

No projects

Milestone

No milestone

Relationships

None yet

    Development

    Participants

    @appleboy@g1eny0ung@rw-access

    Issue actions

      [Bug] Mixed params and exact paths can cause a 404 · Issue #2682 · gin-gonic/gin