diff --git a/README.md b/README.md index eaeb6c38..4003665d 100644 --- a/README.md +++ b/README.md @@ -115,6 +115,12 @@ features = ["yaml_enc"] You can now use `rustbreak::deser::Yaml` as deserialization struct. +🔥🔥 +__Warning__: Using this deserializer can trigger *Undefined Behaviour (UB)*. It +is *strongly* recommended to *not* use this deserializer until these issues are +fixed. See [tracking issue #87] for more details. +🔥🔥 + ### Ron If you would like to use [`ron`](https://github.com/ron-rs/ron) you need to @@ -143,3 +149,4 @@ You can now use `rustbreak::deser::Bincode` as deserialization struct. [doc]:http://neikos.me/rustbreak/rustbreak/index.html [Daybreak]:https://propublica.github.io/daybreak/ +[tracking issue #87]: https://github.com/TheNeikos/rustbreak/issues/87 diff --git a/src/backend/path.rs b/src/backend/path.rs index 234a235c..01e81f4f 100644 --- a/src/backend/path.rs +++ b/src/backend/path.rs @@ -90,25 +90,29 @@ impl Backend for PathBackend { mod tests { use super::{Backend, PathBackend}; use std::io::Write; - use tempfile::NamedTempFile; + use std::path::Path; + use tempfile::{tempdir, NamedTempFile}; #[test] #[cfg_attr(miri, ignore)] fn test_path_backend_existing() { let file = NamedTempFile::new().expect("could not create temporary file"); - let (mut backend, existed) = PathBackend::from_path_or_create(file.path().to_owned()) - .expect("could not create backend"); + let tmppath = file.into_temp_path(); + let path: &Path = tmppath.as_ref(); + let (mut backend, existed) = + PathBackend::from_path_or_create(path.to_owned()).expect("could not create backend"); assert!(existed); let data = [4, 5, 1, 6, 8, 1]; backend.put_data(&data).expect("could not put data"); assert_eq!(backend.get_data().expect("could not get data"), data); + tmppath.close().expect("could not delete temp file"); } #[test] #[cfg_attr(miri, ignore)] fn test_path_backend_new() { - let dir = tempfile::tempdir().expect("could not create temporary directory"); + let dir = tempdir().expect("could not create temporary directory"); let mut file_path = dir.path().to_owned(); file_path.push("rustbreak_path_db.db"); let (mut backend, existed) = @@ -125,18 +129,20 @@ mod tests { #[cfg_attr(miri, ignore)] fn test_path_backend_nofail() { let file = NamedTempFile::new().expect("could not create temporary file"); - let file_path = file.path().to_owned(); - let mut backend = PathBackend::from_path_or_fail(file_path).expect("should not fail"); + let tmppath = file.into_temp_path(); + let path: &Path = tmppath.as_ref(); + let mut backend = PathBackend::from_path_or_fail(path.to_owned()).expect("should not fail"); let data = [4, 5, 1, 6, 8, 1]; backend.put_data(&data).expect("could not put data"); assert_eq!(backend.get_data().expect("could not get data"), data); + tmppath.close().expect("could not delete temp file"); } #[test] #[cfg_attr(miri, ignore)] fn test_path_backend_fail_notfound() { - let dir = tempfile::tempdir().expect("could not create temporary directory"); + let dir = tempdir().expect("could not create temporary directory"); let mut file_path = dir.path().to_owned(); file_path.push("rustbreak_path_db.db"); let err = @@ -154,7 +160,9 @@ mod tests { #[cfg_attr(miri, ignore)] fn test_path_backend_create_and_existing_nocall() { let file = NamedTempFile::new().expect("could not create temporary file"); - let mut backend = PathBackend::from_path_or_create_and(file.path().to_owned(), |_| { + let tmppath = file.into_temp_path(); + let path: &Path = tmppath.as_ref(); + let mut backend = PathBackend::from_path_or_create_and(path.to_owned(), |_| { panic!("Closure called but file already existed"); }) .expect("could not create backend"); @@ -162,13 +170,14 @@ mod tests { backend.put_data(&data).expect("could not put data"); assert_eq!(backend.get_data().expect("could not get data"), data); + tmppath.close().expect("could not delete temp file"); } // If the file does not yet exist, the closure should be called. #[test] #[cfg_attr(miri, ignore)] fn test_path_backend_create_and_new() { - let dir = tempfile::tempdir().expect("could not create temporary directory"); + let dir = tempdir().expect("could not create temporary directory"); let mut file_path = dir.path().to_owned(); file_path.push("rustbreak_path_db.db"); let mut backend = PathBackend::from_path_or_create_and(file_path, |f| { diff --git a/src/deser.rs b/src/deser.rs index b67787a9..5bdb4bf2 100644 --- a/src/deser.rs +++ b/src/deser.rs @@ -30,7 +30,6 @@ pub use self::bincode::Bincode; /// extern crate thiserror; /// extern crate serde; /// #[macro_use] -/// /// use serde::de::Deserialize; /// use serde::Serialize; /// use std::io::Read; @@ -69,7 +68,8 @@ pub use self::bincode::Bincode; /// fn main() {} /// ``` /// -/// **Important**: You can only return custom errors if the `other_errors` feature is enabled +/// **Important**: You can only return custom errors if the `other_errors` +/// feature is enabled pub trait DeSerializer: std::default::Default + Send + Sync + Clone { @@ -93,7 +93,7 @@ mod ron { use crate::deser::DeSerializer; use crate::error; - /// The Struct that allows you to use `ron` the Rusty Object Notation. + /// The Struct that allows you to use `ron`, the Rusty Object Notation. #[derive(Debug, Default, Clone)] pub struct Ron; @@ -118,7 +118,19 @@ mod yaml { use crate::deser::DeSerializer; use crate::error; - /// The struct that allows you to use yaml. + /// The struct that allows you to use yaml. 🔥 *do not use* 🔥 + /// + /// 🔥🔥 __Warning__: Using this [`DeSerializer`] can trigger *Undefined + /// Behaviour (UB)*. 🔥🔥 It is *strongly* recommended to *not* use this + /// [`DeSerializer`] until these issues are fixed. The UB is triggered + /// in a transitive dependency (namely [`linked_hash_map`]) of Rustbreak. + /// There is nothing the Rustbreak devs can do about this. + /// The UB is real and reachable. It triggered by the Rustbreak test suite, + /// and detected by `miri`. See the [tracking issue #87] for more details. + /// 🔥🔥 __DO NOT USE__ 🔥🔥 + /// + /// [`linked_hash_map`]: https://github.com/contain-rs/linked-hash-map + /// [tracking issue #87]: https://github.com/TheNeikos/rustbreak/issues/87 #[derive(Debug, Default, Clone)] pub struct Yaml; diff --git a/src/lib.rs b/src/lib.rs index 02f66730..a87e6e09 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -328,11 +328,7 @@ where /// # extern crate rustbreak; /// # extern crate serde; /// # extern crate tempfile; - /// use rustbreak::{ - /// deser::Ron, - /// error::RustbreakError, - /// FileDatabase, - /// }; + /// use rustbreak::{deser::Ron, error::RustbreakError, FileDatabase}; /// /// #[derive(Debug, Serialize, Deserialize, Clone)] /// struct Data { @@ -1010,6 +1006,7 @@ where mod tests { use super::*; use std::collections::HashMap; + use std::path::Path; use tempfile::NamedTempFile; type TestData = HashMap; @@ -1448,16 +1445,19 @@ mod tests { #[cfg_attr(miri, ignore)] fn pathdb_from_path_existing() { let file = NamedTempFile::new().expect("could not create temporary file"); - let path = file.path().to_owned(); + let tmppath = file.into_temp_path(); + let path: &Path = tmppath.as_ref(); // initialise the file - let db = TestDb::::create_at_path(path.clone(), test_data()) + let db = TestDb::::create_at_path(path.to_owned(), test_data()) .expect("could not create db"); db.save().expect("could not save db"); drop(db); // test that loading now works - let db = TestDb::::load_from_path(path).expect("could not load"); + let db = TestDb::::load_from_path(path.to_owned()).expect("could not load"); let readlock = db.borrow_data().expect("Rustbreak readlock error"); assert_eq!(test_data(), *readlock); + + tmppath.close().expect("could not delete temp file"); } #[test] diff --git a/tests/integration_tests.rs b/tests/integration_tests.rs index a092c22d..6442a919 100644 --- a/tests/integration_tests.rs +++ b/tests/integration_tests.rs @@ -123,10 +123,12 @@ fn create_mmapdb_with_size + Debug>(size: usize) -> MmapDa MmapDatabase::mmap_with_size(Data::default(), size).expect("could not create database") } -fn create_pathdb + Debug>() -> PathDatabase { +fn create_pathdb + Debug>() -> (PathDatabase, tempfile::TempPath) { let file = tempfile::NamedTempFile::new().expect("could not create temporary file"); - PathDatabase::create_at_path(file.path().to_owned(), Data::default()) - .expect("could not create database") + let db = PathDatabase::create_at_path(file.path().to_owned(), Data::default()) + .expect("could not create database"); + + (db, file.into_temp_path()) } macro_rules! test_basic_save_load { @@ -151,6 +153,19 @@ macro_rules! test_basic_save_load { test_convert_data(db); } }; + ($name:ident, $db:expr, $enc:ty, setupenv=true) => { + #[test] + #[cfg_attr(miri, ignore)] + fn $name() { + let env = $db; + let db: Database = env.0; + test_basic_save_load(&db); + test_put_data(&db); + test_multi_borrow(&db); + test_convert_data(db); + env.1.close().expect("Could not close environment"); + } + }; } test_basic_save_load!(file_ron, create_filedb(), Ron); @@ -173,6 +188,6 @@ test_basic_save_load!(mmapsize_ron, create_mmapdb_with_size(10), Ron); test_basic_save_load!(mmapsize_yaml, create_mmapdb_with_size(10), Yaml); test_basic_save_load!(mmapsize_bincode, create_mmapdb_with_size(10), Bincode); -test_basic_save_load!(path_ron, create_pathdb(), Ron); -test_basic_save_load!(path_yaml, create_pathdb(), Yaml); -test_basic_save_load!(path_bincode, create_pathdb(), Bincode); +test_basic_save_load!(path_ron, create_pathdb(), Ron, setupenv = true); +test_basic_save_load!(path_yaml, create_pathdb(), Yaml, setupenv = true); +test_basic_save_load!(path_bincode, create_pathdb(), Bincode, setupenv = true);