Mux PathValue improvemnt and fixes

http/httphi/README.md

@@ -48,3 +48,8 @@ for {
 
 Runnable server over raw Linux sockets, plus query, form and multipart handlers:
 [`example_test.go`](./example_test.go).
+
+
+## Naming
+
+Gonna be honest with y'all. I initially wanted it to be named `httplo` until I saw I could write `httphi.MethHead` with a small change.

http/httphi/exchange_test.go

@@ -317,7 +317,9 @@ func TestHandleHTTP10Served(t *testing.T) {
 // No registered handler must yield 404, not an empty response.
 func TestHandleNoHandler(t *testing.T) {
 	var sm MuxSlice
-	sm.Handle("GET /", func(ex *Exchange) { t.Error("handler must not run") })
+	// "/{$}" is the root and nothing else; a bare "/" is a catch-all that would
+	// match /nowhere too, see [SetPathValues].
+	sm.Handle("GET /{$}", func(ex *Exchange) { t.Error("handler must not run") })
 	conn := serve(t, "GET /nowhere HTTP/1.1\r\nHost: h\r\n\r\n", &sm)
 	const want = "HTTP/1.1 404 Not Found\r\n\r\n"
 	if got := conn.ViewWritten(); got != want {

http/httphi/mux.go

@@ -66,6 +66,7 @@ func Handle(exch *Exchange, mux Mux, backoff lneto.BackoffStrategy) error {
 	// Mux on the request path: the query string is the handler's business.
 	path := reqhdr.RequestPath()
 	meth := reqhdr.Method()
+	clear(exch.pathValues)
 	matchedPattern, handler := mux.LookupHandler(MethodFromBytes(meth), path, exch.pathValues)
 	if handler != nil {
 		exch.matchedPattern = matchedPattern
@@ -124,17 +125,30 @@ var pathSeparator = []byte{'/'}
 // Unlike ServeMux, segments are compared and bound raw, so "/users/{id}" binds
 // "x%2Fy" and not "x/y". Which paths match is unaffected. Bound values alias
 // requestPath rather than copy it.
+//
+// Values are bound while walking, before the match is known, so on failure
+// SetPathValues clears what it bound. A [Mux] may then try patterns in turn
+// without a matching one inheriting values from one that failed.
 func SetPathValues(dstPathVals []PathValue, pattern string, requestPath []byte) (matched, pathValSliceTooShort bool) {
+	n, matched, pathValSliceTooShort := setPathValues(dstPathVals, pattern, requestPath)
+	if !matched {
+		clear(dstPathVals[:n])
+	}
+	return matched, pathValSliceTooShort
+}
+
+// setPathValues is [SetPathValues] reporting how many values it bound, so its
+// caller can discard them when the pattern turns out not to match.
+func setPathValues(dstPathVals []PathValue, pattern string, requestPath []byte) (n int, matched, pathValSliceTooShort bool) {
 	if len(pattern) == 0 || pattern[0] != '/' || len(requestPath) == 0 || requestPath[0] != '/' {
-		return false, false
+		return n, false, false
 	}
 	pattern, requestPath = pattern[1:], requestPath[1:]
-	n := 0
 	for {
 		if len(pattern) == 0 {
 			// Nothing left after a slash: an anonymous "..." taking the rest,
 			// which is why "/files/" matches "/files/a/b" and "/" matches all.
-			return true, false
+			return n, true, false
 		}
 		patSeg, patRest, patMore := strings.Cut(pattern, "/")
 		reqSeg, reqRest, reqMore := bytes.Cut(requestPath, pathSeparator)
@@ -143,40 +157,40 @@ func SetPathValues(dstPathVals []PathValue, pattern string, requestPath []byte)
 		case isWildcard && name == "$":
 			// Matches the end of the path and nothing else, so it must be the
 			// last segment of the pattern and leave no path behind.
-			return !patMore && len(requestPath) == 0, false
+			return n, !patMore && len(requestPath) == 0, false
 
 		case isWildcard && isMulti:
 			// Takes the remainder including slashes, possibly empty.
 			if name != "" {
 				if n >= len(dstPathVals) {
-					return false, true
+					return n, false, true
 				}
 				dstPathVals[n] = PathValue{Key: name, Value: requestPath}
 				n++
 			}
-			return true, false
+			return n, true, false
 
 		case isWildcard:
 			if len(reqSeg) == 0 {
-				return false, false // One segment means a non-empty one.
+				return n, false, false // One segment means a non-empty one.
 			}
 			if n >= len(dstPathVals) {
-				return false, true
+				return n, false, true
 			}
 			dstPathVals[n] = PathValue{Key: name, Value: reqSeg}
 			n++
 
 		default:
 			if b2s(reqSeg) != patSeg {
-				return false, false
+				return n, false, false
 			}
 		}
 		if patMore != reqMore {
 			// One side has a further segment and the other does not, so
 			// "/health" misses "/health/" and "/files/" misses "/files".
-			return false, false
+			return n, false, false
 		} else if !patMore {
-			return true, false // Both spent on the same segment.
+			return n, true, false // Both spent on the same segment.
 		}
 		pattern, requestPath = patRest, reqRest
 	}
@@ -221,8 +235,10 @@ func (sm *MuxSlice) LookupHandler(method Method, path []byte, dstPathVals []Path
 		if endpoint.method != MethUndefined && endpoint.method != method {
 			continue
 		}
-		// Method matches.
-		if endpoint.pathVals > 0 {
+		// 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 ok, _ := SetPathValues(dstPathVals, endpoint.path, path); ok {
 				return endpoint.path, endpoint.handler
 			}

http/httphi/mux_test.go

@@ -224,3 +224,87 @@ func TestExchangePathValueClearedBetweenRequests(t *testing.T) {
 		t.Errorf("want no path value on a literal route, got id=%q from the previous request", leaked)
 	}
 }
+
+// A lookup tries each endpoint in turn and [SetPathValues] binds as it walks, so
+// a pattern that binds values and then fails must not leave them behind for the
+// pattern that does match: a handler would read a wildcard no matched pattern has.
+func TestMuxSliceNoStaleBindingsAcrossCandidates(t *testing.T) {
+	var sm MuxSlice
+	sm.Reset(2)
+	sm.Handle("/a/{x}/{y}/z", func(ex *Exchange) { t.Error("non-matching handler ran") })
+	var gotP, gotX, gotY string
+	sm.Handle("/a/{p}/b", func(ex *Exchange) {
+		gotP = string(ex.PathValue("p"))
+		gotX = string(ex.PathValue("x"))
+		gotY = string(ex.PathValue("y"))
+		ex.WriteHeader(200)
+	})
+
+	exch := new(Exchange)
+	exch.Configure(ExchangeConfig{
+		RawBuf: make([]byte, 2048), RequestBufferLim: 1024,
+		NumHeaderKVCap: defaultNumHeaderKVCap, MaxPathValues: sm.MaxPathValues(),
+	})
+	conn := newConn("GET /a/1/b HTTP/1.1\r\nHost: h\r\n\r\n")
+	conn.Hangup()
+	if !exch.Acquire(conn) {
+		t.Fatal("fresh exchange failed to acquire")
+	}
+	if err := Handle(exch, &sm, nopBackoff); err != nil {
+		t.Fatal(err)
+	}
+	if gotP != "1" {
+		t.Errorf("want p=1 from the matched pattern, got %q", gotP)
+	}
+	if gotX != "" || gotY != "" {
+		t.Errorf("want no x/y from the failed candidate, got x=%q y=%q", gotX, gotY)
+	}
+}
+
+// A pattern ending in '/' carries no brace but is still a wildcard: the trailing
+// slash is an anonymous "{...}", see [SetPathValues]. MuxSlice must route it
+// through the same matcher rather than comparing the path literally.
+func TestMuxSliceTrailingSlashPattern(t *testing.T) {
+	for _, test := range []struct {
+		pattern string
+		path    string
+		want    bool
+	}{
+		{pattern: "/files/", path: "/files/a/b", want: true},
+		{pattern: "/files/", path: "/files/", want: true},
+		{pattern: "/files/", path: "/files", want: false},
+		{pattern: "/files/", path: "/other/a", want: false},
+		{pattern: "/", path: "/anything/at/all", want: true},
+		{pattern: "/", path: "/", want: true},
+		// Without the trailing slash a pattern stays literal.
+		{pattern: "/files", path: "/files/a", want: false},
+		{pattern: "/files", path: "/files", 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) })
+			// MuxSlice must agree with the matcher it delegates to.
+			if ok, _ := SetPathValues(nil, test.pattern, []byte(test.path)); ok != test.want {
+				t.Fatalf("SetPathValues disagrees with the table: got %v", ok)
+			}
+			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("fresh exchange failed to 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)
+			}
+		})
+	}
+}

http/httphi/router.go

@@ -184,7 +184,8 @@ func (r *Router) Configure(cfg RouterConfig) error {
 	r.respBuf = cfg.ResponseHeaderMinBufferSize
 	r.mux = cfg.Mux
 	r.log = cfg.Logger
-	r.maxPathValues = cfg.Mux.MaxPathValues()
+	maxPathValues := cfg.Mux.MaxPathValues()
+	r.maxPathValues = maxPathValues
 	r.normalizeKeys = cfg.NormalizeOutgoingKeys
 	// Freelist entries were sized by the outgoing configuration: recycling one
 	// would serve a request with buffer limits cfg never asked for.
@@ -208,7 +209,7 @@ func (r *Router) Configure(cfg RouterConfig) error {
 		r.exchs = r.exchs[:numgoro]
 		rawBuflen := cfg.RequestHeaderBufferSize + cfg.ResponseHeaderMinBufferSize
 		internal.SliceReuse(&r.globbuf, numgoro*rawBuflen)
-		maxPathValues := cfg.Mux.MaxPathValues()
+
 		for i := range numgoro {
 			// TODO exchange buffer alloc
 			goff := i * rawBuflen

http/httphi/router_test.go

@@ -247,7 +247,9 @@ func TestRouterMux(t *testing.T) {
 				sm     MuxSlice
 				router Router
 			)
-			sm.Handle("GET /", staticPage(t, "root"))
+			// "/{$}" is the root alone: a bare "/" is a catch-all and would serve
+			// "root" for /page and /nowhere as well, see [SetPathValues].
+			sm.Handle("GET /{$}", staticPage(t, "root"))
 			sm.Handle("GET /page", staticPage(t, "page"))
 			sm.Handle("/any", staticPage(t, "any")) // No method: matches any.
 			configSynchronousRouter(t, &router, bufferSize, &sm)