fix: deduplicate Allow header methods in 405 responses#1112
Open
LeSingh1 wants to merge 1 commit into
Open
Conversation
When multiple overlapping parameterized route patterns all have the same
HTTP method registered (e.g. /{a}, /{b}-{c}, /{b}-{c}-{d}), a request
with an unregistered method would trigger methodNotAllowed on each
matching node, appending the allowed method to rctx.methodsAllowed
multiple times. The 405 handler then wrote one Allow header entry per
duplicate, producing responses like 'Allow: POST, POST, POST, POST'.
Fix: change rctx.methodsAllowed from []methodTyp to a methodTyp bitmask.
Each candidate node ORs its allowed methods into the bitmask instead of
appending to a slice, so each method appears at most once. The
methodNotAllowedHandler iterates reverseMethodMap and sets an Allow
header entry for each bit that is set.
Fixes go-chi#996
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
When multiple overlapping parameterized route patterns all register the same HTTP method, a request with an unregistered method produces duplicate entries in the
Allowresponse header.Minimal reproduction (from #996):
Each parameterized node that matched during route traversal independently appended
POSTtorctx.methodsAllowed []methodTyp. ThemethodNotAllowedHandlerthen wrote oneAllowheader value per slice entry — one per matched node — instead of one per distinct method.Fix
Replace
methodsAllowed []methodTypinContextwith amethodTypbitmask. SincemethodTypis already defined as a bitmask of power-of-two constants, each candidate node can OR its methods into the bitmask (rctx.methodsAllowed |= endpoints) rather than append to a slice.methodNotAllowedHandlerthen iteratesreverseMethodMapand emits oneAllowheader entry for each bit that is set — guaranteeing uniqueness without any extra allocation.Changes
context.go:methodsAllowed []methodTyp→methodsAllowed methodTyp;Reset()zeroes with= 0tree.go: twoappend(rctx.methodsAllowed, ...)→rctx.methodsAllowed |= endpointsmux.go:MethodNotAllowedHandlerandmethodNotAllowedHandlertake a singlemethodTypbitmask instead of...methodTyp; handler iteratesreverseMethodMapby bitmaskmux_test.go:TestMethodNotAllowedNoDuplicates— regression test from the exact scenario in Duplicate HTTP methods in 405 Allow header #996Test plan
TestMethodNotAllowedNoDuplicates— asserts no duplicate values inAllowheader and thatPOSTis presentTestMethodNotAllowedstill passes (2 distinct methods, GET and HEAD, each appear once)go test ./...Fixes #996