Skip to content

Conversation

@kolkov
Copy link
Contributor

@kolkov kolkov commented Nov 24, 2025

Summary

Fixes critical routing bug where routes with :param followed by multiple static segments would panic.

Problem

Routes like /users/:id/activate + /users/:id/deactivate caused panic at line 147 in tree.go:

panic: internal error: no common prefix between "/deactivate" and ":id"

Root Cause

Common prefix check compared /deactivate with :id and found no match.

Solution

Implemented httprouter's production-quality pattern:

  • Create empty placeholder child nodes after param nodes
  • Bypass common prefix check for param node children
  • Use direct children assignment (n.children = []*node{child}) to allow empty path placeholders

Changes

  • internal/radix/tree.go (lines 118-138): Added special case handling for param node children
  • internal/radix/tree_param_static_test.go (new file): 6 comprehensive test cases
  • CHANGELOG.md: Documented v0.3.3 release

Testing

✅ All 650+ tests passing with race detector (WSL2 Gentoo)
✅ Coverage: 88.4% internal/radix, 91.7% overall (maintained)
✅ Linter: 0 issues (golangci-lint clean)
✅ Performance: No regressions (256 ns/op static, 326 ns/op parametric)
✅ CI: GREEN

Test Cases Added

  1. TestTree_ParamWithMultipleStaticChildren - Original bug case
  2. TestTree_NestedParams - Nested params (/users/:user_id/posts/:post_id/comments)
  3. TestTree_AlternatingParamStatic - Alternating pattern (/a/:b/c/:d/e)
  4. TestTree_ParamWithLongStaticTail - Long static tails (/:a/b/c/d/e/f/g)
  5. TestTree_ConsecutiveParams - Consecutive params (/:a/:b/:c/d)
  6. TestTree_StaticVsParam - Static priority over params

Reference

Studied httprouter implementation (tree.go lines 217-270, 180-184) for production-quality reference solution.

Checklist

  • Tests added and passing
  • Race detector passed
  • Linter clean
  • CHANGELOG updated
  • Performance validated
  • CI passing

Fixes critical routing bug where routes with :param followed by multiple
static segments would panic. For example, registering both /users/:id/activate
and /users/:id/deactivate caused panic at line 147 in tree.go.

Root cause: Common prefix check compared /deactivate with :id and found
no match, causing "internal error: no common prefix" panic.

Solution: Implemented httprouter's pattern of creating empty placeholder
child nodes after param nodes and bypassing common prefix check when
inserting children of param nodes.

Changes:
- internal/radix/tree.go (lines 118-138): Added special case handling
  for param node children following httprouter's approach
- internal/radix/tree_param_static_test.go: Added 6 comprehensive test
  cases covering all param+static patterns

Testing:
- All 650+ tests passing with race detector (WSL2 Gentoo)
- Coverage maintained: 88.4% internal/radix, 91.7% overall
- Linter: 0 issues (golangci-lint clean)
- Performance: No regressions (256 ns/op static, 326 ns/op parametric)

Studied httprouter implementation (tree.go lines 217-270, 180-184) for
production-quality reference solution.
@codecov
Copy link

codecov bot commented Nov 24, 2025

Codecov Report

✅ All modified and coverable lines are covered by tests.

📢 Thoughts on this report? Let us know!

@kolkov kolkov merged commit efeb7de into develop Nov 24, 2025
11 checks passed
@kolkov kolkov deleted the hotfix/v0.3.3 branch November 24, 2025 00:26
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.

2 participants