fix: Fix broken binary format.

README.md

@@ -114,11 +114,17 @@ recursion. See documentation of WithSchemaContext.
 1.6: Several savefile trait implementations have now gained 'static-bounds. For example,
 Box<T>, Vec<T> and many more now require T:'static. There was no such bound before, but
 since references cannot be deserialized, it was still typically not possible to deserialize
-anything containing a reference. In principle, someone could have implemented Deserialize
-for some type, leaking memory and returning an instance with a non-static lifetime. However,
-this is a very niche use, and it seems much more likely that deserializing types with arbitrary
-lifetimes is an error. Please file an issue if you have an use-case for deserializing types
-with lifetimes.
+anything containing a reference. 
+
+It turns out there is a usecase for serializing objects with lifetimes: Things like
+Cow<str> can be useful. Everything the deserializer produces must still have 'static lifetime in
+practice, because of how the Deserialize trait is defined (there's no other lifetime the
+return value can have).
+
+Serializing things with lifetimes is still possible, the only place where 'static is required
+is the contents of containers such as Box, Vec etc. The reason is that the new recursion
+support needs to be able to create TypeIds, and this is only possible for objects with
+'static lifetime.
 
 ## 0.17.0-beta.13
 

compile_tests/tests/compile-fail/bad_export.rs

@@ -0,0 +1,29 @@
+extern crate savefile;
+extern crate savefile_abi;
+extern crate savefile_derive;
+use std::collections::HashMap;
+use savefile::prelude::*;
+use savefile::{Deserialize, Deserializer, Serialize, Serializer};
+use std::fmt::Debug;
+use std::io::{BufWriter, Cursor, Write};
+use savefile_abi::AbiConnection;
+use savefile_derive::savefile_abi_exportable;
+use savefile_derive::savefile_abi_export;
+
+#[savefile_abi_exportable(version = 0)]
+pub trait ExampleTrait {
+    fn get(&mut self, x: u32) -> u32;
+}
+struct ExampleImpl {
+
+}
+impl ExampleTrait for ExampleImpl {
+    fn get(&mut self, x: u32) -> u32 {
+        x
+    }
+}
+// Test what happens when you mix up the ordering of trait and impl:
+savefile_abi_export!(ExampleTrait, ExampleImpl);
+//~^ 26:1: 26:48: expected trait, found struct `ExampleImpl` [E0404]
+
+fn main() {}

compile_tests/tests/compile-fail/bad_export2.rs

@@ -0,0 +1,26 @@
+extern crate savefile;
+extern crate savefile_abi;
+extern crate savefile_derive;
+use std::collections::HashMap;
+use savefile::prelude::*;
+use savefile::{Deserialize, Deserializer, Serialize, Serializer};
+use std::fmt::Debug;
+use std::io::{BufWriter, Cursor, Write};
+use savefile_abi::AbiConnection;
+use savefile_derive::savefile_abi_exportable;
+use savefile_derive::savefile_abi_export;
+
+#[savefile_abi_exportable(version = 0)]
+pub trait ExampleTrait {
+    fn get(&mut self, x: u32) -> u32;
+}
+#[derive(Default)]
+struct ExampleImpl {
+
+}
+
+// Forgot to implement trait
+savefile_abi_export!(ExampleImpl, ExampleTrait);
+//~^ 23:1: 23:48: the trait bound `ExampleImpl: ExampleTrait` is not satisfied [E0277]
+
+fn main() {}

compile_tests/tests/compile-fail/bad_export3.rs

@@ -0,0 +1,31 @@
+extern crate savefile;
+extern crate savefile_abi;
+extern crate savefile_derive;
+use std::collections::HashMap;
+use savefile::prelude::*;
+use savefile::{Deserialize, Deserializer, Serialize, Serializer};
+use std::fmt::Debug;
+use std::io::{BufWriter, Cursor, Write};
+use savefile_abi::AbiConnection;
+use savefile_derive::savefile_abi_exportable;
+use savefile_derive::savefile_abi_export;
+
+#[savefile_abi_exportable(version = 0)]
+pub trait ExampleTrait {
+    fn get(&mut self, x: u32) -> u32;
+}
+struct ExampleImpl {
+
+}
+
+impl ExampleTrait for ExampleImpl {
+    fn get(&mut self, x: u32) -> u32 {
+        x
+    }
+}
+
+// Forgot to implement Default
+savefile_abi_export!(ExampleImpl, ExampleTrait);
+//~^ 28:1: 28:48: the trait bound `ExampleImpl: Default` is not satisfied [E0277]
+
+fn main() {}

compile_tests/tests/compile-fail/cow_smuggler.rs

