Skip to content

Commit 65fb303

Browse files
authored
Interpreter: Fix setting a struct with less fields than expected (#10120)
It shouldn't leave bad values in the interpreter which later can cause panic anywhere
1 parent 164b58a commit 65fb303

File tree

3 files changed

+70
-6
lines changed

3 files changed

+70
-6
lines changed

internal/interpreter/api.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1418,7 +1418,7 @@ impl ComponentInstance {
14181418
.get_global(comp.borrow(), &&normalize_identifier(global))
14191419
.map_err(|()| SetPropertyError::NoSuchProperty)? // FIXME: should there be a NoSuchGlobal error?
14201420
.as_ref()
1421-
.set_property(&&normalize_identifier(property), value)
1421+
.set_property(&normalize_identifier(property), value)
14221422
}
14231423

14241424
/// Set a handler for the callback in the exported global singleton. A callback with that

internal/interpreter/eval.rs

Lines changed: 16 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1622,7 +1622,7 @@ pub fn store_property(
16221622
component_instance: InstanceRef,
16231623
element: &ElementRc,
16241624
name: &str,
1625-
value: Value,
1625+
mut value: Value,
16261626
) -> Result<(), SetPropertyError> {
16271627
generativity::make_guard!(guard);
16281628
match enclosing_component_instance_for_element(
@@ -1653,7 +1653,7 @@ pub fn store_property(
16531653
.get(name)
16541654
{
16551655
// Do an extra type checking because PropertyInfo::set won't do it for custom structures or array
1656-
if !check_value_type(&value, &orig_decl.property_type) {
1656+
if !check_value_type(&mut value, &orig_decl.property_type) {
16571657
return Err(SetPropertyError::WrongType);
16581658
}
16591659
}
@@ -1682,7 +1682,7 @@ pub fn store_property(
16821682
}
16831683

16841684
/// Return true if the Value can be used for a property of the given type
1685-
fn check_value_type(value: &Value, ty: &Type) -> bool {
1685+
fn check_value_type(value: &mut Value, ty: &Type) -> bool {
16861686
match ty {
16871687
Type::Void => true,
16881688
Type::Invalid
@@ -1711,10 +1711,21 @@ fn check_value_type(value: &Value, ty: &Type) -> bool {
17111711
Type::Easing => matches!(value, Value::EasingCurve(_)),
17121712
Type::Brush => matches!(value, Value::Brush(_)),
17131713
Type::Array(inner) => {
1714-
matches!(value, Value::Model(m) if m.iter().all(|v| check_value_type(&v, inner)))
1714+
matches!(value, Value::Model(m) if m.iter().all(|mut v| check_value_type(&mut v, inner)))
17151715
}
17161716
Type::Struct(s) => {
1717-
matches!(value, Value::Struct(str) if str.iter().all(|(k, v)| s.fields.get(k).is_some_and(|ty| check_value_type(v, ty))))
1717+
let Value::Struct(str) = value else { return false };
1718+
if !str
1719+
.0
1720+
.iter_mut()
1721+
.all(|(k, v)| s.fields.get(k).is_some_and(|ty| check_value_type(v, ty)))
1722+
{
1723+
return false;
1724+
}
1725+
for (k, v) in &s.fields {
1726+
str.0.entry(k.clone()).or_insert_with(|| default_value_for_type(v));
1727+
}
1728+
true
17181729
}
17191730
Type::Enumeration(en) => {
17201731
matches!(value, Value::EnumerationValue(name, _) if name == en.name.as_str())

internal/interpreter/tests.rs

Lines changed: 53 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -44,3 +44,56 @@ fn reuse_window() {
4444
instance
4545
};
4646
}
47+
48+
#[test]
49+
fn set_wrong_struct() {
50+
i_slint_backend_testing::init_no_event_loop();
51+
use crate::{Compiler, Struct, Value};
52+
let code = r#"
53+
struct TimeStruct {
54+
Clock: string,
55+
Enabled: bool,
56+
}
57+
58+
export global Device {
59+
in-out property <TimeStruct> Time: { Clock: "11:37", Enabled: true };
60+
}
61+
62+
export component Clock {
63+
ta := TouchArea {
64+
enabled: Device.Time.Enabled;
65+
}
66+
out property <bool> ta_enabled: ta.enabled;
67+
out property <string> time: Device.Time.Clock;
68+
}
69+
"#;
70+
let compiler = Compiler::default();
71+
let result = spin_on::spin_on(compiler.build_from_source(code.into(), Default::default()));
72+
assert!(!result.has_errors(), "{:?}", result.diagnostics().collect::<Vec<_>>());
73+
let definition = result.component("Clock").unwrap();
74+
let instance = definition.create().unwrap();
75+
assert_eq!(instance.get_property("ta_enabled").unwrap(), Value::from(true));
76+
assert_eq!(instance.get_property("time").unwrap(), Value::String("11:37".into()));
77+
// This only has "Clock" but no "Enabled"
78+
instance
79+
.set_global_property(
80+
"Device",
81+
"Time",
82+
Struct::from_iter([("Clock".into(), Value::String("10:37".into()))]).into(),
83+
)
84+
.unwrap();
85+
assert_eq!(instance.get_property("ta_enabled").unwrap(), Value::from(false));
86+
assert_eq!(instance.get_property("time").unwrap(), Value::String("10:37".into()));
87+
88+
// Setting a struct with wrong fields leads to an error
89+
assert_eq!(
90+
instance.set_global_property(
91+
"Device",
92+
"Time",
93+
Struct::from_iter([("Broken".into(), Value::Number(12.1))]).into(),
94+
),
95+
Err(crate::SetPropertyError::WrongType)
96+
);
97+
assert_eq!(instance.get_property("ta_enabled").unwrap(), Value::from(false));
98+
assert_eq!(instance.get_property("time").unwrap(), Value::String("10:37".into()));
99+
}

0 commit comments

Comments
 (0)