From 51e5417bd37eecbf6db2f2f5e37e5155db039996 Mon Sep 17 00:00:00 2001 From: Campbell Barton Date: Tue, 14 Mar 2023 15:50:46 +1100 Subject: [PATCH] Fix #105678: Crash assigning Image.pixels to an undersized sequence Now only dynamic function parameters that use ParameterDynAlloc support dynamically sized parameters arrays. Add tests for both dynamic arrays that don't support resizing (Image.pixels) and dynamic sized arguments using (VertexGroup.add(index=[..])). Regression in [0] which extended support for dynamic sized function arguments. [0]: dfb8c5974e6f937c3404d0dd59f0e6424de455db --- source/blender/python/intern/bpy_rna_array.c | 55 +++++++++------ tests/python/bl_pyapi_prop_array.py | 70 ++++++++++++++++++++ 2 files changed, 105 insertions(+), 20 deletions(-) diff --git a/source/blender/python/intern/bpy_rna_array.c b/source/blender/python/intern/bpy_rna_array.c index 48ba028edf0..07f63d16deb 100644 --- a/source/blender/python/intern/bpy_rna_array.c +++ b/source/blender/python/intern/bpy_rna_array.c @@ -247,6 +247,7 @@ static int count_items(PyObject *seq, int dim) static int validate_array_length(PyObject *rvalue, PointerRNA *ptr, PropertyRNA *prop, + const bool prop_is_param_dyn_alloc, int lvalue_dim, int *r_totitem, const char *error_prefix) @@ -266,26 +267,20 @@ static int validate_array_length(PyObject *rvalue, return -1; } if ((RNA_property_flag(prop) & PROP_DYNAMIC) && lvalue_dim == 0) { - if (RNA_property_array_length(ptr, prop) != tot) { -#if 0 - /* length is flexible */ - if (!RNA_property_dynamic_array_set_length(ptr, prop, tot)) { - /* BLI_snprintf(error_str, error_str_size, - * "%s.%s: array length cannot be changed to %d", - * RNA_struct_identifier(ptr->type), RNA_property_identifier(prop), tot); */ + const int tot_expected = RNA_property_array_length(ptr, prop); + if (tot_expected != tot) { + *r_totitem = tot; + if (!prop_is_param_dyn_alloc) { PyErr_Format(PyExc_ValueError, - "%s %s.%s: array length cannot be changed to %d", + "%s %s.%s: array length cannot be changed to %d (expected %d)", error_prefix, RNA_struct_identifier(ptr->type), RNA_property_identifier(prop), - tot); + tot, + tot_expected); return -1; } -#else - *r_totitem = tot; return 0; - -#endif } len = tot; @@ -340,6 +335,7 @@ static int validate_array_length(PyObject *rvalue, static int validate_array(PyObject *rvalue, PointerRNA *ptr, PropertyRNA *prop, + const bool prop_is_param_dyn_alloc, int lvalue_dim, ItemTypeCheckFunc check_item_type, const char *item_type_str, @@ -410,7 +406,8 @@ static int validate_array(PyObject *rvalue, return -1; } - return validate_array_length(rvalue, ptr, prop, lvalue_dim, r_totitem, error_prefix); + return validate_array_length( + rvalue, ptr, prop, prop_is_param_dyn_alloc, lvalue_dim, r_totitem, error_prefix); } } @@ -527,15 +524,26 @@ static int py_to_array(PyObject *seq, char *data = NULL; // totdim = RNA_property_array_dimension(ptr, prop, dim_size); /* UNUSED */ + const int flag = RNA_property_flag(prop); - if (validate_array(seq, ptr, prop, 0, check_item_type, item_type_str, &totitem, error_prefix) == - -1) { + /* Use #ParameterDynAlloc which defines it's own array length. */ + const bool prop_is_param_dyn_alloc = param_data && (flag & PROP_DYNAMIC); + + if (validate_array(seq, + ptr, + prop, + prop_is_param_dyn_alloc, + 0, + check_item_type, + item_type_str, + &totitem, + error_prefix) == -1) { return -1; } if (totitem) { /* NOTE: this code is confusing. */ - if (param_data && RNA_property_flag(prop) & PROP_DYNAMIC) { + if (prop_is_param_dyn_alloc) { /* not freeing allocated mem, RNA_parameter_list_free() will do this */ ParameterDynAlloc *param_alloc = (ParameterDynAlloc *)param_data; param_alloc->array_tot = (int)totitem; @@ -626,9 +634,16 @@ static int py_to_array_index(PyObject *py, copy_value_single(py, ptr, prop, NULL, 0, &index, convert_item, rna_set_index); } else { - if (validate_array( - py, ptr, prop, lvalue_dim, check_item_type, item_type_str, &totitem, error_prefix) == - -1) { + const bool prop_is_param_dyn_alloc = false; + if (validate_array(py, + ptr, + prop, + prop_is_param_dyn_alloc, + lvalue_dim, + check_item_type, + item_type_str, + &totitem, + error_prefix) == -1) { return -1; } diff --git a/tests/python/bl_pyapi_prop_array.py b/tests/python/bl_pyapi_prop_array.py index 1132914b14c..5595325d60d 100644 --- a/tests/python/bl_pyapi_prop_array.py +++ b/tests/python/bl_pyapi_prop_array.py @@ -195,6 +195,76 @@ class TestPropArrayMultiDimensional(unittest.TestCase): del id_type.temp +class TestPropArrayDynamicAssign(unittest.TestCase): + """ + Pixels are dynamic in the sense the size can change however the assignment does not define the size. + """ + + dims = 12 + + def setUp(self): + self.image = bpy.data.images.new("", self.dims, self.dims) + + def tearDown(self): + bpy.data.images.remove(self.image) + self.image = None + + def test_assign_fixed_under_1px(self): + image = self.image + with self.assertRaises(ValueError): + image.pixels = [1.0, 1.0, 1.0, 1.0] + + def test_assign_fixed_under_0px(self): + image = self.image + with self.assertRaises(ValueError): + image.pixels = [] + + def test_assign_fixed_over_by_1px(self): + image = self.image + with self.assertRaises(ValueError): + image.pixels = ([1.0, 1.0, 1.0, 1.0] * (self.dims * self.dims)) + [1.0] + + def test_assign_fixed(self): + # Valid assignment, ensure it works as intended. + image = self.image + values = [1.0, 0.0, 1.0, 0.0] * (self.dims * self.dims) + image.pixels = values + self.assertEqual(tuple(values), tuple(image.pixels)) + + +class TestPropArrayDynamicArg(unittest.TestCase): + """ + Index array, a dynamic array argument which defines it's own length. + """ + + dims = 8 + + def setUp(self): + self.me = bpy.data.meshes.new("") + self.me.vertices.add(self.dims) + self.ob = bpy.data.objects.new("", self.me) + + def tearDown(self): + bpy.data.objects.remove(self.ob) + bpy.data.meshes.remove(self.me) + self.me = None + self.ob = None + + def test_param_dynamic(self): + ob = self.ob + vg = ob.vertex_groups.new(name="") + + # Add none. + vg.add(index=(), weight=1.0, type='REPLACE') + for i in range(self.dims): + with self.assertRaises(RuntimeError): + vg.weight(i) + + # Add all. + vg.add(index=range(self.dims), weight=1.0, type='REPLACE') + self.assertEqual(tuple([1.0] * self.dims), tuple([vg.weight(i) for i in range(self.dims)])) + + if __name__ == '__main__': import sys sys.argv = [__file__] + (sys.argv[sys.argv.index("--") + 1:] if "--" in sys.argv else [])