http: fix ServeMux {wildcard...}, {$}, and method-prefixed patterns#56
Open
ultramcu wants to merge 1 commit into
Open
http: fix ServeMux {wildcard...}, {$}, and method-prefixed patterns#56ultramcu wants to merge 1 commit into
ultramcu wants to merge 1 commit into
Conversation
The hand-rolled ServeMux diverged from net/http.ServeMux on three
Go-1.22 routing features:
- {name...} multi-segment wildcards never matched: patternCouldMatch and
extractPathValues split on "/" and required equal segment counts, so a
trailing {x...} could only ever match a single segment. It now consumes
the remainder (including the empty remainder, e.g. /files/ -> path="").
- {$} end-of-path anchors were treated as ordinary captures: /exact/{$}
over-matched /exact/sub and set a bogus PathValue("$"). It now matches
only the exact path and is never captured as a value.
- Method-prefixed patterns ("GET /foo") did not route and corrupted host
routing: the entry was stored under the full key so a bare-path lookup
missed it, and a non-"/" first byte set mux.hosts=true, breaking every
other route. Handle now parses an optional leading method, keys match
decisions on the stripped path, and selects entries by method plus
net/http-compatible specificity.
Verified against net/http.ServeMux as an oracle (host) across the three
features plus precedence interactions and no-regression on exact, {id},
subtree and host patterns; fail-before/pass-after holds.
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.
Fixes #55.
The hand-rolled
ServeMuxinhttp/server.gomishandled three Go-1.22 routing features, each diverging fromnet/http.ServeMux. Per @deadprogram's note on #55 that the divergence wasn't intentional, this fixes all three.Bugs fixed
{name...}multi-segment wildcard never matched.mux.Handle("/files/{path...}", h); GET /files/a/b/creturned 404.patternCouldMatch/extractPathValuessplit on/and required equal segment counts, so a trailing{x...}could only match a single segment. It now consumes the remainder — including the empty remainder (/files/→PathValue("path") == ""), matching net/http.{$}end-anchor treated as an ordinary capture./exact/{$}over-matched/exact/suband set a bogusPathValue("$") == "sub". It now matches only the exact/exact/and is never captured as a value.Method-prefixed patterns (
"GET /foo") didn't route and corrupted host routing. The entry was stored under the full"GET /foo"key (so a bare-path lookup missed it), and because the pattern's first byte wasn't/,mux.hostswas flipped totrue, breaking routing for the whole mux.Handlenow parses an optional leading method, keys match decisions on the stripped path, and selects entries by method plus net/http-compatible specificity.Verification
Verified against the real
net/http.ServeMuxas an oracle (on host) across the three features above, precedence interactions ({id}vs{x...}single/deep,{$}vs a trailing-slash subtree, method vs method-less), and no-regression on exact /{id}/ subtree / host patterns. Fail-before/pass-after holds and the winning pattern is order-independent (transitive) and agrees with net/http on every legal registration set tested.http/server_test.goadds table-driven coverage for all three features plus the precedence and no-regression cases. (Note: it runs under the TinyGo test harness; the package can't bego test-ed standalone on host since there's no module-rootgo.mod, so verification was done with copied helpers against the net/http oracle.)Scope notes
POSTto aGET-only route returns 404, not net/http's405 Method Not Allowed+Allowheader. TinyGo'sServeMuxhas no 405/Allowmachinery; adding it would mean tracking the method set per path and is left out of scope. The routing decision (match vs no-match) matches net/http.http/pattern.go(the full upstream matcher) is intentionally left unused; this fix stays within the existing hand-rolledServeMuxrather than rewiring it./a/{id}/) remain unsupported, as before (not a regression).