Skip to content

Commit

Permalink
Prevent usage of self ID in modifiers working on bare ID
Browse files Browse the repository at this point in the history
This allows the construction of otherwise nonsensical trees
which might lead to unexpected panics further down the road.
  • Loading branch information
adamreichold committed Nov 28, 2024
1 parent 95aaf8b commit 4e030f7
Show file tree
Hide file tree
Showing 3 changed files with 73 additions and 11 deletions.
54 changes: 44 additions & 10 deletions src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand All @@ -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) }
Expand All @@ -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();
Expand All @@ -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) }
Expand All @@ -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;

Expand Down Expand Up @@ -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;

Expand Down Expand Up @@ -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() {
Expand Down Expand Up @@ -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() {
Expand Down
2 changes: 1 addition & 1 deletion src/serde.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
28 changes: 28 additions & 0 deletions tests/node_mut.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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! {
Expand Down

0 comments on commit 4e030f7

Please sign in to comment.