@@ -0,0 +1,26 @@
+extern crate savefile;
+extern crate savefile_abi;
+extern crate savefile_derive;
+use std::collections::HashMap;
+use savefile::prelude::*;
+use savefile::{Deserialize, Deserializer, Serialize, Serializer};
+use std::fmt::Debug;
+use std::io::{BufWriter, Cursor, Write};
+use savefile_abi::AbiConnection;
+use savefile_derive::savefile_abi_exportable;
+use std::borrow::Cow;
+#[savefile_abi_exportable(version = 0)]
+pub trait CowSmuggler {
+    fn smuggle(&mut self, x: Cow<str>) -> Cow<str>;
+}
+impl CowSmuggler for () {
+    fn smuggle(&mut self, x: Cow<str>) -> Cow<str> {
+        x
+//~^ 18:9: 18:10: lifetime may not live long enough
+    }
+    // If someone calls smuggle(..) with a reference to a long-lived, but not static item,
+    // it is important to understand that the returned Cow<str> cannot have the same lifetime.
+    // it may have to be deserialized, and will then be an owned value. It will not be a reference
+    // with the same lifetime as the argument.
+}
+fn main() {}

savefile-abi/CHANGELOG.md

@@ -6,6 +6,12 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 
 ## [Unreleased]
 
+## [0.17.0-beta.15](https://github.com/avl/savefile/compare/savefile-abi-v0.17.0-beta.14...savefile-abi-v0.17.0-beta.15) - 2024-04-30
+
+### Fixed
+- AbiConnection cache could become corrupt if AbiConnection was used incorrectly
+- Improve documentation
+
 ## [0.17.0-beta.14](https://github.com/avl/savefile/compare/savefile-abi-v0.17.0-beta.13...savefile-abi-v0.17.0-beta.14) - 2024-04-30
 
 ### Other

savefile-abi/Cargo.toml

@@ -1,6 +1,6 @@
 [package]
 name = "savefile-abi"
-version = "0.17.0-beta.14"
+version = "0.17.0-beta.15"
 edition = "2021"
 authors = ["Anders Musikka <[email protected]>"]
 documentation = "https://docs.rs/savefile-abi/"
@@ -16,7 +16,7 @@ keywords = ["dylib", "dlopen", "ffi"]
 license = "MIT/Apache-2.0"
 
 [dependencies]
-savefile = { path="../savefile", version = "=0.17.0-beta.14" }
-savefile-derive = { path="../savefile-derive", version = "=0.17.0-beta.14" }
+savefile = { path="../savefile", version = "=0.17.0-beta.15" }
+savefile-derive = { path="../savefile-derive", version = "=0.17.0-beta.15" }
 byteorder = "1.4"
 libloading = "0.8"

savefile-abi/src/lib.rs

@@ -233,10 +233,11 @@ This has a performance penalty, and may require heap allocation.
 
  * It supports trait objects as arguments, including FnMut() and Fn().
 
- * Boxed trait objects can be transferred across FFI-boundaries, passing ownership, while
-   still not invoking UB if the object is dropped on the other side of the FFI-boundary.
+ * Boxed trait objects, including Fn-traits, can be transferred across FFI-boundaries, passing
+   ownership, while still not invoking UB if the object is dropped on the other side of the
+   FFI-boundary.
 
- * It requires enums to be `#[repr(C,u8)]` in order to pass them by reference. Other enums
+ * It requires enums to be `#[repr(uX)]` in order to pass them by reference. Other enums
 will still work correctly, but will be serialized under the hood at a performance penalty.
 
  * It places severe restrictions on types of arguments, since they must be serializable
@@ -274,8 +275,9 @@ One thing to be aware of is that, at present, the AbiConnection::load_shared_lib
 is not marked as unsafe. However, if the .so-file given as argument is corrupt, using this
 method can cause any amount of UB. Thus, it could be argued that it should be marked unsafe.
 
-However, the same is true for _any_ rust shared library. We are simply reliant on the
-compiler and all dependencies we use being implemented correctly. Thus, it has been
+However, the same is true for _any_ shared library used by a rust program, including the
+system C-library. It is also true that rust programs rely on the rust
+compiler being implemented correctly. Thus, it has been
 judged that the issue of corrupt binary files is beyond the scope of safety for Savefile-Abi.
 
 As long as the shared library is a real Savefile-Abi shared library, it should be sound to use,
@@ -312,6 +314,7 @@ use std::path::Path;
 use std::ptr::null;
 use std::sync::{Mutex, MutexGuard};
 use std::{ptr, slice};
+use std::any::TypeId;
 
 use byteorder::ReadBytesExt;
 use libloading::{Library, Symbol};
