Skip to content

fix(radix): fallback to parent wildcard when static child has no handler - #74

Open
AruneshDwivedi wants to merge 2 commits into
fasthttp:masterfrom
AruneshDwivedi:fix/wildcard-fallback
Open

AruneshDwivedi wants to merge 2 commits into
fasthttp:masterfrom
AruneshDwivedi:fix/wildcard-fallback

Conversation

@AruneshDwivedi

Copy link
Copy Markdown

Fixes #72

Problem

When a path exactly matches a static child node that has no direct handler and no wildcard, but its parent has a wildcard route, the lookup incorrectly returned nil (with TSR=true) instead of falling back to the parent's wildcard handler.

This caused routes like /{path:*} and /test/{path:*} to fail to match /test, since the test prefix matched the static child test which had only an auto-generated trailing-slash child (no handler of its own).

Root Cause

In radix/node.go getFromChild, when path == child.path for a static child, the switch checked child.tsr first and immediately returned nil, true without considering that the parent might have a wildcard handler that should take precedence.

Fix

After checking child.handler and child.wildcard, check if the parent node has a wildcard and return it. Only fall back to tsr or continue to other children if no parent wildcard exists.

Testing

Added regression test covering the reported scenario. All existing tests continue to pass.

When a path exactly matches a static child node that has no direct
handler and no wildcard, but its parent has a wildcard route, the
lookup incorrectly returned nil (with TSR=true) instead of falling
back to the parent's wildcard handler.

This caused routes like /{path:*} and /test/{path:*} to fail to match
/test, since the 'test' prefix matched the static child 'test' which
had only an auto-generated trailing-slash child (no handler of its own).

Fix: after checking child.handler and child.wildcard, check if the
parent node has a wildcard and return it. Only fall back to tsr or
continue if no parent wildcard exists. This preserves correct behavior
for /tes (wildcard matches), /test/foo (child wildcard matches),
/test (parent wildcard matches), and / (root wildcard matches).

Signed-off-by: Arunesh Dwivedi <arunesh.devops@gmail.com>
Comment thread radix/node.go Outdated
// that should take precedence over the auto-generated TSR.
if n.wildcard != nil {
if ctx != nil {
ctx.SetUserValue(n.wildcard.paramKey, gstrings.Copy(""))

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should this copy path instead of ""? With the routes from the test, /test now reaches the parent handler, but ctx.UserValue("path") is empty instead of "test".

This fails when added to radix/wildcard_fallback_test.go:

func TestParentWildcardValue(t *testing.T) {
	tree := New()
	handler := func(*fasthttp.RequestCtx) {}
	tree.Add("/{path:*}", handler)
	tree.Add("/test/{path:*}", handler)

	ctx := new(fasthttp.RequestCtx)
	h, tsr := tree.Get("/test", ctx)
	if h == nil || tsr {
		t.Fatalf("expected parent handler, got handler=%v tsr=%v", h != nil, tsr)
	}
	if got := ctx.UserValue("path"); got != "test" {
		t.Fatalf("path = %q, want %q", got, "test")
	}
}

Comment thread radix/node.go Outdated
}

// No parent wildcard, honor the trailing-slash redirect.
if child.tsr {

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Adding an unrelated /team route makes /test return tsr=true again. It splits the shared te prefix, so the wildcard is now on an ancestor instead of the immediate parent. Can we let the lookup fall back to that wildcard before returning the redirect?

This fails when added to radix/wildcard_fallback_test.go:

func TestParentWildcardWithSharedPrefix(t *testing.T) {
	tree := New()
	tree.Add("/{path:*}", func(ctx *fasthttp.RequestCtx) { ctx.WriteString("parent") })
	tree.Add("/test/{path:*}", func(ctx *fasthttp.RequestCtx) { ctx.WriteString("child") })
	tree.Add("/team", func(*fasthttp.RequestCtx) {})

	ctx := new(fasthttp.RequestCtx)
	h, tsr := tree.Get("/test", ctx)
	if h == nil || tsr {
		t.Fatalf("expected parent handler, got handler=%v tsr=%v", h != nil, tsr)
	}
	h(ctx)
	if got := string(ctx.Response.Body()); got != "parent" {
		t.Fatalf("body = %q, want %q", got, "parent")
	}
}

- Set UserValue("path") to actual remaining path instead of empty string
- Fall back to parent wildcard when static child has no handler and no
  wildcard, allowing /test to match /{path:*} correctly
- Also handle shared-prefix case where /team splits the parent into /t,
  so we search ancestors for a wildcard before returning TSR
- Add comprehensive tests covering both reviewer scenarios

Signed-off-by: Arunesh Dwivedi <arunesh.devops@gmail.com>
@AruneshDwivedi

Copy link
Copy Markdown
Author

Hey -- I've addressed both of your points: (1) is now copied via so UserValue("path") returns the correct value instead of empty string, (2) when a shared prefix like splits the parent node, we search up the ancestor chain for a wildcard before falling back to TSR. Added comprehensive tests covering all edge cases.

@AruneshDwivedi

Copy link
Copy Markdown
Author

Addressed both issues: (1) path value now correctly set via gstrings.Copy(path) so ctx.UserValue("path") returns the right value, (2) added ancestor wildcard search for shared-prefix case where /team splits the parent node.

@erikdubbelboer erikdubbelboer left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@AruneshDwivedi the PR now just deletes radix/node.go? 🤷‍♂️

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

registering a child wildcard router masks the parent

2 participants