Reject non-canonical trailing-dot tree paths in resolveChildPath

jawstree/node.go

@@ -136,26 +136,35 @@ func (node *Node) JawsSetPath(elem *jaws.Element, jsPath string, value any) (err
 // nil-targeting segments are rejected, so a client can neither grow the slice nor
 // address a node that does not exist.
 //
-// Indices must be in canonical decimal form (no leading '+', '-' or zeros) so the
-// index a client sends round-trips to the same string [Node.Walk] emits as the
-// node ID. A non-canonical but in-range index would mutate the server node yet be
-// echoed verbatim as the [Node.JawsPathSet] broadcast "id", which no peer's
-// rendered node matches, diverging peer state from the server.
+// The whole path must be in canonical form so that it round-trips to the exact
+// string [Node.Walk] emits as the node ID: segments strictly alternate "children"
+// and a canonical decimal index (no leading '+', '-' or zeros), joined by single
+// dots with no empty segment from a leading, trailing or doubled '.'. A path that
+// resolves to a valid in-range node but is not canonical — for example a trailing
+// dot ("children.0.") or a non-canonical index ("children.+0") — would mutate the
+// server node yet be echoed verbatim as the [Node.JawsPathSet] broadcast "id",
+// which no peer's rendered node matches, diverging peer state from the server.
+//
+// Each index is checked against its canonical form (strconv.Itoa is allocation-free
+// for indices below 100). The trailing-dot case is the one a per-index check alone
+// misses: it leaves an empty final segment the pair-wise loop never visits, so we
+// reject when Cut reports a separator was consumed ('more') but nothing follows it.
 func (node *Node) resolveChildPath(nodePath string) (*Node, error) {
 	cur := node
-	for nodePath != "" {
+	for rest := nodePath; rest != ""; {
 		var seg, idxStr string
-		seg, nodePath, _ = strings.Cut(nodePath, ".")
+		var more bool
+		seg, rest, _ = strings.Cut(rest, ".")
 		if seg != "children" {
 			return nil, fmt.Errorf("%w: unexpected path segment %q", ErrPathRejected, seg)
 		}
-		idxStr, nodePath, _ = strings.Cut(nodePath, ".")
+		idxStr, rest, more = strings.Cut(rest, ".")
 		idx, err := strconv.Atoi(idxStr)
 		if err != nil || idx < 0 || idx >= len(cur.Children) || cur.Children[idx] == nil {
 			return nil, fmt.Errorf("%w: child index %q out of range", ErrPathRejected, idxStr)
 		}
-		if strconv.Itoa(idx) != idxStr {
-			return nil, fmt.Errorf("%w: non-canonical child index %q", ErrPathRejected, idxStr)
+		if strconv.Itoa(idx) != idxStr || (more && rest == "") {
+			return nil, fmt.Errorf("%w: non-canonical path %q", ErrPathRejected, nodePath)
 		}
 		cur = cur.Children[idx]
 	}

jawstree/node_benchmark_test.go

@@ -0,0 +1,41 @@
+package jawstree
+
+import "testing"
+
+var resolveChildPathBenchSink *Node
+
+// BenchmarkResolveChildPath guards the canonical child-path resolver against
+// regression across shallow, deep and rejected (trailing-dot) paths. The resolver
+// runs on every inbound tree-selection event, so it must stay allocation-free on
+// the accepted paths.
+func BenchmarkResolveChildPath(b *testing.B) {
+	// Build a 3-deep tree so the deep path resolves.
+	leaf := func(n string) *Node { return &Node{Name: n} }
+	root := &Node{Name: "root", Children: []*Node{
+		{Name: "a", Children: []*Node{
+			leaf("a0"),
+			{Name: "a1", Children: []*Node{leaf("a1.0"), leaf("a1.1"), leaf("a1.2")}},
+		}},
+		leaf("b"),
+	}}
+
+	cases := []struct {
+		name string
+		path string
+	}{
+		{"shallow", "children.1"},
+		{"deep", "children.0.children.1.children.2"},
+		{"rejected", "children.0.children.1.children.2."}, // trailing dot
+	}
+
+	for _, c := range cases {
+		b.Run(c.name, func(b *testing.B) {
+			b.ReportAllocs()
+			b.ResetTimer()
+			for i := 0; i < b.N; i++ {
+				n, _ := root.resolveChildPath(c.path)
+				resolveChildPathBenchSink = n
+			}
+		})
+	}
+}

jawstree/node_test.go

@@ -148,6 +148,49 @@ func TestNode_JawsSetPath_Gate(t *testing.T) {
 			}
 		}
 	})
+
+	// A non-canonical path that still resolves to a valid in-range node (an empty
+	// trailing/leading/embedded segment) must be rejected. Such a path otherwise
+	// mutates the server node while JawsPathSet echoes the non-canonical path
+	// verbatim as the broadcast id, which no peer's rendered (canonical) node id
+	// matches, silently desyncing peer selection. "children.0..selected" is the
+	// regression: it resolves to Children[0] yet is not the canonical
+	// "children.0.selected" that Node.Walk emits.
+	t.Run("rejects non-canonical paths resolving to a valid node", func(t *testing.T) {
+		for _, path := range []string{
+			"children.0..selected", // trailing dot -> nodePath "children.0."
+			".children.0.selected", // leading empty segment
+			"children..0.selected", // embedded empty segment
+		} {
+			root := newTree()
+			if err := root.JawsSetPath(nil, path, true); !errors.Is(err, jawstree.ErrPathRejected) {
+				t.Errorf("path %q: expected ErrPathRejected, got %v", path, err)
+			}
+			if root.Children[0].Selected {
+				t.Errorf("path %q: Children[0].Selected was mutated despite rejection", path)
+			}
+		}
+	})
+
+	// The fix must not reject legitimate canonical paths, including nested ones.
+	t.Run("accepts canonical paths including nested", func(t *testing.T) {
+		root := &jawstree.Node{Name: "root", Children: []*jawstree.Node{
+			{Name: "a", Children: []*jawstree.Node{{Name: "a0"}, {Name: "a1"}}},
+			{Name: "b"},
+		}}
+		if err := root.JawsSetPath(nil, "children.0.children.1.selected", true); err != nil {
+			t.Fatalf("nested canonical path: %v", err)
+		}
+		if !root.Children[0].Children[1].Selected {
+			t.Error("nested node was not selected")
+		}
+		if err := root.JawsSetPath(nil, "children.1.selected", true); err != nil {
+			t.Fatalf("canonical path: %v", err)
+		}
+		if !root.Children[1].Selected {
+			t.Error("node was not selected")
+		}
+	})
 }
 
 // TestNode_NilChildGuards exercises the defensive nil-child guards in marshalJSON