Fix #112445: Possible to create invalid attribute with rename

Renaming attributes in the property editor didn't take into account
the required domains and types for builtin attributes. It might be
possible to trigger crashes by using a builtin name with the wrong
metadata. At the very least it would be confusing.

This commit extends the attribute provider check for builtin
attributes to give the required domain and type. We check that in
the attribute rename function to make sure the new name is valid.

Pull Request: https://projects.blender.org/blender/blender/pulls/131702

source/blender/blenkernel/BKE_attribute.hh

@@ -463,7 +463,8 @@ class AttributeIter {
 struct AttributeAccessorFunctions {
   bool (*domain_supported)(const void *owner, AttrDomain domain);
   int (*domain_size)(const void *owner, AttrDomain domain);
-  bool (*is_builtin)(const void *owner, StringRef attribute_id);
+  std::optional<AttributeDomainAndType> (*builtin_domain_and_type)(const void *owner,
+                                                                   StringRef attribute_id);
   GAttributeReader (*lookup)(const void *owner, StringRef attribute_id);
   GVArray (*adapt_domain)(const void *owner,
                           const GVArray &varray,
@@ -549,7 +550,15 @@ class AttributeAccessor {
    */
   bool is_builtin(const StringRef attribute_id) const
   {
-    return fn_->is_builtin(owner_, attribute_id);
+    return fn_->builtin_domain_and_type(owner_, attribute_id).has_value();
+  }
+
+  /**
+   * \return The required domain and type for the attribute, if it is builtin.
+   */
+  std::optional<AttributeDomainAndType> get_builtin_domain_and_type(const StringRef name) const
+  {
+    return fn_->builtin_domain_and_type(owner_, name);
   }
 
   /**

source/blender/blenkernel/intern/attribute.cc

@@ -220,33 +220,43 @@ static bool bke_attribute_rename_if_exists(AttributeOwner &owner,
   return BKE_attribute_rename(owner, old_name, new_name, reports);
 }
 
-static bool mesh_edit_mode_attribute_valid(const blender::StringRef name,
-                                           const AttrDomain domain,
-                                           const eCustomDataType data_type,
-                                           ReportList *reports)
+static bool name_valid_for_builtin_domain_and_type(
+    const blender::bke::AttributeAccessor attributes,
+    const blender::StringRefNull name,
+    const AttrDomain domain,
+    const eCustomDataType data_type,
+    ReportList *reports)
 {
-  if (ELEM(name,
-           "position",
-           ".edge_verts",
-           ".corner_vert",
-           ".corner_edge",
-           "sharp_edge",
-           "sharp_face",
-           "material_index"))
-  {
-    BKE_report(reports, RPT_ERROR, "Unable to create builtin attribute in edit mode");
-    return false;
+  if (const std::optional metadata = attributes.get_builtin_domain_and_type(name)) {
+    if (domain != metadata->domain) {
+      BKE_reportf(reports, RPT_ERROR, "Domain unsupported for \"%s\" attribute", name.c_str());
+      return false;
+    }
+    if (data_type != metadata->data_type) {
+      BKE_reportf(reports, RPT_ERROR, "Type unsupported for \"%s\" attribute", name.c_str());
+      return false;
+    }
   }
-  if (name == "id") {
-    if (domain != AttrDomain::Point) {
-      BKE_report(reports, RPT_ERROR, "Domain unsupported for \"id\" attribute");
-      return false;
-    }
-    if (data_type != CD_PROP_INT32) {
-      BKE_report(reports, RPT_ERROR, "Type unsupported for \"id\" attribute");
+  return true;
+}
+
+static bool mesh_attribute_valid(const Mesh &mesh,
+                                 const blender::StringRefNull name,
+                                 const AttrDomain domain,
+                                 const eCustomDataType data_type,
+                                 ReportList *reports)
+{
+  using namespace blender;
+  if (mesh.runtime->edit_mesh) {
+    if (BM_attribute_stored_in_bmesh_builtin(name)) {
+      BKE_report(reports, RPT_ERROR, "Unable to create attribute in edit mode");
       return false;
     }
   }
+  if (!name_valid_for_builtin_domain_and_type(mesh.attributes(), name, domain, data_type, reports))
+  {
+    return false;
+  }
   return true;
 }
 
@@ -287,12 +297,24 @@ bool BKE_attribute_rename(AttributeOwner &owner,
 
   if (owner.type() == AttributeOwnerType::Mesh) {
     Mesh *mesh = owner.get_mesh();
-    if (mesh->runtime->edit_mesh) {
-      if (!mesh_edit_mode_attribute_valid(
-              new_name, BKE_attribute_domain(owner, layer), eCustomDataType(layer->type), reports))
-      {
-        return false;
-      }
+    if (!mesh_attribute_valid(*mesh,
+                              new_name,
+                              BKE_attribute_domain(owner, layer),
+                              eCustomDataType(layer->type),
+                              reports))
+    {
+      return false;
+    }
+  }
+  else if (owner.type() == AttributeOwnerType::Curves) {
+    Curves *curves = owner.get_curves();
+    if (!name_valid_for_builtin_domain_and_type(curves->geometry.wrap().attributes(),
+                                                new_name,
+                                                BKE_attribute_domain(owner, layer),
+                                                eCustomDataType(layer->type),
+                                                reports))
+    {
+      return false;
     }
   }
 
@@ -385,7 +407,7 @@ CustomDataLayer *BKE_attribute_new(AttributeOwner &owner,
   if (owner.type() == AttributeOwnerType::Mesh) {
     Mesh *mesh = owner.get_mesh();
     if (BMEditMesh *em = mesh->runtime->edit_mesh.get()) {
-      if (!mesh_edit_mode_attribute_valid(name, domain, type, reports)) {
+      if (!mesh_attribute_valid(*mesh, name, domain, type, reports)) {
         return nullptr;
       }
       BM_data_layer_add_named(em->bm, customdata, type, uniquename.c_str());

source/blender/blenkernel/intern/attribute_access_intern.hh

@@ -261,9 +261,15 @@ class GeometryAttributeProviders {
 namespace attribute_accessor_functions {
 
 template<const GeometryAttributeProviders &providers>
-inline bool is_builtin(const void * /*owner*/, const StringRef name)
+inline std::optional<AttributeDomainAndType> builtin_domain_and_type(const void * /*owner*/,
+                                                                     const StringRef name)
 {
-  return providers.builtin_attribute_providers().contains_as(name);
+  if (const BuiltinAttributeProvider *provider =
+          providers.builtin_attribute_providers().lookup_default_as(name, nullptr))
+  {
+    return AttributeDomainAndType{provider->domain(), provider->data_type()};
+  }
+  return std::nullopt;
 }
 
 template<const GeometryAttributeProviders &providers>
@@ -391,7 +397,7 @@ inline AttributeAccessorFunctions accessor_functions_for_providers()
 {
   return AttributeAccessorFunctions{nullptr,
                                     nullptr,
-                                    is_builtin<providers>,
+                                    builtin_domain_and_type<providers>,
                                     lookup<providers>,
                                     nullptr,
                                     foreach_attribute<providers>,