From be56c82b2ca6f46ef84eb0ed8b6d4f6d7c4a2b05 Mon Sep 17 00:00:00 2001 From: Patricio Whittingslow Date: Wed, 29 Jul 2026 23:58:59 -0300 Subject: [PATCH] MuxSlice more method muxing improvements --- examples/http-linux/main-httplinux.go | 2 +- http/httphi/bench_test.go | 2 +- http/httphi/exchange.go | 93 +++++++++++++++---- http/httphi/exchange_test.go | 110 +++++++++++++++++++++- http/httphi/httphi_fuzz_test.go | 2 +- http/httphi/mux.go | 82 ++++++++++++++++- http/httphi/mux_test.go | 126 ++++++++++++++++++++++++++ http/httphi/router.go | 38 ++++---- 8 files changed, 412 insertions(+), 43 deletions(-) diff --git a/examples/http-linux/main-httplinux.go b/examples/http-linux/main-httplinux.go index a4548d9..053432e 100644 --- a/examples/http-linux/main-httplinux.go +++ b/examples/http-linux/main-httplinux.go @@ -100,7 +100,7 @@ func homepage(exch *httphi.Exchange) { n += copy(page[n:], htmlTail) exch.StageHeader("Content-Type", "text/html") - exch.StageHeaderInt("Content-Length", int64(n), 10) + exch.StageHeaderInt("Content-Length", int64(n)) exch.WriteHeader(int(httphi.StatusOK)) exch.WriteBody(page[:n]) } diff --git a/http/httphi/bench_test.go b/http/httphi/bench_test.go index 99fb33c..4a15e01 100644 --- a/http/httphi/bench_test.go +++ b/http/httphi/bench_test.go @@ -68,7 +68,7 @@ func BenchmarkHandle(b *testing.B) { request: "GET /?abc=123 HTTP/1.1\r\nHost: tinygo.org\r\nUser-Agent: bench\r\nAccept: */*\r\nConnection: close\r\n\r\n", handler: func(ex *Exchange) { ex.StageHeader("Content-Type", "text/plain") - ex.StageHeaderInt("Content-Length", int64(len(benchBody)), 10) + ex.StageHeaderIntBase("Content-Length", int64(len(benchBody)), 10) data, present := ex.RequestQueryAppend(buf[:0], "abc", true) if !present || !internal.BytesEqual(data, expect) { panic("invalid result") diff --git a/http/httphi/exchange.go b/http/httphi/exchange.go index 9814cd0..337023b 100644 --- a/http/httphi/exchange.go +++ b/http/httphi/exchange.go @@ -53,24 +53,28 @@ type Exchange struct { // ExchangeConfig is the memory an [Exchange] is fixed to for the rest of its // life by [Exchange.Configure]. A [Router] derives one per exchange from its // [RouterConfig], which is what bounds the router's memory. +// +// Fields open with Required, Conditional or Optional and the constraint in +// brackets, as in [RouterConfig]. type ExchangeConfig struct { - // RawBuf is the single buffer holding the request header, the response + // Required [non-empty] single buffer holding the request header, the response // header and any surplus body. See [Exchange.UnsafeRawBuffer]. RawBuf []byte - // RequestBufferLim reserves the first bytes of RawBuf for the request - // header, the rest being the response. Configure panics if it exceeds RawBuf. + // Required [<=len(RawBuf)] bytes of RawBuf reserved for the request header, + // the rest being the response. Configure panics if it exceeds RawBuf. RequestBufferLim int - // NumHeaderKVCap is how many request header fields may be parsed. A request - // carrying more is answered 431, see [httpraw.ErrHeaderTooMany]. + // Required [>0] request header fields that may be parsed. A request carrying + // more is answered 431, see [httpraw.ErrHeaderTooMany]. NumHeaderKVCap int - // NormalizeOutgoingKeys normalizes staged response header keys as they are + // Optional [any] normalization of staged response header keys as they are // written, i.e: "content-type" becomes "Content-Type". NormalizeOutgoingKeys bool - // NoRequestBufferGrowth holds the request header to RequestBufferLim rather - // than growing it. A header outgrowing it is answered 431, see [httpraw.ErrBufferExhausted]. + // Optional [any] cap holding the request header to RequestBufferLim rather than + // growing it. A header outgrowing it is answered 431, see [httpraw.ErrBufferExhausted]. NoRequestBufferGrowth bool - // MaxPathValues is how many wildcards a single pattern may bind, read back with - // [Exchange.PathValue]. A pattern with more never matches, see [SetPathValues]. + // Conditional [>=the most wildcards any one registered pattern binds] number of + // path values bindable, read back with [Exchange.PathValue]. A pattern binding + // more never matches, see [SetPathValues]. Zero suits a mux of literal patterns. MaxPathValues int } @@ -201,11 +205,23 @@ func (exch *Exchange) StageHeader(key, value string) (enoughMemory bool) { return true } -// StageHeaderInt is [Exchange.StageHeader] with an integer value, i.e: Content-Length. +// StageHeaderBytes is [Exchange.StageHeader] with a byte slice value, i.e: a +// field copied out of the request. The value is not retained. +func (exch *Exchange) StageHeaderBytes(key string, value []byte) (enoughMemory bool) { + return exch.StageHeader(key, b2s(value)) +} + +// StageHeaderInt is [Exchange.StageHeaderIntBase] in base 10, which is the base +// every HTTP field value carrying a number uses, i.e: Content-Length. +func (exch *Exchange) StageHeaderInt(key string, value int64) (enoughMemory bool) { + return exch.StageHeaderIntBase(key, value, 10) +} + +// StageHeaderIntBase is [Exchange.StageHeader] with an integer value, i.e: Content-Length. // It formats the value directly into the response buffer without allocating. // base must be in the range 10..36; lower bases are dropped, no HTTP header // field value is written below base 10. -func (exch *Exchange) StageHeaderInt(key string, value int64, base int) (enoughMemory bool) { +func (exch *Exchange) StageHeaderIntBase(key string, value int64, base int) (enoughMemory bool) { if exch.headerWritten || base < 10 || base > 36 { return false } @@ -261,6 +277,42 @@ func (exch *Exchange) WriteHeader(code int) (n int, err error) { return n, err } +// Respond writes a complete response in one call: Content-Type, a Content-Length +// taken from len(body), the status line and the body. An empty contentType +// stages no Content-Type field, for a code that carries no entity. +// +// It also stages "Connection: close", the router serving one exchange per +// connection, so a peer never waits on a response that is not coming. +// +// Returns [Exchange.ResponseError]: staged fields that did not fit and failed +// writes are both reported there, so a truncated response cannot pass silently. +func (exch *Exchange) Respond(code int, contentType string, body []byte) error { + exch.stageResponse(code, contentType, len(body)) + exch.WriteBody(body) // Reports through respErr, checked below. + return exch.respErr +} + +// RespondString is [Exchange.Respond] with a string body, saving the conversion. +func (exch *Exchange) RespondString(code int, contentType, body string) error { + exch.stageResponse(code, contentType, len(body)) + exch.WriteBodyString(body) // Reports through respErr, checked below. + return exch.respErr +} + +// stageResponse stages the fields and status line a complete response needs. +// Drops are recorded on respErr by the Stage* calls, so [Exchange.WriteBody] +// declines to write a partial header afterwards. +func (exch *Exchange) stageResponse(code int, contentType string, bodyLen int) { + if contentType != "" { + exch.StageHeader("Content-Type", contentType) + } + exch.StageHeaderInt("Content-Length", int64(bodyLen)) + // One exchange per connection today, so the peer is told not to wait for a + // second response on it. Revisit once the router loops exchanges. + exch.StageHeader("Connection", "close") + exch.StageStatus(code) +} + // ResponseError returns any error encountered during staging of headers or during writing of response. // Provides an ergonomic way of checking if one ran out of buffer space after staging all headers with [Exchange.StageHeader]. func (exch *Exchange) ResponseError() error { @@ -368,7 +420,7 @@ func (exch *Exchange) WriteBodyString(buf string) (int, error) { return exch.WriteBody(unsafe.Slice(unsafe.StringData(buf), len(buf))) } -// Write writes response body bytes, flushing the header first if the handler +// WriteBody writes response body bytes, flushing the header first if the handler // has not written it yet. Once a write to the connection fails the response is // unrecoverable and every later write returns that same error, so a body never // reaches the wire without its header. @@ -464,11 +516,18 @@ func (exch *Exchange) RequestContentLength() (_ int64, present bool, _ error) { // [Exchange.RequestQuery] or [Exchange.RequestQueryAppend]. The body is consumed, so // call this before [Exchange.ReadBody]. // -// A request with no Content-Length has no body, RFC 9112 6.3, and yields an -// empty form. Use [Exchange.RequestContentLength] to tell that apart from a body -// that arrived empty. +// A request with no Content-Length has no body, RFC 9112 6.3, and a request with +// no Content-Type declares no encoding to parse, RFC 9110 8.3. Both yield an +// empty form and a nil error, a bodiless POST being legal. Use +// [Exchange.RequestContentLength] and [Exchange.RequestContentType] to tell +// either apart from a body that arrived empty. A Content-Type that is present +// and not form encoded is [errNotFormEncoded], an absent one never is. func (exch *Exchange) RequestParseForm(dst *httpraw.Form, buf []byte) error { - if !httpraw.MediaTypeIs(exch.RequestContentType(), "application/x-www-form-urlencoded") { + contentType := exch.RequestContentType() + if contentType == nil { + dst.Reset(nil, 0) + return nil // No declared encoding is no form, as no length is no body. + } else if !httpraw.MediaTypeIs(contentType, "application/x-www-form-urlencoded") { return errNotFormEncoded } else if exch.RequestHeaderRaw().GetFold("Transfer-Encoding") != nil { // Chunked bodies are framed, so reading Content-Length bytes off the diff --git a/http/httphi/exchange_test.go b/http/httphi/exchange_test.go index 173176f..d53f793 100644 --- a/http/httphi/exchange_test.go +++ b/http/httphi/exchange_test.go @@ -582,7 +582,7 @@ func TestExchangeSetHeaderInt(t *testing.T) { t.Run(name, func(t *testing.T) { conn := newConn("") exch := newExchange(t, conn, ExchangeConfig{RawBuf: make([]byte, 2*256), RequestBufferLim: 256}) - exch.StageHeaderInt("N", test.value, test.base) + exch.StageHeaderIntBase("N", test.value, test.base) exch.WriteHeader(200) got, _ := strings.CutPrefix(conn.ViewWritten(), "HTTP/1.1 200 OK\r\n") if got != test.want { @@ -596,7 +596,7 @@ func TestExchangeSetHeaderInt(t *testing.T) { func TestExchangeSetHeaderIntNoAlloc(t *testing.T) { exch := newExchange(t, newConn(""), ExchangeConfig{RawBuf: make([]byte, 2*256), RequestBufferLim: 256}) allocs := testing.AllocsPerRun(100, func() { - exch.StageHeaderInt("Content-Length", 1234567890, 10) + exch.StageHeaderIntBase("Content-Length", 1234567890, 10) }) if allocs != 0 { t.Errorf("SetHeaderInt allocated %v times, want 0", allocs) @@ -810,10 +810,13 @@ func TestExchangeRequestParseForm(t *testing.T) { contentType: "text/plain", wantErr: errNotFormEncoded, }, { + // An absent field is not a wrong one: no media type is no body, the + // same answer "no content length" gets above. Only a type that is + // present and not form encoded is an error. name: "no media type", formVals: []formPair{{key: "a", value: "1"}}, noContentType: true, - wantErr: errNotFormEncoded, + wantVals: []formPair{}, }, { // The coding is refused on the field alone, so the body stays off. name: "chunked", @@ -1472,3 +1475,104 @@ func TestExchangeRequestParseFormFoldedTransferEncoding(t *testing.T) { }) } } + +// StageHeaderInt defaults to base 10, the only base an HTTP field value uses, +// so callers do not repeat it at every site. +func TestExchangeStageHeaderIntDefaultsBase10(t *testing.T) { + conn := newConn("") + exch := newExchange(t, conn, ExchangeConfig{RawBuf: make([]byte, 256), RequestBufferLim: 128}) + if !exch.StageHeaderInt("Content-Length", 1234567890) { + t.Fatal("want field staged") + } + exch.WriteHeader(200) + const want = "HTTP/1.1 200 OK\r\nContent-Length:1234567890\r\n\r\n" + if got := conn.ViewWritten(); got != want { + t.Errorf("want %q, got %q", want, got) + } +} + +// StageHeaderBytes stages a value already held as bytes without the caller +// converting it to a string first. +func TestExchangeStageHeaderBytes(t *testing.T) { + conn := newConn("") + exch := newExchange(t, conn, ExchangeConfig{RawBuf: make([]byte, 256), RequestBufferLim: 128}) + value := []byte("text/html") + if !exch.StageHeaderBytes("Content-Type", value) { + t.Fatal("want field staged") + } + // The value must be copied, not aliased: mutating it after staging must not + // change what reaches the wire. + value[0] = 'X' + exch.WriteHeader(200) + const want = "HTTP/1.1 200 OK\r\nContent-Type:text/html\r\n\r\n" + if got := conn.ViewWritten(); got != want { + t.Errorf("want %q, got %q", want, got) + } +} + +// Respond replaces the stage/stage/stage/write boilerplate every handler paid, +// deriving Content-Length from the body so it cannot disagree with what is sent. +func TestExchangeRespond(t *testing.T) { + for _, test := range []struct { + name string + code int + contentType string + body string + want string + }{ + { + name: "html", code: 200, contentType: "text/html", body: "

hi

", + want: "HTTP/1.1 200 OK\r\nContent-Type:text/html\r\nContent-Length:11\r\nConnection:close\r\n\r\n

hi

", + }, + { + name: "empty body still declares zero length", code: 200, contentType: "text/plain", body: "", + want: "HTTP/1.1 200 OK\r\nContent-Type:text/plain\r\nContent-Length:0\r\nConnection:close\r\n\r\n", + }, + { + name: "no content type staged when empty", code: 204, contentType: "", body: "", + want: "HTTP/1.1 204 No Content\r\nContent-Length:0\r\nConnection:close\r\n\r\n", + }, + { + name: "error code carries a body", code: 500, contentType: "text/plain", body: "boom", + want: "HTTP/1.1 500 Internal Server Error\r\nContent-Type:text/plain\r\nContent-Length:4\r\nConnection:close\r\n\r\nboom", + }, + } { + t.Run(test.name+"/bytes", func(t *testing.T) { + conn := newConn("") + exch := newExchange(t, conn, ExchangeConfig{RawBuf: make([]byte, 512), RequestBufferLim: 256}) + if err := exch.Respond(test.code, test.contentType, []byte(test.body)); err != nil { + t.Fatalf("Respond: %s", err) + } + if got := conn.ViewWritten(); got != test.want { + t.Errorf("want %q, got %q", test.want, got) + } + }) + t.Run(test.name+"/string", func(t *testing.T) { + conn := newConn("") + exch := newExchange(t, conn, ExchangeConfig{RawBuf: make([]byte, 512), RequestBufferLim: 256}) + if err := exch.RespondString(test.code, test.contentType, test.body); err != nil { + t.Fatalf("RespondString: %s", err) + } + if got := conn.ViewWritten(); got != test.want { + t.Errorf("want %q, got %q", test.want, got) + } + }) + } +} + +// A response that does not fit must be reported, not shipped truncated: the +// whole point of folding the boilerplate into one call. +func TestExchangeRespondReportsOverflow(t *testing.T) { + conn := newConn("") + exch := newExchange(t, conn, ExchangeConfig{RawBuf: make([]byte, 64), RequestBufferLim: 32}) + err := exch.Respond(200, strings.Repeat("t", 200), []byte("body")) + if err == nil { + t.Fatal("want an error for a response header that cannot fit") + } + if got := conn.ViewWritten(); got != "" { + t.Errorf("nothing must reach the wire, got %q", got) + } + if exch.ResponseError() == nil { + t.Error("want the failure recorded on the exchange too") + } +} diff --git a/http/httphi/httphi_fuzz_test.go b/http/httphi/httphi_fuzz_test.go index 209f452..a4c07b9 100644 --- a/http/httphi/httphi_fuzz_test.go +++ b/http/httphi/httphi_fuzz_test.go @@ -190,7 +190,7 @@ func FuzzHandleRequest(f *testing.F) { // escaping, not this package's framing. for i := range stage { exch.StageHeader("X-Fuzz", "value") - exch.StageHeaderInt("X-Fuzz-Int", int64(i), 10) + exch.StageHeaderIntBase("X-Fuzz-Int", int64(i), 10) } } if ops&opReadBody != 0 { diff --git a/http/httphi/mux.go b/http/httphi/mux.go index d639cd5..3908990 100644 --- a/http/httphi/mux.go +++ b/http/httphi/mux.go @@ -104,6 +104,8 @@ type Mux interface { // requestPath is a buffer owned by the [Exchange] usually and should not be held after LookupHandler returns. LookupHandler(get Method, requestPath []byte, dstPathVals []PathValue) (matchedPattern string, handler HandlerFunc) // MaxPathValues specifies the required size of dstPathVals in a call to [Mux.LookupHandler]. + // MaxPathValues should return -1 if no paths have been configured to catch situation + // where the Mux has been passed to a [Router.Configuration] before registering paths. MaxPathValues() int } @@ -230,6 +232,11 @@ func (sm *MuxSlice) Reset(capacity int) { // LookupHandler returns the handler registered for request path, or nil if none matches. // The first registration matching both method and uri wins. +// +// Every method this package does not name is [MethUnknown], so a request with an +// extension method matches a bare-path registration and any registration naming +// an extension method, whichever it names. Tell PROPFIND from MKCOL inside the +// handler with [Exchange.RequestMethodRaw]. func (sm *MuxSlice) LookupHandler(method Method, path []byte, dstPathVals []PathValue) (matched string, _ HandlerFunc) { for _, endpoint := range sm._handlers { if endpoint.method != MethUndefined && endpoint.method != method { @@ -238,7 +245,7 @@ func (sm *MuxSlice) LookupHandler(method Method, path []byte, dstPathVals []Path // Method matches. A pattern ending in '/' is a wildcard despite binding no // values: the trailing slash is an anonymous "{...}", so it must go // through the matcher and not a literal compare, see [SetPathValues]. - if endpoint.pathVals > 0 || strings.HasSuffix(endpoint.path, "/") { + if isWildcardPattern(endpoint.path) { if ok, _ := SetPathValues(dstPathVals, endpoint.path, path); ok { return endpoint.path, endpoint.handler } @@ -251,6 +258,9 @@ func (sm *MuxSlice) LookupHandler(method Method, path []byte, dstPathVals []Path // MaxPathValues returns the maximum number of path values any endpoint could have. func (sm *MuxSlice) MaxPathValues() (maxPathValues int) { + if len(sm._handlers) == 0 { + return -1 // Signal no handlers registered. + } for _, endpoint := range sm._handlers { maxPathValues = max(maxPathValues, endpoint.pathVals) } @@ -259,22 +269,86 @@ func (sm *MuxSlice) MaxPathValues() (maxPathValues int) { // Handle registers handler for reg, either a bare path matching any method or a // method and path separated by a space, i.e: "/health" or "GET /health". -// Handle does not check for duplicate registrations: the first one added wins. +// +// Handle panics on a registration that could never serve a request: a method +// token carrying lowercase (methods are case sensitive and uppercase, RFC 9110 +// 9.1, so "Get" matches no GET request), a path not rooted at '/', or an exact +// duplicate of an earlier registration, which the first one always shadows. +// Registration is program startup, so a fault belongs there and not in a +// permanent silent 404. func (sm *MuxSlice) Handle(optMethodAndPath string, handler HandlerFunc) { - v := internal.SliceReclaim(&sm._handlers) method := MethUndefined methodOrURL, url, methodFound := strings.Cut(optMethodAndPath, " ") if methodFound { + if hasLowerASCII(methodOrURL) { + panic("httphi: method must be uppercase in registration " + optMethodAndPath) + } method = MethodFrom(methodOrURL) } else { url = methodOrURL } - v.pathVals = strings.Count(optMethodAndPath, "{") + if len(url) == 0 || url[0] != '/' { + panic("httphi: path must begin with '/' in registration " + optMethodAndPath) + } + for _, endpoint := range sm._handlers { + if endpoint.method == method && endpoint.path == url { + if method == MethUnknown { + // Two extension methods are both MethUnknown, so the second is + // unreachable. Register one and branch in the handler, see + // [MuxSlice.LookupHandler]. + panic("httphi: extension method already registered on path in " + optMethodAndPath) + } + panic("httphi: duplicate registration " + optMethodAndPath) + } + } + v := internal.SliceReclaim(&sm._handlers) + v.pathVals = countPathValues(url) v.method = method v.path = url v.handler = handler } +// countPathValues is how many values pattern can bind, which is what sizes the +// slice [SetPathValues] writes into. Only a named wildcard segment binds: "{$}" +// marks the path's end, an anonymous "{...}" has no name to bind under, and a +// brace inside a literal segment is not a wildcard at all. +func countPathValues(pattern string) (n int) { + if len(pattern) == 0 || pattern[0] != '/' { + return 0 + } + pattern = pattern[1:] + for len(pattern) > 0 { + segment, rest, more := strings.Cut(pattern, "/") + if name, _, ok := pathWildcard(segment); ok && name != "" && name != "$" { + n++ + } + if !more { + break + } + pattern = rest + } + return n +} + +// isWildcardPattern reports whether pattern must go through [SetPathValues] +// rather than a literal comparison. Distinct from the value count: "{$}" and a +// trailing slash match by walking segments while binding nothing. +func isWildcardPattern(pattern string) bool { + return strings.IndexByte(pattern, '{') >= 0 || strings.HasSuffix(pattern, "/") +} + +// hasLowerASCII reports whether s carries an ASCII lowercase letter, which a +// method token registered by mistake ("Get") does and a legal extension method +// ("PROPFIND") does not. +func hasLowerASCII(s string) bool { + for i := 0; i < len(s); i++ { + if s[i] >= 'a' && s[i] <= 'z' { + return true + } + } + return false +} + // Method is a HTTP request method, parsed by [MethodFrom]. type Method uint8 diff --git a/http/httphi/mux_test.go b/http/httphi/mux_test.go index f9911eb..6351d69 100644 --- a/http/httphi/mux_test.go +++ b/http/httphi/mux_test.go @@ -308,3 +308,129 @@ func TestMuxSliceTrailingSlashPattern(t *testing.T) { }) } } + +// A malformed registration is a programming error, and one that otherwise costs +// a permanent silent 404 at runtime: "Get /x" parses to MethUnknown, which no +// GET request ever matches but every extension-method request does. Fail at +// registration, where the stack points at the offending line. +func TestMuxSliceHandlePanicsOnBadRegistration(t *testing.T) { + for _, test := range []struct { + name string + reg string + }{ + {name: "lowercase method", reg: "Get /x"}, + {name: "all lower method", reg: "get /x"}, + {name: "mixed case method", reg: "pOsT /x"}, + {name: "no leading slash", reg: "GET x"}, + {name: "bare path no slash", reg: "x"}, + {name: "empty path after method", reg: "GET "}, + } { + t.Run(test.name, func(t *testing.T) { + var sm MuxSlice + sm.Reset(1) + defer func() { + if recover() == nil { + t.Errorf("want panic registering %q", test.reg) + } + }() + sm.Handle(test.reg, func(ex *Exchange) {}) + }) + } +} + +// An exact duplicate is unreachable code: the first registration always wins. +func TestMuxSliceHandlePanicsOnDuplicate(t *testing.T) { + var sm MuxSlice + sm.Reset(2) + sm.Handle("GET /x", func(ex *Exchange) {}) + defer func() { + if recover() == nil { + t.Error("want panic registering the same method and path twice") + } + }() + sm.Handle("GET /x", func(ex *Exchange) {}) +} + +// Extension methods are legal and uppercase, so they must still register: only +// the case-mangled forms are rejected. +func TestMuxSliceHandleAllowsExtensionMethod(t *testing.T) { + var sm MuxSlice + sm.Reset(2) + sm.Handle("PROPFIND /dav", func(ex *Exchange) {}) + sm.Handle("/any-method", func(ex *Exchange) {}) // Bare path matches any method. + if sm.MaxPathValues() != 0 { + t.Errorf("want 0 path values, got %d", sm.MaxPathValues()) + } +} + +// pathVals sizes the slice SetPathValues writes into, so it must count exactly +// the wildcards that bind. Counting braces over-reports: "{$}" marks the path's +// end, an anonymous "{...}" has no name, and a brace inside a literal segment is +// not a wildcard at all. Each of those binds nothing. +func TestMuxSliceMaxPathValuesCountsOnlyBindingWildcards(t *testing.T) { + for _, test := range []struct { + pattern string + want int + }{ + {pattern: "/health", want: 0}, + {pattern: "/", want: 0}, + {pattern: "/files/", want: 0}, // Anonymous trailing wildcard. + {pattern: "/{$}", want: 0}, // End-of-path marker. + {pattern: "/a/{$}", want: 0}, // + {pattern: "/b_{bucket}", want: 0}, // Literal: brace not a whole segment. + {pattern: "/{...}", want: 0}, // Multi wildcard with no name. + {pattern: "/users/{id}", want: 1}, // + {pattern: "/files/{p...}", want: 1}, // + {pattern: "/{a}/{b}", want: 2}, // + {pattern: "/b/{bucket}/o/{obj...}", want: 2}, + } { + t.Run(test.pattern, func(t *testing.T) { + var sm MuxSlice + sm.Reset(1) + sm.Handle(test.pattern, func(ex *Exchange) {}) + if got := sm.MaxPathValues(); got != test.want { + t.Errorf("want %d path values, got %d", test.want, got) + } + }) + } +} + +// Sizing and routing are different questions: "{$}" binds no values yet still +// needs the matcher, so an exact pathVals count must not send it to the literal +// comparison instead. +func TestMuxSliceZeroValueWildcardStillMatches(t *testing.T) { + for _, test := range []struct { + pattern string + path string + want bool + }{ + {pattern: "/{$}", path: "/", want: true}, + {pattern: "/{$}", path: "/x", want: false}, + {pattern: "/a/{$}", path: "/a/", want: true}, + {pattern: "/a/{$}", path: "/a/b", want: false}, + {pattern: "/{...}", path: "/any/thing", want: true}, + } { + t.Run(test.pattern+"__"+test.path, func(t *testing.T) { + var sm MuxSlice + sm.Reset(1) + var served bool + sm.Handle(test.pattern, func(ex *Exchange) { served = true; ex.WriteHeader(200) }) + exch := new(Exchange) + exch.Configure(ExchangeConfig{ + RawBuf: make([]byte, 2048), RequestBufferLim: 1024, + NumHeaderKVCap: defaultNumHeaderKVCap, MaxPathValues: sm.MaxPathValues(), + }) + conn := newConn("GET " + test.path + " HTTP/1.1\r\nHost: h\r\n\r\n") + conn.Hangup() + if !exch.Acquire(conn) { + t.Fatal("acquire") + } + if err := Handle(exch, &sm, nopBackoff); err != nil { + t.Fatal(err) + } + if served != test.want { + t.Errorf("want served=%v, got %v", test.want, served) + } + }) + } +} diff --git a/http/httphi/router.go b/http/httphi/router.go index 7e4177f..cbe6242 100644 --- a/http/httphi/router.go +++ b/http/httphi/router.go @@ -70,32 +70,35 @@ type job struct { } // RouterConfig configures a [Router]. See [Router.Configure]. +// +// Each field below opens with Required, Conditional or Optional followed by the +// constraint in brackets, that being what [RouterConfig.Validate] rejects on. type RouterConfig struct { - // FixedNumGoroutines must be set to either -1 (freely allocate new goroutines) or to the number of goroutines - // to spawn on [Router.Configure] being called. + // Required [-1 or >0] number of goroutines to spawn on [Router.Configure], + // -1 meaning allocate them freely per connection instead. FixedNumGoroutines int - // RequestHeaderBufferSize determines the buffer allocated - // for processing request HTTP headers including request-target (URI), protocol and key/value pairs. + // Required [>=32, sum with ResponseHeaderMinBufferSize <=65535] buffer for the + // request header: request-target (URI), protocol and key/value pairs. RequestHeaderBufferSize int - // ResponseHeaderMinBufferSize determines buffer allocated for processing response headers. - // Response buffer will reuse unused request memory so this is not a strict limit. - // "HTTP/1.1 200 OK\r\n" does not count towards this memory, only actual Headers key/value pairs use this memory. - // After memory is fully consumed [Exchange.StageHeader] will not append more headers. + // Required [>=2, <=65535] buffer for response headers. Reuses unused request + // memory so it is not a strict limit, and the status line does not count + // towards it. Once consumed [Exchange.StageHeader] appends no more fields. ResponseHeaderMinBufferSize int - // Number of request header key/value pairs to parse before failing and returning [StatusRequestHeaderFieldsTooLarge]. + // Required [>0] request header key/value pairs to parse before failing with + // [StatusRequestHeaderFieldsTooLarge]. RequestNumHeaderKVCap int - // NormalizeOutgoingKeys normalizes response header field keys as they are + // Optional [any] normalization of response header field keys as they are // staged, i.e: "content-type" becomes "Content-Type". NormalizeOutgoingKeys bool - // MaxAwaitingConns is the depth of the queue connections wait in for a free - // goroutine. [Router.Handle] drops connections once it is full. Required and - // must be non-zero when running a fixed number of goroutines, unused otherwise. + // Conditional [>0 when FixedNumGoroutines>0, unused otherwise] depth of the + // queue connections wait in. [Router.Handle] drops connections once it is full. MaxAwaitingConns int - // Mux resolves each request's method and path to the handler serving it. Required. + // Required [non-nil] resolver of each request's method and path to the handler + // serving it. Routes must be registered before Configure, see [Mux.MaxPathValues]. Mux Mux - // Logger receives failed exchanges. Optional, nil disables logging. + // Optional [nil disables] sink for failed exchanges. Logger *slog.Logger } @@ -185,6 +188,9 @@ func (r *Router) Configure(cfg RouterConfig) error { r.mux = cfg.Mux r.log = cfg.Logger maxPathValues := cfg.Mux.MaxPathValues() + if maxPathValues < 0 { + return errors.New("Mux paths must be registered before configuring Router") + } r.maxPathValues = maxPathValues r.normalizeKeys = cfg.NormalizeOutgoingKeys // Freelist entries were sized by the outgoing configuration: recycling one @@ -306,7 +312,7 @@ func (r *Router) goroWorker(gen uint32, queue chan job, mux Mux) { for job := range queue { exch := job.exch if exch == nil { - panic("httplo: unreachable nil job") + panic("httphi: unreachable nil job") } else if gen != r.gen.Load() { // Not released with freeExch since generation torn down, // new buffer may have been allocated for Exchanges.