Skip to content

fix(router): removing a route with a custom method panics - #3160

Merged
vishr merged 3 commits into
masterfrom
fix/router-remove-custom-method
Oct 6, 2026
Merged

vishr merged 3 commits into
masterfrom
fix/router-remove-custom-method

Conversation

@vishr

@vishr vishr commented Oct 5, 2026

Copy link
Copy Markdown
Member

Router.Remove panics with a nil pointer dereference for a route registered with a custom method, for example e.Add("PURGE", "/cache", h) followed by e.Router().Remove("PURGE", "/cache"). ConcurrentRouter.Remove delegates to it, so it panics too.

Cause: Remove clears the handler with setHandler(method, nil). Methods with their own field (GET, POST, …) just set it to nil. Custom methods fall through to the default branch of routeMethods.set, which read r.handler on the nil route.

Fix: treat a nil route like a nil handler and delete the anyOther entry. Remove is the only caller that passes nil.

Test: TestDefaultRouter_RemoveCustomMethod covers two cases:

  • Removing a custom-method route from a path that keeps its GET route: the Routes list is updated, a later PURGE request gets 405, and PURGE is gone from the Allow header.
  • Removing the only route on a path: a later request gets 404.

It panics on master and passes here. go test -race ./..., go vet, staticcheck and golint pass.

v4 has no Router.Remove, so this is v5 only.

vishr added 3 commits October 5, 2026 16:09
Router.Remove clears the handler by calling setHandler(method, nil). For a
method without its own field, routeMethods.set then read r.handler on the
nil route and panicked. Treat a nil route like a nil handler and delete
the entry.
@vishr
vishr requested a balanced review from Copilot October 6, 2026 03:22

Copilot AI 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.

Copilot review overview

🟢 Approval recommended

The narrow nil guard fixes the panic while preserving existing behavior, with regression coverage for both removal outcomes.

Review effort: Balanced
Findings: None

What changed in this PR

Fixes the Echo v5 router panic when removing a route registered with a custom HTTP method.

Changes:

  • Handles nil routes before accessing their handlers.
  • Adds regression coverage for removal with and without another route remaining.
File Description
router.go Guards against nil routes when deleting custom methods.
router_test.go Verifies route listings, 405/404 responses, and the Allow header after removal.

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

@vishr
vishr merged commit 2968240 into master Oct 6, 2026
12 checks passed
@vishr
vishr deleted the fix/router-remove-custom-method branch October 6, 2026 03: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