fix: in fixpath case, wildcard node should be the last child. #4394

liruohrh · 2025-10-12T09:47:46Z

define {prefix}/{param path like :id} and {prefix}/, and than access no defined path like {prefix}/... will panic for panic("invalid node type").
for it didn't obtain the wildcard child node correctly.

here is a example case. after start server, access http://localhost:8080/prefix/a/b/c , will cause panic.

package main

import (
	"errors"
	"net/http"

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

func main() {
	r := gin.Default()
	r.RedirectFixedPath = true
	r.GET("/prefix/:id", func(c *gin.Context) {
		c.JSON(200, "id")
	})
	r.GET("/prefix/xxx", func(c *gin.Context) {
		c.JSON(200, "xxx")
	})
	if err := r.Run(":8080"); err != nil && errors.Is(err, http.ErrServerClosed) {
		panic(err)
	}
}

Copilot

Pull Request Overview

Fixes a panic in the path routing logic when accessing undefined paths with wildcards enabled. The issue occurs when the tree walker incorrectly selects the first child node instead of the wildcard child node, which should be the last child in the children array.

Changed child node selection logic to use the last child instead of the first child
Prevents "invalid node type" panic when accessing undefined paths like /prefix/a/b/c when routes like /prefix/:id and /prefix/ are defined

_{Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.}

tree.go

appleboy · 2025-10-13T00:10:36Z

Please add unit testing for the cause.

codecov · 2025-10-13T00:11:42Z

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 98.81%. Comparing base (3dc1cd6) to head (deb81c1).
⚠️ Report is 194 commits behind head on master.

Additional details and impacted files

@@            Coverage Diff             @@
##           master    #4394      +/-   ##
==========================================
- Coverage   99.21%   98.81%   -0.40%     
==========================================
  Files          42       44       +2     
  Lines        3182     3467     +285     
==========================================
+ Hits         3157     3426     +269     
- Misses         17       30      +13     
- Partials        8       11       +3

Flag	Coverage Δ
	`?`
--ldflags="-checklinkname=0" -tags sonic	`98.80% <100.00%> (?)`
-tags go_json	`98.75% <100.00%> (?)`
-tags nomsgpack	`98.80% <100.00%> (?)`
go-1.18	`?`
go-1.19	`?`
go-1.20	`?`
go-1.21	`?`
go-1.24	`98.81% <100.00%> (?)`
go-1.25	`98.81% <100.00%> (?)`
macos-latest	`98.81% <100.00%> (-0.40%)`	⬇️
ubuntu-latest	`98.81% <100.00%> (-0.40%)`	⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

liruohrh · 2025-10-13T12:41:17Z

Please add unit testing for the cause.

Sorry, I thought such a small point didn't need to add tests. I added a test situation on the basis of the original test function. This test situation will cause panic before the error is fixed.

liruohrh changed the title ~~fix: wildcard node should be the last child.~~ fix: in fixpath case, wildcard node should be the last child. Oct 12, 2025

appleboy requested a review from Copilot October 13, 2025 00:10

Copilot AI reviewed Oct 13, 2025

View reviewed changes

tree.go Show resolved Hide resolved

fix: wildcard node should be the last child.

deb81c1

liruohrh force-pushed the fix/fixpath_wildcardnode branch from a40e3ca to deb81c1 Compare October 13, 2025 12:36

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

fix: in fixpath case, wildcard node should be the last child. #4394

fix: in fixpath case, wildcard node should be the last child. #4394

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

fix: in fixpath case, wildcard node should be the last child. #4394

Are you sure you want to change the base?

fix: in fixpath case, wildcard node should be the last child. #4394

Conversation

Uh oh!

Choose a reason for hiding this comment

Pull Request Overview

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Codecov Report

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants