Merge pull request #70 from sval-rs/fix/panic-owned-buffer

Avoid panicking when buffering values
sval-rs · Oct 19, 2023 · de775bb · de775bb
2 parents 38ce845 + 7ec9d8f
commit de775bb
Show file tree

Hide file tree

Showing 9 changed files with 167 additions and 15 deletions.
diff --git a/src/internal/cast/mod.rs b/src/internal/cast/mod.rs
@@ -221,6 +221,11 @@ impl<'v> Internal<'v> {
             fn serde1(&mut self, v: &dyn super::serde::v1::Serialize) -> Result<(), Error> {
                 super::serde::v1::internal_visit(v, self)
             }
+
+            fn poisoned(&mut self, _: &'static str) -> Result<(), Error> {
+                self.0 = Cast::None;
+                Ok(())
+            }
         }
 
         match &self {

diff --git a/src/internal/fmt.rs b/src/internal/fmt.rs
@@ -217,6 +217,12 @@ impl<'v> Debug for ValueBag<'v> {
             ) -> Result<(), Error> {
                 crate::internal::serde::v1::fmt(self.0, v)
             }
+
+            fn poisoned(&mut self, msg: &'static str) -> Result<(), Error> {
+                write!(self.0, "<{msg}>")?;
+
+                Ok(())
+            }
         }
 
         self.internal_visit(&mut DebugVisitor(f))
@@ -314,6 +320,12 @@ impl<'v> Display for ValueBag<'v> {
             ) -> Result<(), Error> {
                 crate::internal::serde::v1::fmt(self.0, v)
             }
+
+            fn poisoned(&mut self, msg: &'static str) -> Result<(), Error> {
+                write!(self.0, "<{msg}>")?;
+
+                Ok(())
+            }
         }
 
         self.internal_visit(&mut DisplayVisitor(f))

diff --git a/src/internal/mod.rs b/src/internal/mod.rs
@@ -85,6 +85,10 @@ pub(crate) enum Internal<'v> {
     #[cfg(feature = "serde1")]
     /// A structured value from `serde`.
     Serde1(&'v dyn serde::v1::DowncastSerialize),
+
+    /// A poisoned value.
+    #[cfg_attr(not(feature = "owned"), allow(dead_code))]
+    Poisoned(&'static str),
 }
 
 /// The internal serialization contract.
@@ -139,6 +143,8 @@ pub(crate) trait InternalVisitor<'v> {
     fn borrowed_serde1(&mut self, v: &'v dyn serde::v1::Serialize) -> Result<(), Error> {
         self.serde1(v)
     }
+
+    fn poisoned(&mut self, msg: &'static str) -> Result<(), Error>;
 }
 
 impl<'a, 'v, V: InternalVisitor<'v> + ?Sized> InternalVisitor<'v> for &'a mut V {
@@ -235,6 +241,10 @@ impl<'a, 'v, V: InternalVisitor<'v> + ?Sized> InternalVisitor<'v> for &'a mut V
     fn borrowed_serde1(&mut self, v: &'v dyn serde::v1::Serialize) -> Result<(), Error> {
         (**self).borrowed_serde1(v)
     }
+
+    fn poisoned(&mut self, msg: &'static str) -> Result<(), Error> {
+        (**self).poisoned(msg)
+    }
 }
 
 impl<'v> ValueBag<'v> {
@@ -291,6 +301,8 @@ impl<'v> Internal<'v> {
             Internal::AnonSerde1(value) => Internal::AnonSerde1(*value),
             #[cfg(feature = "serde1")]
             Internal::Serde1(value) => Internal::Serde1(*value),
+
+            Internal::Poisoned(msg) => Internal::Poisoned(msg),
         }
     }
 
@@ -332,6 +344,8 @@ impl<'v> Internal<'v> {
             Internal::AnonSerde1(value) => visitor.borrowed_serde1(*value),
             #[cfg(feature = "serde1")]
             Internal::Serde1(value) => visitor.borrowed_serde1(value.as_super()),
+
+            Internal::Poisoned(msg) => visitor.poisoned(*msg),
         }
     }
 }

diff --git a/src/internal/owned.rs b/src/internal/owned.rs
@@ -34,6 +34,9 @@ pub(crate) enum OwnedInternal {
 
     #[cfg(feature = "sval2")]
     Sval2(internal::sval::v2::owned::OwnedValue),
+
+    /// A poisoned value.
+    Poisoned(&'static str),
 }
 
 impl OwnedInternal {
@@ -64,6 +67,8 @@ impl OwnedInternal {
 
             #[cfg(feature = "sval2")]
             OwnedInternal::Sval2(v) => Internal::AnonSval2(v),
+
+            OwnedInternal::Poisoned(msg) => Internal::Poisoned(msg),
         }
     }
 }
@@ -136,13 +141,22 @@ impl<'v> Internal<'v> {
 
             #[cfg(feature = "sval2")]
             fn sval2(&mut self, v: &dyn internal::sval::v2::Value) -> Result<(), Error> {
-                self.0 = OwnedInternal::Sval2(internal::sval::v2::owned::buffer(v));
+                self.0 = internal::sval::v2::owned::buffer(v)
+                    .map(OwnedInternal::Sval2)
+                    .unwrap_or(OwnedInternal::Poisoned("failed to buffer the value"));
                 Ok(())
             }
 
             #[cfg(feature = "serde1")]
             fn serde1(&mut self, v: &dyn internal::serde::v1::Serialize) -> Result<(), Error> {
-                self.0 = OwnedInternal::Serde1(internal::serde::v1::owned::buffer(v));
+                self.0 = internal::serde::v1::owned::buffer(v)
+                    .map(OwnedInternal::Serde1)
+                    .unwrap_or(OwnedInternal::Poisoned("failed to buffer the value"));
+                Ok(())
+            }
+
+            fn poisoned(&mut self, msg: &'static str) -> Result<(), Error> {
+                self.0 = OwnedInternal::Poisoned(msg);
                 Ok(())
             }
         }

diff --git a/src/internal/serde/v1.rs b/src/internal/serde/v1.rs
@@ -186,6 +186,11 @@ impl<'v> value_bag_serde1::lib::Serialize for ValueBag<'v> {
                 self.result = Some(value_bag_serde1::erased::serialize(v, self.serializer()?));
                 self.result()
             }
+
+            fn poisoned(&mut self, msg: &'static str) -> Result<(), Error> {
+                self.result = Some(Err(S::Error::custom(msg)));
+                self.result()
+            }
         }
 
         let mut visitor = Serde1Visitor {
@@ -448,8 +453,10 @@ pub(crate) mod owned {
 
     pub(crate) type OwnedSerialize = Box<value_bag_serde1::buf::Owned>;
 
-    pub(crate) fn buffer(v: impl value_bag_serde1::lib::Serialize) -> OwnedSerialize {
-        Box::new(value_bag_serde1::buf::Owned::buffer(v).unwrap())
+    pub(crate) fn buffer(
+        v: impl value_bag_serde1::lib::Serialize,
+    ) -> Result<OwnedSerialize, value_bag_serde1::buf::Error> {
+        value_bag_serde1::buf::Owned::buffer(v).map(Box::new)
     }
 }
 
@@ -631,4 +638,34 @@ mod tests {
             );
         }
     }
+
+    #[cfg(feature = "owned")]
+    mod owned_support {
+        use super::*;
+
+        #[test]
+        #[cfg_attr(target_arch = "wasm32", wasm_bindgen_test)]
+        fn serde1_to_owned_poison() {
+            struct Kaboom;
+
+            impl value_bag_serde1::lib::Serialize for Kaboom {
+                fn serialize<S>(&self, _: S) -> Result<S::Ok, S::Error>
+                where
+                    S: value_bag_serde1::lib::Serializer,
+                {
+                    Err(S::Error::custom("kaboom"))
+                }
+            }
+
+            let value = ValueBag::capture_serde1(&Kaboom)
+                .to_owned()
+                .by_ref()
+                .to_test_token();
+
+            assert_eq!(
+                TestToken::Poisoned("failed to buffer the value".into()),
+                value
+            );
+        }
+    }
 }
diff --git a/src/internal/sval/v2.rs b/src/internal/sval/v2.rs
@@ -162,6 +162,10 @@ impl<'sval> value_bag_sval2::lib_ref::ValueRef<'sval> for ValueBag<'sval> {
             ) -> Result<(), Error> {
                 crate::internal::serde::v1::sval2(self.0, v)
             }
+
+            fn poisoned(&mut self, msg: &'static str) -> Result<(), Error> {
+                Err(Error::msg(msg))
+            }
         }
 
         self.internal_visit(&mut Sval2Visitor(s))
@@ -315,8 +319,10 @@ pub(crate) mod owned {
 
     pub(crate) type OwnedValue = value_bag_sval2::buffer::Value<'static>;
 
-    pub(crate) fn buffer(v: impl value_bag_sval2::lib::Value) -> OwnedValue {
-        OwnedValue::collect_owned(v).unwrap()
+    pub(crate) fn buffer(
+        v: impl value_bag_sval2::lib::Value,
+    ) -> Result<OwnedValue, value_bag_sval2::buffer::Error> {
+        OwnedValue::collect_owned(v)
     }
 }
 
@@ -523,4 +529,34 @@ mod tests {
             );
         }
     }
