Skip to content

Commit

Permalink
Updated lower validation for Python/Ruby
Browse files Browse the repository at this point in the history
I want to fix mozilla#1797 by cloning the object handle in lower().  However,
this presents an issue in the dynamic languages because they perform
various failable checks in lower(): type checks, bounds checks, etc.  If
the check fails, then the cloned handle will be leaked.

Prevent this by adding a `check()` method.  We do the check first then
once we get to `lower()`, nothing can fail.
  • Loading branch information
bendk committed Nov 2, 2023
1 parent 56300c3 commit 538fcdc
Show file tree
Hide file tree
Showing 34 changed files with 199 additions and 50 deletions.
4 changes: 4 additions & 0 deletions uniffi_bindgen/src/bindings/python/gen_python/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -413,6 +413,10 @@ pub mod filters {
Ok(format!("{}.lift", ffi_converter_name(as_ct)?))
}

pub fn check_fn(as_ct: &impl AsCodeType) -> Result<String, askama::Error> {
Ok(format!("{}.check", ffi_converter_name(as_ct)?))
}

pub fn lower_fn(as_ct: &impl AsCodeType) -> Result<String, askama::Error> {
Ok(format!("{}.lower", ffi_converter_name(as_ct)?))
}
Expand Down
16 changes: 10 additions & 6 deletions uniffi_bindgen/src/bindings/python/templates/BooleanHelper.py
Original file line number Diff line number Diff line change
@@ -1,16 +1,20 @@
class _UniffiConverterBool(_UniffiConverterPrimitive):
class _UniffiConverterBool:
@classmethod
def check(cls, value):
return not not value

@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
5 changes: 4 additions & 1 deletion uniffi_bindgen/src/bindings/python/templates/BytesHelper.py
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,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._handle_map.new_handle(cb)
Expand Down
9 changes: 9 additions & 0 deletions uniffi_bindgen/src/bindings/python/templates/CustomType.py
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down Expand Up @@ -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") }}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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)
19 changes: 19 additions & 0 deletions uniffi_bindgen/src/bindings/python/templates/EnumTemplate.py
Original file line number Diff line number Diff line change
Expand Up @@ -81,6 +81,25 @@ 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 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 %}

@staticmethod
def write(value, buf):
{%- for variant in e.variants() %}
{%- if e.is_flat() %}
Expand Down
14 changes: 14 additions & 0 deletions uniffi_bindgen/src/bindings/python/templates/ErrorTemplate.py
Original file line number Diff line number Diff line change
Expand Up @@ -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() %}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Original file line number Diff line number Diff line change
Expand Up @@ -12,9 +12,12 @@ class {{ ffi_converter_name }}:
_handle_map = UniffiHandleMap()

@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._handle_map.new_handle(eventloop)

@classmethod
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Original file line number Diff line number Diff line change
Expand Up @@ -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)
2 changes: 1 addition & 1 deletion uniffi_bindgen/src/bindings/python/templates/Int8Helper.py
Original file line number Diff line number Diff line change
Expand Up @@ -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)
6 changes: 6 additions & 0 deletions uniffi_bindgen/src/bindings/python/templates/MapTemplate.py
Original file line number Diff line number Diff line change
Expand Up @@ -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))
Expand Down
10 changes: 9 additions & 1 deletion uniffi_bindgen/src/bindings/python/templates/ObjectTemplate.py
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,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 %}
Expand Down Expand Up @@ -83,6 +82,15 @@ class {{ ffi_converter_name }}:
def lift(value: UniffiHandle):
return {{ impl_name }}._make_instance_(value)

@staticmethod
def check(value: {{ type_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: {{ type_name }}):
{%- match obj.imp() %}
Expand Down
Original file line number Diff line number Diff line change
@@ -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:
Expand Down
10 changes: 10 additions & 0 deletions uniffi_bindgen/src/bindings/python/templates/RecordTemplate.py
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,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):
{%- if rec.has_fields() %}
Expand Down
16 changes: 1 addition & 15 deletions uniffi_bindgen/src/bindings/python/templates/RustBufferHelper.py
Original file line number Diff line number Diff line change
@@ -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):
Expand All @@ -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
Expand All @@ -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.
Expand Down
Original file line number Diff line number Diff line change
@@ -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)
Expand Down
2 changes: 0 additions & 2 deletions uniffi_bindgen/src/bindings/python/templates/StringHelper.py
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand All @@ -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()
Original file line number Diff line number Diff line change
Expand Up @@ -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):
Expand Down
Original file line number Diff line number Diff line change
@@ -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) }},
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Loading

0 comments on commit 538fcdc

Please sign in to comment.