From 4e030f72d49326a5b1e4f5bb78e42cd88a7340e2 Mon Sep 17 00:00:00 2001
From: Adam Reichold <adam.reichold@t-online.de>
Date: Thu, 28 Nov 2024 21:03:45 +0100
Subject: [PATCH] Prevent usage of self ID in modifiers working on bare ID

This allows the construction of otherwise nonsensical trees
which might lead to unexpected panics further down the road.
---
 src/lib.rs        | 54 ++++++++++++++++++++++++++++++++++++++---------
 src/serde.rs      |  2 +-
 tests/node_mut.rs | 28 ++++++++++++++++++++++++
 3 files changed, 73 insertions(+), 11 deletions(-)

diff --git a/src/lib.rs b/src/lib.rs
index ee5636d..fe566cf 100644
--- a/src/lib.rs
+++ b/src/lib.rs
@@ -494,6 +494,12 @@ impl<'a, T: 'a> NodeMut<'a, T> {
     ///
     /// Panics if `new_child_id` is not valid.
     pub fn append_id(&mut self, new_child_id: NodeId) -> NodeMut<T> {
+        assert_ne!(
+            self.id(),
+            new_child_id,
+            "Cannot append node as a child to itself"
+        );
+
         let last_child_id = self.node().children.map(|(_, id)| id);
         {
             let mut new_child = self.tree.get_mut(new_child_id).unwrap();
@@ -509,11 +515,10 @@ impl<'a, T: 'a> NodeMut<'a, T> {
         }
 
         {
-            if let Some((first_child_id, _)) = self.node().children {
-                self.node().children = Some((first_child_id, new_child_id));
-            } else {
-                self.node().children = Some((new_child_id, new_child_id));
-            }
+            self.node().children = match self.node().children {
+                Some((first_child_id, _)) => Some((first_child_id, new_child_id)),
+                None => Some((new_child_id, new_child_id)),
+            };
         }
 
         unsafe { self.tree.get_unchecked_mut(new_child_id) }
@@ -525,6 +530,12 @@ impl<'a, T: 'a> NodeMut<'a, T> {
     ///
     /// Panics if `new_child_id` is not valid.
     pub fn prepend_id(&mut self, new_child_id: NodeId) -> NodeMut<T> {
+        assert_ne!(
+            self.id(),
+            new_child_id,
+            "Cannot prepend node as a child to itself"
+        );
+
         let first_child_id = self.node().children.map(|(id, _)| id);
         {
             let mut new_child = self.tree.get_mut(new_child_id).unwrap();
@@ -540,11 +551,10 @@ impl<'a, T: 'a> NodeMut<'a, T> {
         }
 
         {
-            if let Some((_, last_child_id)) = self.node().children {
-                self.node().children = Some((new_child_id, last_child_id));
-            } else {
-                self.node().children = Some((new_child_id, new_child_id));
-            }
+            self.node().children = match self.node().children {
+                Some((_, last_child_id)) => Some((new_child_id, last_child_id)),
+                None => Some((new_child_id, new_child_id)),
+            };
         }
 
         unsafe { self.tree.get_unchecked_mut(new_child_id) }
@@ -557,6 +567,12 @@ impl<'a, T: 'a> NodeMut<'a, T> {
     /// - Panics if `new_sibling_id` is not valid.
     /// - Panics if this node is an orphan.
     pub fn insert_id_before(&mut self, new_sibling_id: NodeId) -> NodeMut<T> {
+        assert_ne!(
+            self.id(),
+            new_sibling_id,
+            "Cannot insert node as a sibling of itself"
+        );
+
         let parent_id = self.node().parent.unwrap();
         let prev_sibling_id = self.node().prev_sibling;
 
@@ -594,6 +610,12 @@ impl<'a, T: 'a> NodeMut<'a, T> {
     /// - Panics if `new_sibling_id` is not valid.
     /// - Panics if this node is an orphan.
     pub fn insert_id_after(&mut self, new_sibling_id: NodeId) -> NodeMut<T> {
+        assert_ne!(
+            self.id(),
+            new_sibling_id,
+            "Cannot insert node as a sibling of itself"
+        );
+
         let parent_id = self.node().parent.unwrap();
         let next_sibling_id = self.node().next_sibling;
 
@@ -630,6 +652,12 @@ impl<'a, T: 'a> NodeMut<'a, T> {
     ///
     /// Panics if `from_id` is not valid.
     pub fn reparent_from_id_append(&mut self, from_id: NodeId) {
+        assert_ne!(
+            self.id(),
+            from_id,
+            "Cannot reparent node's children to itself"
+        );
+
         let new_child_ids = {
             let mut from = self.tree.get_mut(from_id).unwrap();
             match from.node().children.take() {
@@ -663,6 +691,12 @@ impl<'a, T: 'a> NodeMut<'a, T> {
     ///
     /// Panics if `from_id` is not valid.
     pub fn reparent_from_id_prepend(&mut self, from_id: NodeId) {
+        assert_ne!(
+            self.id(),
+            from_id,
+            "Cannot reparent node's children to itself"
+        );
+
         let new_child_ids = {
             let mut from = self.tree.get_mut(from_id).unwrap();
             match from.node().children.take() {
diff --git a/src/serde.rs b/src/serde.rs
index 137c2ad..4fed0b3 100644
--- a/src/serde.rs
+++ b/src/serde.rs
@@ -28,7 +28,7 @@ impl<'a, T> From<NodeRef<'a, T>> for SerNode<'a, T> {
     }
 }
 
-impl<'a, T: Serialize> Serialize for SerNode<'a, T> {
+impl<T: Serialize> Serialize for SerNode<'_, T> {
     fn serialize<S>(&self, serializer: S) -> Result<S::Ok, S::Error>
     where
         S: serde::Serializer,
diff --git a/tests/node_mut.rs b/tests/node_mut.rs
index f2e8c63..e3a9575 100644
--- a/tests/node_mut.rs
+++ b/tests/node_mut.rs
@@ -311,6 +311,34 @@ fn detach() {
     assert!(c.next_sibling().is_none());
 }
 
+#[test]
+#[should_panic(expected = "Cannot append node as a child to itself")]
+fn append_id_itself() {
+    let mut tree = tree! {
+        'a' => {
+            'b' => { 'c', 'd' },
+            'e' => { 'f', 'g' },
+        }
+    };
+    let mut a = tree.root_mut();
+    let mut e = a.last_child().unwrap();
+    e.append_id(e.id());
+}
+
+#[test]
+#[should_panic(expected = "Cannot prepend node as a child to itself")]
+fn prepend_id_itself() {
+    let mut tree = tree! {
+        'a' => {
+            'b' => { 'c', 'd' },
+            'e' => { 'f', 'g' },
+        }
+    };
+    let mut a = tree.root_mut();
+    let mut e = a.last_child().unwrap();
+    e.prepend_id(e.id());
+}
+
 #[test]
 fn reparent_from_id_append() {
     let mut tree = tree! {