+
+    #[cfg(feature = "owned")]
+    mod owned_support {
+        use super::*;
+
+        #[test]
+        #[cfg_attr(target_arch = "wasm32", wasm_bindgen_test)]
+        fn sval2_to_owned_poison() {
+            struct Kaboom;
+
+            impl value_bag_sval2::lib::Value for Kaboom {
+                fn stream<'sval, S: value_bag_sval2::lib::Stream<'sval> + ?Sized>(
+                    &'sval self,
+                    _: &mut S,
+                ) -> value_bag_sval2::lib::Result {
+                    value_bag_sval2::lib::error()
+                }
+            }
+
+            let value = ValueBag::capture_sval2(&Kaboom)
+                .to_owned()
+                .by_ref()
+                .to_test_token();
+
+            assert_eq!(
+                TestToken::Poisoned("failed to buffer the value".into()),
+                value
+            );
+        }
+    }
 }
diff --git a/src/owned.rs b/src/owned.rs
@@ -42,10 +42,13 @@ impl OwnedValueBag {
 mod tests {
     #[cfg(target_arch = "wasm32")]
     use wasm_bindgen_test::*;
-    
+
     use super::*;
 
-    use crate::{fill, std::{mem, string::ToString}};
+    use crate::{
+        fill,
+        std::{mem, string::ToString},
+    };
 
     const SIZE_LIMIT_U64: usize = 4;
 
@@ -74,7 +77,10 @@ mod tests {
     fn fill_to_owned() {
         let value = ValueBag::from_fill(&|slot: fill::Slot| slot.fill_any(42u64)).to_owned();
 
-        assert!(matches!(value.inner, internal::owned::OwnedInternal::BigUnsigned(42)));
+        assert!(matches!(
+            value.inner,
+            internal::owned::OwnedInternal::BigUnsigned(42)
+        ));
     }
 
     #[test]
@@ -83,8 +89,14 @@ mod tests {
         let debug = ValueBag::from_debug(&"a value").to_owned();
         let display = ValueBag::from_display(&"a value").to_owned();
 
-        assert!(matches!(debug.inner, internal::owned::OwnedInternal::Debug(_)));
-        assert!(matches!(display.inner, internal::owned::OwnedInternal::Display(_)));
+        assert!(matches!(
+            debug.inner,
+            internal::owned::OwnedInternal::Debug(_)
+        ));
+        assert!(matches!(
+            display.inner,
+            internal::owned::OwnedInternal::Display(_)
+        ));
 
         assert_eq!("\"a value\"", debug.to_string());
         assert_eq!("a value", display.to_string());
@@ -102,9 +114,14 @@ mod tests {
     fn error_to_owned() {
         use crate::std::io;
 
-        let value = ValueBag::from_dyn_error(&io::Error::new(io::ErrorKind::Other, "something failed!")).to_owned();
+        let value =
+            ValueBag::from_dyn_error(&io::Error::new(io::ErrorKind::Other, "something failed!"))
+                .to_owned();
 
-        assert!(matches!(value.inner, internal::owned::OwnedInternal::Error(_)));
+        assert!(matches!(
+            value.inner,
+            internal::owned::OwnedInternal::Error(_)
+        ));
 
         let value = value.by_ref();
 
@@ -119,7 +136,10 @@ mod tests {
     fn serde1_to_owned() {
         let value = ValueBag::from_serde1(&42u64).to_owned();
 
-        assert!(matches!(value.inner, internal::owned::OwnedInternal::Serde1(_)));
+        assert!(matches!(
+            value.inner,
+            internal::owned::OwnedInternal::Serde1(_)
+        ));
 
         let value = value.by_ref();
 
@@ -132,7 +152,10 @@ mod tests {
     fn sval2_to_owned() {
         let value = ValueBag::from_sval2(&42u64).to_owned();
 
-        assert!(matches!(value.inner, internal::owned::OwnedInternal::Sval2(_)));
+        assert!(matches!(
+            value.inner,
+            internal::owned::OwnedInternal::Sval2(_)
+        ));
 
         let value = value.by_ref();
 

diff --git a/src/test.rs b/src/test.rs
@@ -48,6 +48,8 @@ pub enum TestToken {
     Serde {
         version: u32,
     },
+
+    Poisoned(String),
 }
 
 impl<'v> ValueBag<'v> {
@@ -132,6 +134,11 @@ impl<'v> ValueBag<'v> {
                 self.0 = Some(TestToken::Serde { version: 1 });
                 Ok(())
             }
+
+            fn poisoned(&mut self, msg: &'static str) -> Result<(), Error> {
+                self.0 = Some(TestToken::Poisoned(msg.into()));
+                Ok(())
+            }
         }
 
         let mut visitor = TestVisitor(None);

diff --git a/src/visit.rs b/src/visit.rs
@@ -307,6 +307,10 @@ impl<'v> ValueBag<'v> {
             fn serde1(&mut self, v: &dyn internal::serde::v1::Serialize) -> Result<(), Error> {
                 internal::serde::v1::internal_visit(v, self)
             }
+
+            fn poisoned(&mut self, msg: &'static str) -> Result<(), Error> {
+                Err(Error::msg(msg))
+            }
         }
 
         self.internal_visit(&mut Visitor(visitor))