Deprecate out-of-bounds access possibility in safe code.

With the `id` and `tree` fields of `NodeRef` and `NodeMut` being public,
it was possible to assign to them.
For example, it was possible to build a `NodeMut`
for large ID/index in a small tree/Vec.
Since some APIs use unchecked indexing, this would let users of this library
cause out-of-bounds access in a `Vec` without writing `unsafe` code themselves.

This deprecates the fields,
and instead providing read-only access via accessor methods.

This lets dependent crates realize that there is a problem
they might need to fix.

Author: SimonSapin

Cargo.toml

@@ -1,6 +1,6 @@
 [package]
 name = "ego-tree"
-version = "0.5.0"
+version = "0.5.1"
 description = "Vec-backed ID-tree"
 keywords = ["tree", "vec", "id", "index"]
 authors = ["Curtis McEnroe <programble@gmail.com>"]

src/lib.rs

@@ -76,9 +76,11 @@ impl<T> Node<T> {
 #[derive(Debug)]
 pub struct NodeRef<'a, T: 'a> {
     /// Node ID.
+    #[deprecated(since = "0.5.1", note = "use the id() method instead and upgrade to 0.6")]
     pub id: NodeId,
 
     /// Tree containing the node.
+    #[deprecated(since = "0.5.1", note = "use the tree() method instead and upgrade to 0.6")]
     pub tree: &'a Tree<T>,
 
     node: &'a Node<T>,
@@ -88,9 +90,11 @@ pub struct NodeRef<'a, T: 'a> {
 #[derive(Debug)]
 pub struct NodeMut<'a, T: 'a> {
     /// Node ID.
+    #[deprecated(since = "0.5.1", note = "use the id() method instead and upgrade to 0.6")]
     pub id: NodeId,
 
     /// Tree containing the node.
+    #[deprecated(since = "0.5.1", note = "use the tree() method instead and upgrade to 0.6")]
     pub tree: &'a mut Tree<T>,
 }
 
@@ -103,13 +107,15 @@ impl<'a, T: 'a> Clone for NodeRef<'a, T> {
 
 impl<'a, T: 'a> Eq for NodeRef<'a, T> { }
 impl<'a, T: 'a> PartialEq for NodeRef<'a, T> {
+    #[allow(deprecated)]
     fn eq(&self, other: &Self) -> bool {
         self.id == other.id
             && self.tree as *const _ == other.tree as *const _
             && self.node as *const _ == other.node as *const _
     }
 }
 
+#[allow(deprecated)]
 impl<T> Tree<T> {
     /// Creates a tree with a root node.
     pub fn new(root: T) -> Self {
@@ -170,7 +176,18 @@ impl<T> Tree<T> {
     }
 }
 
+#[allow(deprecated)]
 impl<'a, T: 'a> NodeRef<'a, T> {
+    /// Returns the ID of this node.
+    pub fn id(&self) -> NodeId {
+        self.id
+    }
+
+    /// Returns the tree owning this node.
+    pub fn tree(&self) -> &'a Tree<T> {
+        self.tree
+    }
+
     /// Returns the value of this node.
     pub fn value(&self) -> &'a T {
         &self.node.value
@@ -212,7 +229,18 @@ impl<'a, T: 'a> NodeRef<'a, T> {
     }
 }
 
+#[allow(deprecated)]
 impl<'a, T: 'a> NodeMut<'a, T> {
+    /// Returns the ID of this node.
+    pub fn id(&self) -> NodeId {
+        self.id
+    }
+
+    /// Returns the tree owning this node.
+    pub fn tree(&mut self) -> &mut Tree<T> {
+        self.tree
+    }
+
     fn node(&mut self) -> &mut Node<T> {
         unsafe { self.tree.node_mut(self.id) }
     }
@@ -520,6 +548,7 @@ impl<'a, T: 'a> NodeMut<'a, T> {
     }
 }
 
+#[allow(deprecated)]
 impl<'a, T: 'a> From<NodeMut<'a, T>> for NodeRef<'a, T> {
     fn from(node: NodeMut<'a, T>) -> Self {
         unsafe { node.tree.get_unchecked(node.id) }

tests/node_mut.rs

@@ -298,7 +298,7 @@ fn reparent_from_id_append() {
             'e' => { 'f', 'g' },
         }
     };
-    let e_id = tree.root().last_child().unwrap().id;
+    let e_id = tree.root().last_child().unwrap().id();
     tree.root_mut().first_child().unwrap().reparent_from_id_append(e_id);
 
     let b = tree.root().first_child().unwrap();
@@ -322,7 +322,7 @@ fn reparent_from_id_prepend() {
             'e' => { 'c', 'd' },
         }
     };
-    let e_id = tree.root().last_child().unwrap().id;
+    let e_id = tree.root().last_child().unwrap().id();
     tree.root_mut().first_child().unwrap().reparent_from_id_prepend(e_id);
 
     let b = tree.root().first_child().unwrap();

tests/tree.rs

@@ -37,14 +37,14 @@ fn orphan() {
 #[test]
 fn get() {
     let tree = Tree::new('a');
-    let id = tree.root().id;
+    let id = tree.root().id();
     assert_eq!(Some(tree.root()), tree.get(id));
 }
 
 #[test]
 fn get_mut() {
     let mut tree = Tree::new('a');
-    let id = tree.root().id;
+    let id = tree.root().id();
     assert_eq!(Some('a'), tree.get_mut(id).map(|mut n| *n.value()));
 }