From 4d29aac7db50db0f1f7831936ecfb8f34069964d Mon Sep 17 00:00:00 2001 From: Ben Dean-Kawamura Date: Thu, 26 Oct 2023 15:57:24 -0400 Subject: [PATCH] Updated lower validation for Python/Ruby The dynamic languages performed various checks in lower(): type checks, bounds checks, etc. This presented an issue when we wanted to call `inc_ref` for object types. If we did this, then another value failed its check, we would leak the reference. Prevent this by adding a `check()` method. We do the check first then once we get to `lower()`, nothing can fail. --- examples/callbacks/src/lib.rs | 2 + .../sprites/tests/bindings/test_sprites.kts | 2 +- examples/traits/src/lib.rs | 1 + .../coverall/tests/bindings/test_coverall.py | 17 ++++----- .../coverall/tests/bindings/test_coverall.rb | 19 ++++------ .../reexport-scaffolding-macro/src/lib.rs | 31 ++++++++------- .../ui/interface_trait_not_sync_and_send.rs | 1 + .../interface_trait_not_sync_and_send.stderr | 8 ++-- .../src/bindings/python/gen_python/mod.rs | 4 ++ .../python/templates/BooleanHelper.py | 18 +++++---- .../bindings/python/templates/BytesHelper.py | 5 ++- .../templates/CallbackInterfaceRuntime.py | 4 ++ .../bindings/python/templates/CustomType.py | 9 +++++ .../python/templates/DurationHelper.py | 8 +++- .../bindings/python/templates/EnumTemplate.py | 17 +++++++++ .../python/templates/ErrorTemplate.py | 14 +++++++ .../python/templates/Float32Helper.py | 2 +- .../python/templates/Float64Helper.py | 2 +- .../templates/ForeignExecutorTemplate.py | 5 ++- .../bindings/python/templates/Int16Helper.py | 2 +- .../bindings/python/templates/Int32Helper.py | 2 +- .../bindings/python/templates/Int64Helper.py | 2 +- .../bindings/python/templates/Int8Helper.py | 2 +- .../bindings/python/templates/MapTemplate.py | 6 +++ .../python/templates/ObjectTemplate.py | 12 ++++-- .../python/templates/OptionalTemplate.py | 5 +++ .../python/templates/RecordTemplate.py | 10 +++++ .../python/templates/RustBufferHelper.py | 16 +------- .../python/templates/SequenceTemplate.py | 5 +++ .../bindings/python/templates/StringHelper.py | 2 - .../python/templates/TimestampHelper.py | 4 ++ .../templates/TopLevelFunctionTemplate.py | 1 + .../bindings/python/templates/UInt16Helper.py | 2 +- .../bindings/python/templates/UInt32Helper.py | 2 +- .../bindings/python/templates/UInt64Helper.py | 2 +- .../bindings/python/templates/UInt8Helper.py | 2 +- .../src/bindings/python/templates/macros.py | 2 + .../src/bindings/ruby/gen_ruby/mod.rs | 16 ++++++++ .../bindings/ruby/templates/ObjectTemplate.rb | 15 +++++--- .../ruby/templates/RustBufferTemplate.rb | 38 +++++++++++++++++++ .../templates/TopLevelFunctionTemplate.rb | 4 +- .../src/bindings/ruby/templates/macros.rb | 10 +++-- .../src/bindings/swift/templates/Slab.swift | 2 +- 43 files changed, 240 insertions(+), 93 deletions(-) diff --git a/examples/callbacks/src/lib.rs b/examples/callbacks/src/lib.rs index dac5653d1b..9e30bf1d86 100644 --- a/examples/callbacks/src/lib.rs +++ b/examples/callbacks/src/lib.rs @@ -21,6 +21,7 @@ impl From for TelephoneError { } // SIM cards. +#[uniffi::trait_interface] pub trait SimCard: Send + Sync { fn name(&self) -> String; } @@ -39,6 +40,7 @@ fn get_sim_cards() -> Vec> { } // The call-answer, callback interface. +#[uniffi::trait_interface] pub trait CallAnswerer { fn answer(&self) -> Result; } diff --git a/examples/sprites/tests/bindings/test_sprites.kts b/examples/sprites/tests/bindings/test_sprites.kts index 42451f28dd..ddd983351c 100644 --- a/examples/sprites/tests/bindings/test_sprites.kts +++ b/examples/sprites/tests/bindings/test_sprites.kts @@ -16,7 +16,7 @@ s.destroy() try { s.moveBy(Vector(0.0, 0.0)) assert(false) { "Should not be able to call anything after `destroy`" } -} catch(e: IllegalStateException) { +} catch(e: InternalException) { assert(true) } diff --git a/examples/traits/src/lib.rs b/examples/traits/src/lib.rs index 9d84bdae7f..51d81d83d8 100644 --- a/examples/traits/src/lib.rs +++ b/examples/traits/src/lib.rs @@ -13,6 +13,7 @@ fn press(button: Arc) -> Arc { button } +#[uniffi::trait_interface] pub trait Button: Send + Sync { fn name(&self) -> String; } diff --git a/fixtures/coverall/tests/bindings/test_coverall.py b/fixtures/coverall/tests/bindings/test_coverall.py index ec6056b926..b4e27a450e 100644 --- a/fixtures/coverall/tests/bindings/test_coverall.py +++ b/fixtures/coverall/tests/bindings/test_coverall.py @@ -242,16 +242,13 @@ def test_return_objects(self): coveralls = None self.assertEqual(get_num_alive(), 0) - # FIXME: since we're now inc-refing the handle, Python will leak objects if another lower fails. - # For now, let's disable this test. - # - # def test_bad_objects(self): - # coveralls = Coveralls("test_bad_objects") - # patch = Patch(Color.RED) - # # `coveralls.take_other` wants `Coveralls` not `Patch` - # with self.assertRaisesRegex(TypeError, "Coveralls.*Patch"): - # coveralls.take_other(patch) - # + def test_bad_objects(self): + coveralls = Coveralls("test_bad_objects") + patch = Patch(Color.RED) + # `coveralls.take_other` wants `Coveralls` not `Patch` + with self.assertRaisesRegex(TypeError, "Coveralls.*Patch"): + coveralls.take_other(patch) + def test_dict_with_defaults(self): """ This does not call Rust code. """ diff --git a/fixtures/coverall/tests/bindings/test_coverall.rb b/fixtures/coverall/tests/bindings/test_coverall.rb index c74a2f032f..4fe1140419 100644 --- a/fixtures/coverall/tests/bindings/test_coverall.rb +++ b/fixtures/coverall/tests/bindings/test_coverall.rb @@ -244,17 +244,14 @@ def test_return_objects assert_equal Coverall.get_num_alive(), 0 end - # FIXME: since we're now inc-refing the handle, Ruby will leak objects if another lower fails. - # For now, let's disable this test. - # - # def test_bad_objects - # coveralls = Coverall::Coveralls.new "test_bad_objects" - # patch = Coverall::Patch.new Coverall::Color::RED - # # `coveralls.take_other` wants `Coveralls` not `Patch` - # assert_raise_message /Expected a Coveralls instance, got.*Patch/ do - # coveralls.take_other patch - # end - # end + def test_bad_objects + coveralls = Coverall::Coveralls.new "test_bad_objects" + patch = Coverall::Patch.new Coverall::Color::RED + # `coveralls.take_other` wants `Coveralls` not `Patch` + assert_raise_message /Expected a Coveralls instance, got.*Patch/ do + coveralls.take_other patch + end + end def test_bytes coveralls = Coverall::Coveralls.new "test_bytes" diff --git a/fixtures/reexport-scaffolding-macro/src/lib.rs b/fixtures/reexport-scaffolding-macro/src/lib.rs index 6bd04f2ccd..860fe2d019 100644 --- a/fixtures/reexport-scaffolding-macro/src/lib.rs +++ b/fixtures/reexport-scaffolding-macro/src/lib.rs @@ -6,9 +6,10 @@ mod tests { use cargo_metadata::Message; use libloading::{Library, Symbol}; use std::ffi::CString; - use std::os::raw::c_void; use std::process::{Command, Stdio}; - use uniffi::{FfiConverter, ForeignCallback, RustBuffer, RustCallStatus, RustCallStatusCode}; + use uniffi::{ + FfiConverter, ForeignCallback, Handle, RustBuffer, RustCallStatus, RustCallStatusCode, + }; use uniffi_bindgen::ComponentInterface; struct UniFfiTag; @@ -149,26 +150,27 @@ mod tests { .ffi_func() .name(), ); - let coveralls_new: Symbol< - unsafe extern "C" fn(RustBuffer, &mut RustCallStatus) -> *const c_void, - > = get_symbol( - &library, - object_def.primary_constructor().unwrap().ffi_func().name(), - ); + let coveralls_new: Symbol Handle> = + get_symbol( + &library, + object_def.primary_constructor().unwrap().ffi_func().name(), + ); let coveralls_get_name: Symbol< - unsafe extern "C" fn(*const c_void, &mut RustCallStatus) -> RustBuffer, + unsafe extern "C" fn(Handle, &mut RustCallStatus) -> RustBuffer, > = get_symbol( &library, object_def.get_method("get_name").ffi_func().name(), ); - let coveralls_free: Symbol ()> = + let coveralls_inc_ref: Symbol ()> = + get_symbol(&library, object_def.ffi_object_inc_ref().name()); + let coveralls_free: Symbol ()> = get_symbol(&library, object_def.ffi_object_free().name()); let num_alive = unsafe { get_num_alive(&mut call_status) }; assert_eq!(call_status.code, RustCallStatusCode::Success); assert_eq!(num_alive, 0); - let obj_id = unsafe { + let obj_handle = unsafe { coveralls_new( >::lower("TestName".into()), &mut call_status, @@ -176,7 +178,10 @@ mod tests { }; assert_eq!(call_status.code, RustCallStatusCode::Success); - let name_buf = unsafe { coveralls_get_name(obj_id, &mut call_status) }; + unsafe { coveralls_inc_ref(obj_handle, &mut call_status) }; + assert_eq!(call_status.code, RustCallStatusCode::Success); + + let name_buf = unsafe { coveralls_get_name(obj_handle, &mut call_status) }; assert_eq!(call_status.code, RustCallStatusCode::Success); assert_eq!( >::try_lift(name_buf).unwrap(), @@ -187,7 +192,7 @@ mod tests { assert_eq!(call_status.code, RustCallStatusCode::Success); assert_eq!(num_alive, 1); - unsafe { coveralls_free(obj_id, &mut call_status) }; + unsafe { coveralls_free(obj_handle, &mut call_status) }; assert_eq!(call_status.code, RustCallStatusCode::Success); let num_alive = unsafe { get_num_alive(&mut call_status) }; diff --git a/fixtures/uitests/tests/ui/interface_trait_not_sync_and_send.rs b/fixtures/uitests/tests/ui/interface_trait_not_sync_and_send.rs index 1bcd3a4237..6671a9624a 100644 --- a/fixtures/uitests/tests/ui/interface_trait_not_sync_and_send.rs +++ b/fixtures/uitests/tests/ui/interface_trait_not_sync_and_send.rs @@ -4,6 +4,7 @@ uniffi_macros::generate_and_include_scaffolding!("../../../../fixtures/uitests/s fn main() { /* empty main required by `trybuild` */} // This will fail to compile, because the trait is not explicit Send+Sync +#[uniffi::trait_interface] pub trait Trait { } diff --git a/fixtures/uitests/tests/ui/interface_trait_not_sync_and_send.stderr b/fixtures/uitests/tests/ui/interface_trait_not_sync_and_send.stderr index a8e7dc90a1..3925745647 100644 --- a/fixtures/uitests/tests/ui/interface_trait_not_sync_and_send.stderr +++ b/fixtures/uitests/tests/ui/interface_trait_not_sync_and_send.stderr @@ -27,9 +27,9 @@ note: required by a bound in `FfiConverterArc` = note: this error originates in the attribute macro `::uniffi::export_for_udl` (in Nightly builds, run with -Z macro-backtrace for more info) error[E0277]: `(dyn ProcMacroTrait + 'static)` cannot be shared between threads safely - --> tests/ui/interface_trait_not_sync_and_send.rs:11:1 + --> tests/ui/interface_trait_not_sync_and_send.rs:12:1 | -11 | #[uniffi::export] +12 | #[uniffi::export] | ^^^^^^^^^^^^^^^^^ `(dyn ProcMacroTrait + 'static)` cannot be shared between threads safely | = help: the trait `Sync` is not implemented for `(dyn ProcMacroTrait + 'static)` @@ -41,9 +41,9 @@ note: required by a bound in `FfiConverterArc` = note: this error originates in the attribute macro `uniffi::export` (in Nightly builds, run with -Z macro-backtrace for more info) error[E0277]: `(dyn ProcMacroTrait + 'static)` cannot be sent between threads safely - --> tests/ui/interface_trait_not_sync_and_send.rs:11:1 + --> tests/ui/interface_trait_not_sync_and_send.rs:12:1 | -11 | #[uniffi::export] +12 | #[uniffi::export] | ^^^^^^^^^^^^^^^^^ `(dyn ProcMacroTrait + 'static)` cannot be sent between threads safely | = help: the trait `Send` is not implemented for `(dyn ProcMacroTrait + 'static)` diff --git a/uniffi_bindgen/src/bindings/python/gen_python/mod.rs b/uniffi_bindgen/src/bindings/python/gen_python/mod.rs index 0d6545fc21..700dfc3ece 100644 --- a/uniffi_bindgen/src/bindings/python/gen_python/mod.rs +++ b/uniffi_bindgen/src/bindings/python/gen_python/mod.rs @@ -413,6 +413,10 @@ pub mod filters { Ok(format!("{}.lift", ffi_converter_name(as_ct)?)) } + pub fn check_fn(as_ct: &impl AsCodeType) -> Result { + Ok(format!("{}.check", ffi_converter_name(as_ct)?)) + } + pub fn lower_fn(as_ct: &impl AsCodeType) -> Result { Ok(format!("{}.lower", ffi_converter_name(as_ct)?)) } diff --git a/uniffi_bindgen/src/bindings/python/templates/BooleanHelper.py b/uniffi_bindgen/src/bindings/python/templates/BooleanHelper.py index 6775e9e132..52ca6a2646 100644 --- a/uniffi_bindgen/src/bindings/python/templates/BooleanHelper.py +++ b/uniffi_bindgen/src/bindings/python/templates/BooleanHelper.py @@ -1,16 +1,20 @@ -class _UniffiConverterBool(_UniffiConverterPrimitive): +class _UniffiConverterBool: @classmethod def check(cls, value): - return not not value + pass + + @classmethod + def lower(cls, value): + return 1 if value else 0 + + @staticmethod + def lift(value): + return value != 0 @classmethod def read(cls, buf): return cls.lift(buf.read_u8()) @classmethod - def write_unchecked(cls, value, buf): + def write(cls, value, buf): buf.write_u8(value) - - @staticmethod - def lift(value): - return value != 0 diff --git a/uniffi_bindgen/src/bindings/python/templates/BytesHelper.py b/uniffi_bindgen/src/bindings/python/templates/BytesHelper.py index 196b5b29fa..f649b5e41a 100644 --- a/uniffi_bindgen/src/bindings/python/templates/BytesHelper.py +++ b/uniffi_bindgen/src/bindings/python/templates/BytesHelper.py @@ -7,10 +7,13 @@ def read(buf): return buf.read(size) @staticmethod - def write(value, buf): + def check(value): try: memoryview(value) except TypeError: raise TypeError("a bytes-like object is required, not {!r}".format(type(value).__name__)) + + @staticmethod + def write(value, buf): buf.write_i32(len(value)) buf.write(value) diff --git a/uniffi_bindgen/src/bindings/python/templates/CallbackInterfaceRuntime.py b/uniffi_bindgen/src/bindings/python/templates/CallbackInterfaceRuntime.py index 2d98eba5db..6257407c22 100644 --- a/uniffi_bindgen/src/bindings/python/templates/CallbackInterfaceRuntime.py +++ b/uniffi_bindgen/src/bindings/python/templates/CallbackInterfaceRuntime.py @@ -24,6 +24,10 @@ def read(cls, buf): handle = buf.read_u64() cls.lift(handle) + @classmethod + def check(cls, cb): + pass + @classmethod def lower(cls, cb): handle = cls._slab.insert(cb) diff --git a/uniffi_bindgen/src/bindings/python/templates/CustomType.py b/uniffi_bindgen/src/bindings/python/templates/CustomType.py index 5be6155b84..f7e626c163 100644 --- a/uniffi_bindgen/src/bindings/python/templates/CustomType.py +++ b/uniffi_bindgen/src/bindings/python/templates/CustomType.py @@ -17,6 +17,10 @@ def read(buf): def lift(value): return {{ builtin|ffi_converter_name }}.lift(value) + @staticmethod + def check(value): + return {{ builtin|ffi_converter_name }}.check(value) + @staticmethod def lower(value): return {{ builtin|ffi_converter_name }}.lower(value) @@ -51,6 +55,11 @@ def lift(value): builtin_value = {{ builtin|lift_fn }}(value) return {{ config.into_custom.render("builtin_value") }} + @staticmethod + def check(value): + builtin_value = {{ config.from_custom.render("value") }} + return {{ builtin|check_fn }}(builtin_value) + @staticmethod def lower(value): builtin_value = {{ config.from_custom.render("value") }} diff --git a/uniffi_bindgen/src/bindings/python/templates/DurationHelper.py b/uniffi_bindgen/src/bindings/python/templates/DurationHelper.py index 10974e009d..3bb06e1099 100644 --- a/uniffi_bindgen/src/bindings/python/templates/DurationHelper.py +++ b/uniffi_bindgen/src/bindings/python/templates/DurationHelper.py @@ -12,10 +12,14 @@ def read(buf): return datetime.timedelta(seconds=seconds, microseconds=microseconds) @staticmethod - def write(value, buf): + def check(value): seconds = value.seconds + value.days * 24 * 3600 - nanoseconds = value.microseconds * 1000 if seconds < 0: raise ValueError("Invalid duration, must be non-negative") + + @staticmethod + def write(value, buf): + seconds = value.seconds + value.days * 24 * 3600 + nanoseconds = value.microseconds * 1000 buf.write_i64(seconds) buf.write_u32(nanoseconds) diff --git a/uniffi_bindgen/src/bindings/python/templates/EnumTemplate.py b/uniffi_bindgen/src/bindings/python/templates/EnumTemplate.py index 84d089baf9..8f0989e634 100644 --- a/uniffi_bindgen/src/bindings/python/templates/EnumTemplate.py +++ b/uniffi_bindgen/src/bindings/python/templates/EnumTemplate.py @@ -81,6 +81,23 @@ def read(buf): {%- endfor %} raise InternalError("Raw enum value doesn't match any cases") + def check(value): + {%- if e.variants().is_empty() %} + pass + {%- else %} + {%- for variant in e.variants() %} + {%- if e.is_flat() %} + if value == {{ type_name }}.{{ variant.name()|enum_variant_py }}: + {%- else %} + if value.is_{{ variant.name()|var_name }}(): + {%- endif %} + {%- for field in variant.fields() %} + {{ field|check_fn }}(value.{{ field.name()|var_name }}) + {%- endfor %} + return + {%- endfor %} + {%- endif %} + def write(value, buf): {%- for variant in e.variants() %} {%- if e.is_flat() %} diff --git a/uniffi_bindgen/src/bindings/python/templates/ErrorTemplate.py b/uniffi_bindgen/src/bindings/python/templates/ErrorTemplate.py index 26a1e6452a..b4c9849626 100644 --- a/uniffi_bindgen/src/bindings/python/templates/ErrorTemplate.py +++ b/uniffi_bindgen/src/bindings/python/templates/ErrorTemplate.py @@ -59,6 +59,20 @@ def read(buf): {%- endfor %} raise InternalError("Raw enum value doesn't match any cases") + @staticmethod + def check(value): + {%- if e.variants().is_empty() %} + pass + {%- else %} + {%- for variant in e.variants() %} + if isinstance(value, {{ type_name }}.{{ variant.name()|class_name }}): + {%- for field in variant.fields() %} + {{ field|check_fn }}(value.{{ field.name()|var_name }}) + {%- endfor %} + return + {%- endfor %} + {%- endif %} + @staticmethod def write(value, buf): {%- for variant in e.variants() %} diff --git a/uniffi_bindgen/src/bindings/python/templates/Float32Helper.py b/uniffi_bindgen/src/bindings/python/templates/Float32Helper.py index a52107a638..49a1a7286e 100644 --- a/uniffi_bindgen/src/bindings/python/templates/Float32Helper.py +++ b/uniffi_bindgen/src/bindings/python/templates/Float32Helper.py @@ -4,5 +4,5 @@ def read(buf): return buf.read_float() @staticmethod - def write_unchecked(value, buf): + def write(value, buf): buf.write_float(value) diff --git a/uniffi_bindgen/src/bindings/python/templates/Float64Helper.py b/uniffi_bindgen/src/bindings/python/templates/Float64Helper.py index 772f5080e9..e2084c7b13 100644 --- a/uniffi_bindgen/src/bindings/python/templates/Float64Helper.py +++ b/uniffi_bindgen/src/bindings/python/templates/Float64Helper.py @@ -4,5 +4,5 @@ def read(buf): return buf.read_double() @staticmethod - def write_unchecked(value, buf): + def write(value, buf): buf.write_double(value) diff --git a/uniffi_bindgen/src/bindings/python/templates/ForeignExecutorTemplate.py b/uniffi_bindgen/src/bindings/python/templates/ForeignExecutorTemplate.py index 9da7bad33b..166fa95b42 100644 --- a/uniffi_bindgen/src/bindings/python/templates/ForeignExecutorTemplate.py +++ b/uniffi_bindgen/src/bindings/python/templates/ForeignExecutorTemplate.py @@ -12,9 +12,12 @@ class {{ ffi_converter_name }}: _slab = UniffiSlab() @classmethod - def lower(cls, eventloop): + def check(cls, eventloop): if not isinstance(eventloop, asyncio.BaseEventLoop): raise TypeError("_uniffi_executor_callback: Expected EventLoop instance") + + @classmethod + def lower(cls, eventloop): return cls._slab.insert(eventloop) @classmethod diff --git a/uniffi_bindgen/src/bindings/python/templates/Int16Helper.py b/uniffi_bindgen/src/bindings/python/templates/Int16Helper.py index 99f19dc1c0..befa563384 100644 --- a/uniffi_bindgen/src/bindings/python/templates/Int16Helper.py +++ b/uniffi_bindgen/src/bindings/python/templates/Int16Helper.py @@ -8,5 +8,5 @@ def read(buf): return buf.read_i16() @staticmethod - def write_unchecked(value, buf): + def write(value, buf): buf.write_i16(value) diff --git a/uniffi_bindgen/src/bindings/python/templates/Int32Helper.py b/uniffi_bindgen/src/bindings/python/templates/Int32Helper.py index 3b142c8749..70644f6717 100644 --- a/uniffi_bindgen/src/bindings/python/templates/Int32Helper.py +++ b/uniffi_bindgen/src/bindings/python/templates/Int32Helper.py @@ -8,5 +8,5 @@ def read(buf): return buf.read_i32() @staticmethod - def write_unchecked(value, buf): + def write(value, buf): buf.write_i32(value) diff --git a/uniffi_bindgen/src/bindings/python/templates/Int64Helper.py b/uniffi_bindgen/src/bindings/python/templates/Int64Helper.py index 6e94379cbf..232f127bd6 100644 --- a/uniffi_bindgen/src/bindings/python/templates/Int64Helper.py +++ b/uniffi_bindgen/src/bindings/python/templates/Int64Helper.py @@ -8,5 +8,5 @@ def read(buf): return buf.read_i64() @staticmethod - def write_unchecked(value, buf): + def write(value, buf): buf.write_i64(value) diff --git a/uniffi_bindgen/src/bindings/python/templates/Int8Helper.py b/uniffi_bindgen/src/bindings/python/templates/Int8Helper.py index 732530e3cb..c1de1625e7 100644 --- a/uniffi_bindgen/src/bindings/python/templates/Int8Helper.py +++ b/uniffi_bindgen/src/bindings/python/templates/Int8Helper.py @@ -8,5 +8,5 @@ def read(buf): return buf.read_i8() @staticmethod - def write_unchecked(value, buf): + def write(value, buf): buf.write_i8(value) diff --git a/uniffi_bindgen/src/bindings/python/templates/MapTemplate.py b/uniffi_bindgen/src/bindings/python/templates/MapTemplate.py index 387227ed09..6f413bb7e6 100644 --- a/uniffi_bindgen/src/bindings/python/templates/MapTemplate.py +++ b/uniffi_bindgen/src/bindings/python/templates/MapTemplate.py @@ -2,6 +2,12 @@ {%- let value_ffi_converter = value_type|ffi_converter_name %} class {{ ffi_converter_name }}(_UniffiConverterRustBuffer): + @classmethod + def check(cls, items): + for (key, value) in items.items(): + {{ key_ffi_converter }}.check(key) + {{ value_ffi_converter }}.check(value) + @classmethod def write(cls, items, buf): buf.write_i32(len(items)) diff --git a/uniffi_bindgen/src/bindings/python/templates/ObjectTemplate.py b/uniffi_bindgen/src/bindings/python/templates/ObjectTemplate.py index 69a440a05f..7b6e900587 100644 --- a/uniffi_bindgen/src/bindings/python/templates/ObjectTemplate.py +++ b/uniffi_bindgen/src/bindings/python/templates/ObjectTemplate.py @@ -39,7 +39,6 @@ def _make_instance_(cls, handle): @classmethod def {{ cons.name()|fn_name }}(cls, {% call py::arg_list_decl(cons) %}): {%- call py::setup_args_extra_indent(cons) %} - # Call the (fallible) function before creating any half-baked object instances. uniffi_handle = {% call py::to_ffi_call(cons) %} return cls._make_instance_(uniffi_handle) {% endfor %} @@ -94,6 +93,15 @@ def lift(value: int): return {{ impl_name }}._make_instance_(value) {%- endif %} + @staticmethod + def check(value: {{ protocol_name }}): + {%- if obj.is_trait_interface() %} + pass + {%- else %} + if not isinstance(value, {{ impl_name }}): + raise TypeError("Expected {{ impl_name }} instance, {} found".format(type(value).__name__)) + {%- endif %} + @staticmethod def lower(value: {{ protocol_name }}): {%- if obj.is_trait_interface() %} @@ -105,8 +113,6 @@ def lower(value: {{ protocol_name }}): else: return {{ ffi_converter_name }}._slab.insert(value) {%- else %} - if not isinstance(value, {{ impl_name }}): - raise TypeError("Expected {{ impl_name }} instance, {} found".format(type(value).__name__)) return value._uniffi_clone_handle() {%- endif %} diff --git a/uniffi_bindgen/src/bindings/python/templates/OptionalTemplate.py b/uniffi_bindgen/src/bindings/python/templates/OptionalTemplate.py index e406c51d49..14b7a0f3af 100644 --- a/uniffi_bindgen/src/bindings/python/templates/OptionalTemplate.py +++ b/uniffi_bindgen/src/bindings/python/templates/OptionalTemplate.py @@ -1,6 +1,11 @@ {%- let inner_ffi_converter = inner_type|ffi_converter_name %} class {{ ffi_converter_name }}(_UniffiConverterRustBuffer): + @classmethod + def check(cls, value): + if value is not None: + {{ inner_ffi_converter }}.check(value) + @classmethod def write(cls, value, buf): if value is None: diff --git a/uniffi_bindgen/src/bindings/python/templates/RecordTemplate.py b/uniffi_bindgen/src/bindings/python/templates/RecordTemplate.py index 99a30e120f..b2420b6671 100644 --- a/uniffi_bindgen/src/bindings/python/templates/RecordTemplate.py +++ b/uniffi_bindgen/src/bindings/python/templates/RecordTemplate.py @@ -42,6 +42,16 @@ def read(buf): {%- endfor %} ) + @staticmethod + def check(value): + {%- if rec.fields().is_empty() %} + pass + {%- else %} + {%- for field in rec.fields() %} + {{ field|check_fn }}(value.{{ field.name()|var_name }}) + {%- endfor %} + {%- endif %} + @staticmethod def write(value, buf): {%- for field in rec.fields() %} diff --git a/uniffi_bindgen/src/bindings/python/templates/RustBufferHelper.py b/uniffi_bindgen/src/bindings/python/templates/RustBufferHelper.py index daabd5b4b9..88cc96b6fb 100644 --- a/uniffi_bindgen/src/bindings/python/templates/RustBufferHelper.py +++ b/uniffi_bindgen/src/bindings/python/templates/RustBufferHelper.py @@ -1,25 +1,13 @@ # Types conforming to `_UniffiConverterPrimitive` pass themselves directly over the FFI. class _UniffiConverterPrimitive: - @classmethod - def check(cls, value): - return value - @classmethod def lift(cls, value): return value - + @classmethod def lower(cls, value): - return cls.lowerUnchecked(cls.check(value)) - - @classmethod - def lowerUnchecked(cls, value): return value - @classmethod - def write(cls, value, buf): - cls.write_unchecked(cls.check(value), buf) - class _UniffiConverterPrimitiveInt(_UniffiConverterPrimitive): @classmethod def check(cls, value): @@ -31,7 +19,6 @@ def check(cls, value): raise TypeError("__index__ returned non-int (type {})".format(type(value).__name__)) if not cls.VALUE_MIN <= value < cls.VALUE_MAX: raise ValueError("{} requires {} <= value < {}".format(cls.CLASS_NAME, cls.VALUE_MIN, cls.VALUE_MAX)) - return super().check(value) class _UniffiConverterPrimitiveFloat(_UniffiConverterPrimitive): @classmethod @@ -42,7 +29,6 @@ def check(cls, value): raise TypeError("must be real number, not {}".format(type(value).__name__)) if not isinstance(value, float): raise TypeError("__float__ returned non-float (type {})".format(type(value).__name__)) - return super().check(value) # Helper class for wrapper types that will always go through a _UniffiRustBuffer. # Classes should inherit from this and implement the `read` and `write` static methods. diff --git a/uniffi_bindgen/src/bindings/python/templates/SequenceTemplate.py b/uniffi_bindgen/src/bindings/python/templates/SequenceTemplate.py index 3c9f5a4596..c69a36f178 100644 --- a/uniffi_bindgen/src/bindings/python/templates/SequenceTemplate.py +++ b/uniffi_bindgen/src/bindings/python/templates/SequenceTemplate.py @@ -1,6 +1,11 @@ {%- let inner_ffi_converter = inner_type|ffi_converter_name %} class {{ ffi_converter_name}}(_UniffiConverterRustBuffer): + @classmethod + def check(cls, value): + for item in value: + {{ inner_ffi_converter }}.check(item) + @classmethod def write(cls, value, buf): items = len(value) diff --git a/uniffi_bindgen/src/bindings/python/templates/StringHelper.py b/uniffi_bindgen/src/bindings/python/templates/StringHelper.py index 40890b6abc..80a014c888 100644 --- a/uniffi_bindgen/src/bindings/python/templates/StringHelper.py +++ b/uniffi_bindgen/src/bindings/python/templates/StringHelper.py @@ -15,7 +15,6 @@ def read(buf): @staticmethod def write(value, buf): - value = _UniffiConverterString.check(value) utf8_bytes = value.encode("utf-8") buf.write_i32(len(utf8_bytes)) buf.write(utf8_bytes) @@ -27,7 +26,6 @@ def lift(buf): @staticmethod def lower(value): - value = _UniffiConverterString.check(value) with _UniffiRustBuffer.alloc_with_builder() as builder: builder.write(value.encode("utf-8")) return builder.finalize() diff --git a/uniffi_bindgen/src/bindings/python/templates/TimestampHelper.py b/uniffi_bindgen/src/bindings/python/templates/TimestampHelper.py index 8402f6095d..9acbb2fee9 100644 --- a/uniffi_bindgen/src/bindings/python/templates/TimestampHelper.py +++ b/uniffi_bindgen/src/bindings/python/templates/TimestampHelper.py @@ -17,6 +17,10 @@ def read(buf): else: return datetime.datetime.fromtimestamp(0, tz=datetime.timezone.utc) - datetime.timedelta(seconds=-seconds, microseconds=microseconds) + @staticmethod + def check(value): + pass + @staticmethod def write(value, buf): if value >= datetime.datetime.fromtimestamp(0, datetime.timezone.utc): diff --git a/uniffi_bindgen/src/bindings/python/templates/TopLevelFunctionTemplate.py b/uniffi_bindgen/src/bindings/python/templates/TopLevelFunctionTemplate.py index f258b60a1c..8faa11bedf 100644 --- a/uniffi_bindgen/src/bindings/python/templates/TopLevelFunctionTemplate.py +++ b/uniffi_bindgen/src/bindings/python/templates/TopLevelFunctionTemplate.py @@ -1,6 +1,7 @@ {%- if func.is_async() %} def {{ func.name()|fn_name }}({%- call py::arg_list_decl(func) -%}): + {%- call py::setup_args(func) %} return _uniffi_rust_call_async( _UniffiLib.{{ func.ffi_func().name() }}({% call py::arg_list_lowered(func) %}), _UniffiLib.{{func.ffi_rust_future_poll(ci) }}, diff --git a/uniffi_bindgen/src/bindings/python/templates/UInt16Helper.py b/uniffi_bindgen/src/bindings/python/templates/UInt16Helper.py index 081c6731ce..039bf76162 100644 --- a/uniffi_bindgen/src/bindings/python/templates/UInt16Helper.py +++ b/uniffi_bindgen/src/bindings/python/templates/UInt16Helper.py @@ -8,5 +8,5 @@ def read(buf): return buf.read_u16() @staticmethod - def write_unchecked(value, buf): + def write(value, buf): buf.write_u16(value) diff --git a/uniffi_bindgen/src/bindings/python/templates/UInt32Helper.py b/uniffi_bindgen/src/bindings/python/templates/UInt32Helper.py index b80e75177d..1650bf9b60 100644 --- a/uniffi_bindgen/src/bindings/python/templates/UInt32Helper.py +++ b/uniffi_bindgen/src/bindings/python/templates/UInt32Helper.py @@ -8,5 +8,5 @@ def read(buf): return buf.read_u32() @staticmethod - def write_unchecked(value, buf): + def write(value, buf): buf.write_u32(value) diff --git a/uniffi_bindgen/src/bindings/python/templates/UInt64Helper.py b/uniffi_bindgen/src/bindings/python/templates/UInt64Helper.py index 4b87e58547..f354545e26 100644 --- a/uniffi_bindgen/src/bindings/python/templates/UInt64Helper.py +++ b/uniffi_bindgen/src/bindings/python/templates/UInt64Helper.py @@ -8,5 +8,5 @@ def read(buf): return buf.read_u64() @staticmethod - def write_unchecked(value, buf): + def write(value, buf): buf.write_u64(value) diff --git a/uniffi_bindgen/src/bindings/python/templates/UInt8Helper.py b/uniffi_bindgen/src/bindings/python/templates/UInt8Helper.py index 33026706f2..cee130b4d9 100644 --- a/uniffi_bindgen/src/bindings/python/templates/UInt8Helper.py +++ b/uniffi_bindgen/src/bindings/python/templates/UInt8Helper.py @@ -8,5 +8,5 @@ def read(buf): return buf.read_u8() @staticmethod - def write_unchecked(value, buf): + def write(value, buf): buf.write_u8(value) diff --git a/uniffi_bindgen/src/bindings/python/templates/macros.py b/uniffi_bindgen/src/bindings/python/templates/macros.py index 99d48d343b..3359e759e6 100644 --- a/uniffi_bindgen/src/bindings/python/templates/macros.py +++ b/uniffi_bindgen/src/bindings/python/templates/macros.py @@ -70,6 +70,7 @@ #} {%- macro setup_args(func) %} {%- for arg in func.arguments() %} + {{ arg|check_fn }}({{ arg.name()|var_name }}) {%- match arg.default_value() %} {%- when None %} {%- when Some with(literal) %} @@ -85,6 +86,7 @@ #} {%- macro setup_args_extra_indent(func) %} {%- for arg in func.arguments() %} + {{ arg|check_fn }}({{ arg.name()|var_name }}) {%- match arg.default_value() %} {%- when None %} {%- when Some with(literal) %} diff --git a/uniffi_bindgen/src/bindings/ruby/gen_ruby/mod.rs b/uniffi_bindgen/src/bindings/ruby/gen_ruby/mod.rs index 2471c6aafc..d8bb65df47 100644 --- a/uniffi_bindgen/src/bindings/ruby/gen_ruby/mod.rs +++ b/uniffi_bindgen/src/bindings/ruby/gen_ruby/mod.rs @@ -265,6 +265,22 @@ mod filters { }) } + pub fn check_rb(nm: &str, type_: &Type) -> Result { + Ok(match type_ { + Type::Object { name, .. } => format!("({}._uniffi_check {nm})", class_name_rb(name)?), + Type::Enum { .. } + | Type::Record { .. } + | Type::Optional { .. } + | Type::Sequence { .. } + | Type::Map { .. } => format!( + "RustBuffer.check_{}({})", + class_name_rb(&canonical_name(type_))?, + nm + ), + _ => "".to_owned(), + }) + } + pub fn lower_rb(nm: &str, type_: &Type) -> Result { Ok(match type_ { Type::Int8 diff --git a/uniffi_bindgen/src/bindings/ruby/templates/ObjectTemplate.rb b/uniffi_bindgen/src/bindings/ruby/templates/ObjectTemplate.rb index 4482ccfe88..e221a7c427 100644 --- a/uniffi_bindgen/src/bindings/ruby/templates/ObjectTemplate.rb +++ b/uniffi_bindgen/src/bindings/ruby/templates/ObjectTemplate.rb @@ -29,20 +29,23 @@ def _uniffi_clone_handle() return @handle end - # A private helper for lowering instances into a handle. + # Private helpers for lowering instances into a handle. # This does an explicit typecheck, because accidentally lowering a different type of # object in a place where this type is expected, could lead to memory unsafety. - def self._uniffi_lower(inst) + def self._uniffi_check(inst) if not inst.is_a? self raise TypeError.new "Expected a {{ obj.name()|class_name_rb }} instance, got #{inst}" end + end + + def self._uniffi_lower(inst) return inst._uniffi_clone_handle() end {%- match obj.primary_constructor() %} {%- when Some with (cons) %} def initialize({% call rb::arg_list_decl(cons) -%}) - {%- call rb::coerce_args_extra_indent(cons) %} + {%- call rb::setup_args_extra_indent(cons) %} handle = {% call rb::to_ffi_call(cons) %} @handle = handle ObjectSpace.define_finalizer(self, self.class._uniffi_define_finalizer_by_handle(handle, self.object_id)) @@ -52,7 +55,7 @@ def initialize({% call rb::arg_list_decl(cons) -%}) {% for cons in obj.alternate_constructors() -%} def self.{{ cons.name()|fn_name_rb }}({% call rb::arg_list_decl(cons) %}) - {%- call rb::coerce_args_extra_indent(cons) %} + {%- call rb::setup_args_extra_indent(cons) %} # Call the (fallible) function before creating any half-baked object instances. # Lightly yucky way to bypass the usual "initialize" logic # and just create a new instance with the required handle. @@ -65,14 +68,14 @@ def self.{{ cons.name()|fn_name_rb }}({% call rb::arg_list_decl(cons) %}) {%- when Some with (return_type) -%} def {{ meth.name()|fn_name_rb }}({% call rb::arg_list_decl(meth) %}) - {%- call rb::coerce_args_extra_indent(meth) %} + {%- call rb::setup_args_extra_indent(meth) %} result = {% call rb::to_ffi_call_with_prefix("_uniffi_clone_handle()", meth) %} return {{ "result"|lift_rb(return_type) }} end {%- when None -%} def {{ meth.name()|fn_name_rb }}({% call rb::arg_list_decl(meth) %}) - {%- call rb::coerce_args_extra_indent(meth) %} + {%- call rb::setup_args_extra_indent(meth) %} {% call rb::to_ffi_call_with_prefix("_uniffi_clone_handle()", meth) %} end {% endmatch %} diff --git a/uniffi_bindgen/src/bindings/ruby/templates/RustBufferTemplate.rb b/uniffi_bindgen/src/bindings/ruby/templates/RustBufferTemplate.rb index 0194c9666d..154867e7ee 100644 --- a/uniffi_bindgen/src/bindings/ruby/templates/RustBufferTemplate.rb +++ b/uniffi_bindgen/src/bindings/ruby/templates/RustBufferTemplate.rb @@ -128,6 +128,12 @@ def consumeInto{{ canonical_type_name }} {%- let rec = ci|get_record_definition(record_name) -%} # The Record type {{ record_name }}. + def self.check_{{ canonical_type_name }}(v) + {%- for field in rec.fields() %} + {{ "v.{}"|format(field.name()|var_name_rb)|check_rb(field.as_type().borrow()) }} + {%- endfor %} + end + def self.alloc_from_{{ canonical_type_name }}(v) RustBuffer.allocWithBuilder do |builder| builder.write_{{ canonical_type_name }}(v) @@ -146,6 +152,19 @@ def consumeInto{{ canonical_type_name }} {%- let e = ci|get_enum_definition(enum_name) -%} # The Enum type {{ enum_name }}. + def self.check_{{ canonical_type_name }}(v) + {%- if !e.is_flat() %} + {%- for variant in e.variants() %} + if v.{{ variant.name()|var_name_rb }}? + {%- for field in variant.fields() %} + {{ "v.{}"|format(field.name())|check_rb(field.as_type().borrow()) }} + {%- endfor %} + return + end + {%- endfor %} + {%- endif %} + end + def self.alloc_from_{{ canonical_type_name }}(v) RustBuffer.allocWithBuilder do |builder| builder.write_{{ canonical_type_name }}(v) @@ -162,6 +181,12 @@ def consumeInto{{ canonical_type_name }} {% when Type::Optional { inner_type } -%} # The Optional type for {{ canonical_name(inner_type) }}. + + def self.check_{{ canonical_type_name }}(v) + if not v.nil? + {{ "v"|check_rb(inner_type.borrow()) }} + end + end def self.alloc_from_{{ canonical_type_name }}(v) RustBuffer.allocWithBuilder do |builder| @@ -179,6 +204,12 @@ def consumeInto{{ canonical_type_name }} {% when Type::Sequence { inner_type } -%} # The Sequence type for {{ canonical_name(inner_type) }}. + def self.check_{{ canonical_type_name }}(v) + v.each do |item| + {{ "item"|check_rb(inner_type.borrow()) }} + end + end + def self.alloc_from_{{ canonical_type_name }}(v) RustBuffer.allocWithBuilder do |builder| builder.write_{{ canonical_type_name }}(v) @@ -195,6 +226,13 @@ def consumeInto{{ canonical_type_name }} {% when Type::Map { key_type: k, value_type: inner_type } -%} # The Map type for {{ canonical_name(inner_type) }}. + def self.check_{{ canonical_type_name }}(v) + v.each do |k, v| + {{ "k"|check_rb(k.borrow()) }} + {{ "v"|check_rb(inner_type.borrow()) }} + end + end + def self.alloc_from_{{ canonical_type_name }}(v) RustBuffer.allocWithBuilder do |builder| builder.write_{{ canonical_type_name }}(v) diff --git a/uniffi_bindgen/src/bindings/ruby/templates/TopLevelFunctionTemplate.rb b/uniffi_bindgen/src/bindings/ruby/templates/TopLevelFunctionTemplate.rb index 13214cf31b..b6dce0effa 100644 --- a/uniffi_bindgen/src/bindings/ruby/templates/TopLevelFunctionTemplate.rb +++ b/uniffi_bindgen/src/bindings/ruby/templates/TopLevelFunctionTemplate.rb @@ -2,7 +2,7 @@ {%- when Some with (return_type) %} def self.{{ func.name()|fn_name_rb }}({%- call rb::arg_list_decl(func) -%}) - {%- call rb::coerce_args(func) %} + {%- call rb::setup_args(func) %} result = {% call rb::to_ffi_call(func) %} return {{ "result"|lift_rb(return_type) }} end @@ -10,7 +10,7 @@ def self.{{ func.name()|fn_name_rb }}({%- call rb::arg_list_decl(func) -%}) {% when None %} def self.{{ func.name()|fn_name_rb }}({%- call rb::arg_list_decl(func) -%}) - {%- call rb::coerce_args(func) %} + {%- call rb::setup_args(func) %} {% call rb::to_ffi_call(func) %} end {% endmatch %} diff --git a/uniffi_bindgen/src/bindings/ruby/templates/macros.rb b/uniffi_bindgen/src/bindings/ruby/templates/macros.rb index 8dc3e5e613..e9962f5831 100644 --- a/uniffi_bindgen/src/bindings/ruby/templates/macros.rb +++ b/uniffi_bindgen/src/bindings/ruby/templates/macros.rb @@ -60,14 +60,16 @@ [{%- for arg in func.arguments() -%}{{ arg.type_().borrow()|type_ffi }}, {% endfor -%} RustCallStatus.by_ref] {%- endmacro -%} -{%- macro coerce_args(func) %} +{%- macro setup_args(func) %} {%- for arg in func.arguments() %} - {{ arg.name() }} = {{ arg.name()|coerce_rb(ci.namespace()|class_name_rb, arg.as_type().borrow()) -}} + {{ arg.name() }} = {{ arg.name()|coerce_rb(ci.namespace()|class_name_rb, arg.as_type().borrow()) }} + {{ arg.name()|check_rb(arg.as_type().borrow()) }} {% endfor -%} {%- endmacro -%} -{%- macro coerce_args_extra_indent(func) %} - {%- for arg in func.arguments() %} +{%- macro setup_args_extra_indent(meth) %} + {%- for arg in meth.arguments() %} {{ arg.name() }} = {{ arg.name()|coerce_rb(ci.namespace()|class_name_rb, arg.as_type().borrow()) }} + {{ arg.name()|check_rb(arg.as_type().borrow()) }} {%- endfor %} {%- endmacro -%} diff --git a/uniffi_bindgen/src/bindings/swift/templates/Slab.swift b/uniffi_bindgen/src/bindings/swift/templates/Slab.swift index d96c1b5d31..5d1ac30601 100644 --- a/uniffi_bindgen/src/bindings/swift/templates/Slab.swift +++ b/uniffi_bindgen/src/bindings/swift/templates/Slab.swift @@ -1,4 +1,4 @@ -func uniffiHandleIsFromRust(_ handle: Int64) -> Bool { +fileprivate func uniffiHandleIsFromRust(_ handle: Int64) -> Bool { return (handle & 0x0001_0000_0000) == 0 }