@@ -850,7 +853,7 @@ pub fn parse_return_value_impl<T>(
 /// Parse an RawAbiCallResult instance into a Result<Box<dyn T>, SavefileError> .
 /// This is used on the caller side, and the type T will always be statically known.
 /// TODO: There's some duplicated code here, compare parse_return_value
-pub fn parse_return_boxed_trait<T>(outcome: &RawAbiCallResult) -> Result<Box<AbiConnection<T>>, SavefileError>
+pub fn parse_return_boxed_trait<T:'static>(outcome: &RawAbiCallResult) -> Result<Box<AbiConnection<T>>, SavefileError>
 where
     T: AbiExportable + ?Sized,
 {
@@ -872,7 +875,7 @@ static ENTRY_CACHE: Mutex<
 > = Mutex::new(None);
 
 static ABI_CONNECTION_TEMPLATES: Mutex<
-    Option<HashMap<unsafe extern "C" fn(flag: AbiProtocol), AbiConnectionTemplate>>,
+    Option<HashMap<(TypeId,unsafe extern "C" fn(flag: AbiProtocol)), AbiConnectionTemplate>>,
 > = Mutex::new(None);
 
 struct Guard<'a, K: Hash + Eq, V> {
@@ -969,7 +972,7 @@ pub unsafe extern "C" fn abi_result_receiver<T: Deserialize>(
 
 /// Raw entry point for receiving return values from other shared libraries
 #[doc(hidden)]
-pub unsafe extern "C" fn abi_boxed_trait_receiver<T>(outcome: *const RawAbiCallResult, result_receiver: *mut ())
+pub unsafe extern "C" fn abi_boxed_trait_receiver<T:'static>(outcome: *const RawAbiCallResult, result_receiver: *mut ())
 where
     T: AbiExportable + ?Sized,
 {
@@ -1073,7 +1076,7 @@ fn arg_layout_compatible(
     }
 }
 
-impl<T: AbiExportable + ?Sized> AbiConnection<T> {
+impl<T: AbiExportable + ?Sized + 'static> AbiConnection<T> {
     /// Analyse the difference in definitions between the two sides,
     /// and create an AbiConnection
     #[allow(clippy::too_many_arguments)]
@@ -1395,7 +1398,11 @@ impl<T: AbiExportable + ?Sized> AbiConnection<T> {
     ) -> Result<AbiConnection<T>, SavefileError> {
         let mut templates = Guard::lock(&ABI_CONNECTION_TEMPLATES);
 
-        let template = match templates.entry(remote_entry) {
+        let typeid = TypeId::of::<T>();
+        // In principle, it would be enough to key 'templates' based on 'remote_entry'.
+        // However, if we do, and the user ever uses AbiConnection<T> with the _wrong_ entry point,
+        // we risk poisoning the cache with erroneous data. 
+        let template = match templates.entry((typeid,remote_entry)) {
             Entry::Occupied(template) => template.get().clone(),
             Entry::Vacant(vacant) => {
                 let own_version = T::get_latest_version();

savefile-derive/CHANGELOG.md

@@ -6,6 +6,11 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 
 ## [Unreleased]
 
+## [0.17.0-beta.15](https://github.com/avl/savefile/compare/savefile-derive-v0.17.0-beta.14...savefile-derive-v0.17.0-beta.15) - 2024-04-30
+
+### Fixed
+- AbiConnection cache could become corrupt if AbiConnection was used incorrectly
+
 ## [0.17.0-beta.14](https://github.com/avl/savefile/compare/savefile-derive-v0.17.0-beta.13...savefile-derive-v0.17.0-beta.14) - 2024-04-30
 
 ### Fixed

savefile-derive/Cargo.toml

@@ -1,6 +1,6 @@
 [package]
 name = "savefile-derive"
-version = "0.17.0-beta.14"
+version = "0.17.0-beta.15"
 authors = ["Anders Musikka <[email protected]>"]
 
 description = "Custom derive macros for savefile crate - simple, convenient, fast, versioned, binary serialization/deserialization library."

savefile-derive/src/serialize.rs

@@ -41,10 +41,6 @@ pub(super) fn savefile_derive_crate_serialize(input: DeriveInput) -> TokenStream
     let expanded = match &input.data {
         &syn::Data::Enum(ref enum1) => {
             let mut output = Vec::new();
-            //let variant_count = enum1.variants.len();
-            /*if variant_count >= 256 {
-                panic!("This library is not capable of serializing enums with 256 variants or more. Our deepest apologies, we thought no-one would ever create such an enum!");
-            }*/
             let enum_size = get_enum_size(&input.attrs, enum1.variants.len());
 
             for (var_idx_usize, variant) in enum1.variants.iter().enumerate() {

savefile-test/Cargo.toml

@@ -12,7 +12,7 @@ nightly=["savefile/nightly"]
 
 [dependencies]
 savefile = { path = "../savefile", features = ["size_sanity_checks", "encryption", "compression","bit-set","bit-vec","rustc-hash","serde_derive", "quickcheck"]}
-savefile-derive = { path = "../savefile-derive", version = "=0.17.0-beta.14" }
+savefile-derive = { path = "../savefile-derive", version = "=0.17.0-beta.15" }
 savefile-abi = { path = "../savefile-abi" }
 bit-vec = "0.6"
 arrayvec="0.7"