From 298f369dececfacdc954d8bcee84860de0f2afb2 Mon Sep 17 00:00:00 2001 From: Wisdom Ogwu Date: Mon, 15 Nov 2021 17:18:25 +0100 Subject: [PATCH 1/6] Added new error for cyclic paths --- grovedb/src/lib.rs | 2 ++ 1 file changed, 2 insertions(+) diff --git a/grovedb/src/lib.rs b/grovedb/src/lib.rs index ccae4ec83..649183a39 100644 --- a/grovedb/src/lib.rs +++ b/grovedb/src/lib.rs @@ -30,6 +30,8 @@ pub enum Error { InvalidPath(&'static str), #[error("unable to decode")] EdError(#[from] ed::Error), + #[error("cyclic reference path")] + CyclicReferencePath, } impl From for Error { From a654ecbcaa6d8f71b7813fed87285e5fec15a4f8 Mon Sep 17 00:00:00 2001 From: Wisdom Ogwu Date: Mon, 15 Nov 2021 17:22:07 +0100 Subject: [PATCH 2/6] Updated follow reference function to use path vector --- grovedb/src/subtree.rs | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-) diff --git a/grovedb/src/subtree.rs b/grovedb/src/subtree.rs index d4237b49b..09be7f45e 100644 --- a/grovedb/src/subtree.rs +++ b/grovedb/src/subtree.rs @@ -29,14 +29,21 @@ impl Element { } /// Recursively follow `Element::Reference` - fn follow_reference(self, merk: &Merk) -> Result { + fn follow_reference(self, merk: &Merk, paths: &mut Vec>) -> Result { if let Element::Reference(reference_merk_key) = self { + // Check if the reference merk key has been visited before + // if it has then we have a cycle + if paths.contains(&reference_merk_key) { + return Err(Error::CyclicReferencePath); + } let element = Element::decode( merk.get(reference_merk_key.as_slice())? .ok_or(Error::InvalidPath("key not found in Merk"))? .as_slice(), )?; - element.follow_reference(merk) + + paths.push(reference_merk_key); + element.follow_reference(merk, paths) } else { Ok(self) } From 8b9e4fe55aade7d6e80cef0f390f88d00a81ec11 Mon Sep 17 00:00:00 2001 From: Wisdom Ogwu Date: Mon, 15 Nov 2021 17:24:07 +0100 Subject: [PATCH 3/6] Updated the get function to use the new reference function signature --- grovedb/src/subtree.rs | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/grovedb/src/subtree.rs b/grovedb/src/subtree.rs index 09be7f45e..575eeedad 100644 --- a/grovedb/src/subtree.rs +++ b/grovedb/src/subtree.rs @@ -79,7 +79,9 @@ impl Element { .ok_or(Error::InvalidPath("key not found in Merk"))? .as_slice(), )?; - element.follow_reference(&merk) + + let mut reference_paths: Vec> = Vec::new(); + element.follow_reference(&merk, &mut reference_paths) } pub fn insert(&self, merk: &mut Merk, path: &[&[u8]], key: &[u8]) -> Result<(), Error> { From 5bf9b4c9b1fbdcccb5a9de012ebe5a8ef293096e Mon Sep 17 00:00:00 2001 From: Wisdom Ogwu Date: Mon, 15 Nov 2021 17:39:14 +0100 Subject: [PATCH 4/6] Ignoring grove.db --- .gitignore | 1 + grovedb/src/subtree.rs | 21 +++++++++++++++++++++ 2 files changed, 22 insertions(+) diff --git a/.gitignore b/.gitignore index 8ff0eb369..4b1aa93d0 100644 --- a/.gitignore +++ b/.gitignore @@ -1,3 +1,4 @@ /target .idea Cargo.lock +grove.db \ No newline at end of file diff --git a/grovedb/src/subtree.rs b/grovedb/src/subtree.rs index 575eeedad..06369efc1 100644 --- a/grovedb/src/subtree.rs +++ b/grovedb/src/subtree.rs @@ -150,4 +150,25 @@ mod tests { Element::Item(b"value".to_vec()), ); } + + #[test] + fn test_circular_references() { + let tmp_dir = TempDir::new("db").unwrap(); + let mut merk = Merk::open(tmp_dir.path()).unwrap(); + + Element::Tree + .insert(&mut merk, &[], b"tree-key") + .expect("expected successful insertion"); + // Create reference-1 that points to reference-2 + // TODO: Look into preventing invalid reference (references must point to + // something that exists) + Element::new_reference(&[b"tree-key"], b"reference-2") + .insert(&mut merk, &[b"tree-key"], b"reference-1") + .expect("expected successful reference insertion"); + Element::new_reference(&[b"tree-key"], b"reference-1") + .insert(&mut merk, &[b"tree-key"], b"reference-2") + .expect("expected successful reference insertion"); + + assert!(Element::get(&merk, &[b"tree-key"], b"reference-1").is_err()); + } } From b52a61142bf707d75f32fa1330f5352905bac8c4 Mon Sep 17 00:00:00 2001 From: Wisdom Ogwu Date: Mon, 15 Nov 2021 17:41:17 +0100 Subject: [PATCH 5/6] Updated comments --- grovedb/src/subtree.rs | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/grovedb/src/subtree.rs b/grovedb/src/subtree.rs index 06369efc1..57406c80a 100644 --- a/grovedb/src/subtree.rs +++ b/grovedb/src/subtree.rs @@ -159,9 +159,8 @@ mod tests { Element::Tree .insert(&mut merk, &[], b"tree-key") .expect("expected successful insertion"); - // Create reference-1 that points to reference-2 - // TODO: Look into preventing invalid reference (references must point to - // something that exists) + + // r1 points to r2 and r2 points to r1 (cycle!) Element::new_reference(&[b"tree-key"], b"reference-2") .insert(&mut merk, &[b"tree-key"], b"reference-1") .expect("expected successful reference insertion"); From 9b9cbb5779433158af64b9cc480fed8fcabd9d29 Mon Sep 17 00:00:00 2001 From: Wisdom Ogwu Date: Mon, 15 Nov 2021 18:58:22 +0100 Subject: [PATCH 6/6] modified follow reference to not expose path implementation --- grovedb/src/subtree.rs | 44 +++++++++++++++++++++++++----------------- 1 file changed, 26 insertions(+), 18 deletions(-) diff --git a/grovedb/src/subtree.rs b/grovedb/src/subtree.rs index 57406c80a..bc6521a9d 100644 --- a/grovedb/src/subtree.rs +++ b/grovedb/src/subtree.rs @@ -29,24 +29,33 @@ impl Element { } /// Recursively follow `Element::Reference` - fn follow_reference(self, merk: &Merk, paths: &mut Vec>) -> Result { - if let Element::Reference(reference_merk_key) = self { - // Check if the reference merk key has been visited before - // if it has then we have a cycle - if paths.contains(&reference_merk_key) { - return Err(Error::CyclicReferencePath); + fn follow_reference(self, merk: &Merk) -> Result { + fn follow_reference_with_path( + element: Element, + merk: &Merk, + paths: &mut Vec>, + ) -> Result { + if let Element::Reference(reference_merk_key) = element { + // Check if the reference merk key has been visited before + // if it has then we have a cycle + if paths.contains(&reference_merk_key) { + return Err(Error::CyclicReferencePath); + } + let element = Element::decode( + merk.get(reference_merk_key.as_slice())? + .ok_or(Error::InvalidPath("key not found in Merk"))? + .as_slice(), + )?; + + paths.push(reference_merk_key); + follow_reference_with_path(element, merk, paths) + } else { + Ok(element) } - let element = Element::decode( - merk.get(reference_merk_key.as_slice())? - .ok_or(Error::InvalidPath("key not found in Merk"))? - .as_slice(), - )?; - - paths.push(reference_merk_key); - element.follow_reference(merk, paths) - } else { - Ok(self) } + + let mut reference_paths: Vec> = Vec::new(); + follow_reference_with_path(self, merk, &mut reference_paths) } /// A helper method to build Merk keys (and RocksDB as well) out of path + @@ -80,8 +89,7 @@ impl Element { .as_slice(), )?; - let mut reference_paths: Vec> = Vec::new(); - element.follow_reference(&merk, &mut reference_paths) + element.follow_reference(&merk) } pub fn insert(&self, merk: &mut Merk, path: &[&[u8]], key: &[u8]) -> Result<(), Error> {