[FIX] core: prevent modification of field attributes

Certain attributes of fields must only be used within the ORM
and should not be used outside it (which could lead to unexpected behaviour).

task-4551158

Author: thle-odoo

odoo/addons/base/models/ir_model.py

@@ -496,7 +496,7 @@ def _check_manual_name(self, name):
 
 
 # retrieve field types defined by the framework only (not extensions)
-FIELD_TYPES = [(key, key) for key in sorted(fields.Field.by_type)]
+FIELD_TYPES = [(key, key) for key in sorted(fields.Field._by_type__)]
 
 
 class IrModelFields(models.Model):
@@ -2413,7 +2413,7 @@ def _module_data_uninstall(self, modules_to_remove):
                     else:
                         # the field is shared across registries; don't modify it
                         Field = type(field)
-                        field_ = Field(_base_fields=[field, Field(prefetch=False)])
+                        field_ = Field(_base_fields__=(field, Field(prefetch=False)))
                         add_field(self.env[ir_field.model], ir_field.name, field_)
                         field_.setup(model)
                         has_shared_field = True

odoo/orm/fields.py

@@ -16,7 +16,7 @@
 from odoo.exceptions import AccessError, MissingError
 from odoo.tools import Query, SQL, sql
 from odoo.tools.constants import PREFETCH_MAX
-from odoo.tools.misc import SENTINEL, OrderedSet, Sentinel, unique
+from odoo.tools.misc import SENTINEL, OrderedSet, ReadonlyDict, Sentinel, unique
 
 from .domains import NEGATIVE_CONDITION_OPERATORS, Domain
 from .utils import COLLECTION_TYPES, SQL_OPERATORS, SUPERUSER_ID, expand_ids
@@ -253,13 +253,13 @@ def _read_group_many2one_field(self, records, domain):
     # Company-dependent fields are stored as jsonb (see column_type).
     _column_type: tuple[str, str] | None = None
 
-    args: dict[str, typing.Any] | None = None  # the parameters given to __init__()
+    _args__: dict[str, typing.Any] | None = None  # the parameters given to __init__()
     _module: str | None = None          # the field's module name
     _modules: tuple[str, ...] = ()      # modules that define this field
     _setup_done = True                  # whether the field is completely set up
     _sequence: int                      # absolute ordering of the field
-    _base_fields: tuple[Self, ...] = ()  # the fields defining self, in override order
-    _extra_keys: tuple[str, ...] = ()   # unknown attributes set on the field
+    _base_fields__: tuple[Self, ...] = ()  # the fields defining self, in override order
+    _extra_keys__: tuple[str, ...] = ()  # unknown attributes set on the field
     _direct: bool = False               # whether self may be used directly (shared)
     _toplevel: bool = False             # whether self is on the model's registry class
 
@@ -304,12 +304,12 @@ def _read_group_many2one_field(self, records, domain):
     exportable: bool = True
 
     # mapping from type name to field type
-    by_type: dict[str, Field] = {}
+    _by_type__: dict[str, Field] = {}
 
     def __init__(self, string: str | Sentinel = SENTINEL, **kwargs):
         kwargs['string'] = string
         self._sequence = next(_global_seq)
-        self.args = {key: val for key, val in kwargs.items() if val is not SENTINEL}
+        self._args__ = ReadonlyDict({key: val for key, val in kwargs.items() if val is not SENTINEL})
 
     def __str__(self):
         if not self.name:
@@ -327,7 +327,7 @@ def __init_subclass__(cls):
             return
 
         if cls.type:
-            cls.by_type.setdefault(cls.type, cls)
+            cls._by_type__.setdefault(cls.type, cls)
 
         # compute class attributes to avoid calling dir() on fields
         cls.related_attrs = []
@@ -337,6 +337,8 @@ def __init_subclass__(cls):
                 cls.related_attrs.append((attr[9:], attr))
             elif attr.startswith('_description_'):
                 cls.description_attrs.append((attr[13:], attr))
+        cls.related_attrs = tuple(cls.related_attrs)
+        cls.description_attrs = tuple(cls.description_attrs)
 
     ############################################################################
     #
@@ -345,33 +347,33 @@ def __init_subclass__(cls):
     # The base field setup is done by field.__set_name__(), which determines the
     # field's name, model name, module and its parameters.
     #
-    # The dictionary field.args gives the parameters passed to the field's
+    # The dictionary field._args__ gives the parameters passed to the field's
     # constructor.  Most parameters have an attribute of the same name on the
     # field.  The parameters as attributes are assigned by the field setup.
     #
     # When several definition classes of the same model redefine a given field,
     # the field occurrences are "merged" into one new field instantiated at
     # runtime on the registry class of the model.  The occurrences of the field
-    # are given to the new field as the parameter '_base_fields'; it is a list
+    # are given to the new field as the parameter '_base_fields__'; it is a list
     # of fields in override order (or reverse MRO).
     #
-    # In order to save memory, a field should avoid having field.args and/or
+    # In order to save memory, a field should avoid having field._args__ and/or
     # many attributes when possible.  We call "direct" a field that can be set
     # up directly from its definition class.  Direct fields are non-related
     # fields defined on models, and can be shared across registries.  We call
     # "toplevel" a field that is put on the model's registry class, and is
     # therefore specific to the registry.
     #
     # Toplevel field are set up once, and are no longer set up from scratch
-    # after that.  Those fields can save memory by discarding field.args and
-    # field._base_fields once set up, because those are no longer necessary.
+    # after that.  Those fields can save memory by discarding field._args__ and
+    # field._base_fields__ once set up, because those are no longer necessary.
     #
     # Non-toplevel non-direct fields are the fields on definition classes that
     # may not be shared.  In other words, those fields are never used directly,
     # and are always recreated as toplevel fields.  On those fields, the base
-    # setup is useless, because only field.args is used for setting up other
+    # setup is useless, because only field._args__ is used for setting up other
     # fields.  We therefore skip the base setup for those fields.  The only
-    # attributes of those fields are: '_sequence', 'args', 'model_name', 'name'
+    # attributes of those fields are: '_sequence', '_args__', 'model_name', 'name'
     # and '_module', which makes their __dict__'s size minimal.
 
     def __set_name__(self, owner: type[BaseModel], name: str) -> None:
@@ -391,14 +393,14 @@ def __set_name__(self, owner: type[BaseModel], name: str) -> None:
             self._module = owner._module
             owner._field_definitions.append(self)
 
-        if not self.args.get('related'):
+        if not self._args__.get('related'):
             self._direct = True
         if self._direct or self._toplevel:
             self._setup_attrs__(owner, name)
             if self._toplevel:
-                # free memory, self.args and self._base_fields are no longer useful
-                self.__dict__.pop('args', None)
-                self.__dict__.pop('_base_fields', None)
+                # free memory, self._args__ and self._base_fields__ are no longer useful
+                self.__dict__.pop('_args__', None)
+                self.__dict__.pop('_base_fields__', None)
 
     #
     # Setup field parameter attributes
@@ -409,21 +411,20 @@ def _get_attrs(self, model_class: type[BaseModel], name: str) -> dict[str, typin
         # determine all inherited field attributes
         attrs = {}
         modules: list[str] = []
-        for field in self.args.get('_base_fields', ()):
+        for field in self._args__.get('_base_fields__', ()):
             if not isinstance(self, type(field)):
                 # 'self' overrides 'field' and their types are not compatible;
                 # so we ignore all the parameters collected so far
                 attrs.clear()
                 modules.clear()
                 continue
-            attrs.update(field.args)
+            attrs.update(field._args__)
             if field._module:
                 modules.append(field._module)
-        attrs.update(self.args)
+        attrs.update(self._args__)
         if self._module:
             modules.append(self._module)
 
-        attrs['args'] = self.args
         attrs['model_name'] = model_class._name
         attrs['name'] = name
         attrs['_module'] = modules[-1] if modules else None
@@ -486,9 +487,9 @@ def _setup_attrs__(self, model_class: type[BaseModel], name: str) -> None:
         attrs = self._get_attrs(model_class, name)
 
         # determine parameters that must be validated
-        extra_keys = [key for key in attrs if not hasattr(self, key)]
+        extra_keys = tuple(key for key in attrs if not hasattr(self, key))
         if extra_keys:
-            attrs['_extra_keys'] = extra_keys
+            attrs['_extra_keys__'] = extra_keys
 
         self.__dict__.update(attrs)
 
@@ -520,7 +521,7 @@ def setup(self, model: BaseModel) -> None:
         """ Perform the complete setup of a field. """
         if not self._setup_done:
             # validate field params
-            for key in self._extra_keys:
+            for key in self._extra_keys__:
                 if not model._valid_field_parameter(self, key):
                     _logger.warning(
                         "Field %s: unknown parameter %r, if this is an actual"
@@ -635,7 +636,7 @@ def setup_related(self, model: BaseModel) -> None:
             if attr not in self.__dict__ and prop.startswith('_related_'):
                 setattr(self, attr, getattr(field, prop))
 
-        for attr in field._extra_keys:
+        for attr in field._extra_keys__:
             if not hasattr(self, attr) and model._valid_field_parameter(self, attr):
                 setattr(self, attr, getattr(field, attr))
 

odoo/orm/fields_selection.py

@@ -3,7 +3,7 @@
 import typing
 from collections import defaultdict
 
-from odoo.tools.misc import SENTINEL, Sentinel, merge_sequences
+from odoo.tools.misc import ReadonlyDict, SENTINEL, Sentinel, merge_sequences
 from odoo.tools.sql import pg_varchar
 
 from .fields import Field, _logger, determine, resolve_mro
@@ -92,19 +92,19 @@ def _get_attrs(self, model_class, name):
 
     def _setup_attrs__(self, model_class, name):
         super()._setup_attrs__(model_class, name)
-        if not self._base_fields:
+        if not self._base_fields__:
             return
 
         # determine selection (applying 'selection_add' extensions) as a dict
         values = None
 
-        for field in self._base_fields:
+        for field in self._base_fields__:
             # We cannot use field.selection or field.selection_add here
             # because those attributes are overridden by ``_setup_attrs__``.
-            if 'selection' in field.args:
+            if 'selection' in field._args__:
                 if self.related:
                     _logger.warning("%s: selection attribute will be ignored as the field is related", self)
-                selection = field.args['selection']
+                selection = field._args__['selection']
                 if isinstance(selection, (list, tuple)):
                     if values is not None and list(values) != [kv[0] for kv in selection]:
                         _logger.warning("%s: selection=%r overrides existing selection; use selection_add instead", self, selection)
@@ -117,17 +117,17 @@ def _setup_attrs__(self, model_class, name):
                 else:
                     raise ValueError(f"{self!r}: selection={selection!r} should be a list, a callable or a method name")
 
-            if 'selection_add' in field.args:
+            if 'selection_add' in field._args__:
                 if self.related:
                     _logger.warning("%s: selection_add attribute will be ignored as the field is related", self)
-                selection_add = field.args['selection_add']
+                selection_add = field._args__['selection_add']
                 assert isinstance(selection_add, list), \
                     "%s: selection_add=%r must be a list" % (self, selection_add)
                 assert values is not None, \
                     "%s: selection_add=%r on non-list selection %r" % (self, selection_add, self.selection)
 
                 values_add = {kv[0]: (kv[1] if len(kv) > 1 else None) for kv in selection_add}
-                ondelete = field.args.get('ondelete') or {}
+                ondelete = field._args__.get('ondelete') or {}
                 new_values = [key for key in values_add if key not in values]
                 for key in new_values:
                     ondelete.setdefault(key, 'set null')
@@ -185,13 +185,13 @@ def _selection_modules(self, model):
             module = field._module
             if not module:
                 continue
-            if 'selection' in field.args:
+            if 'selection' in field._args__:
                 value_modules.clear()
-                if isinstance(field.args['selection'], list):
-                    for value, _label in field.args['selection']:
+                if isinstance(field._args__['selection'], list):
+                    for value, _label in field._args__['selection']:
                         value_modules[value].add(module)
-            if 'selection_add' in field.args:
-                for value_label in field.args['selection_add']:
+            if 'selection_add' in field._args__:
+                for value_label in field._args__['selection_add']:
                     if len(value_label) > 1:
                         value_modules[value_label[0]].add(module)
         return value_modules

odoo/orm/model_classes.py

@@ -18,6 +18,7 @@
     frozendict,
     sql,
 )
+from odoo.tools.misc import ReadonlyDict
 
 if typing.TYPE_CHECKING:
     from odoo.api import Environment
@@ -182,7 +183,7 @@ def add_to_registry(registry: Registry, model_def: type[BaseModel]) -> type[Base
             '_inherit_module': {},                  # map parent to introducing module
             '_inherit_children': OrderedSet(),      # names of children models
             '_inherits_children': set(),            # names of children models
-            '_fields': {},                          # populated in _setup()
+            '_fields': ReadonlyDict({}),            # populated in _setup()
             '_table_objects': frozendict(),         # populated in _setup()
         })
 
@@ -359,7 +360,7 @@ def _setup(model: BaseModel):
     # avoid clashes with inheritance between different models
     for name in model_cls._fields:
         discardattr(model_cls, name)
-    model_cls._fields.clear()
+    model_cls._fields = ReadonlyDict({})
 
     # collect the definitions of each field (base definition + overrides)
     definitions = defaultdict(list)
@@ -375,17 +376,17 @@ def _setup(model: BaseModel):
             # field is translated to avoid converting its column to varchar
             # and losing data
             translate = next((
-                field.args['translate'] for field in reversed(fields_) if 'translate' in field.args
+                field._args__['translate'] for field in reversed(fields_) if 'translate' in field._args__
             ), False)
             if not translate:
                 # patch the field definition by adding an override
                 _logger.debug("Patching %s.%s with translate=True", model_cls._name, name)
                 fields_.append(type(fields_[0])(translate=True))
         if len(fields_) == 1 and fields_[0]._direct and fields_[0].model_name == model_cls._name:
-            model_cls._fields[name] = fields_[0]
+            model_cls._fields = ReadonlyDict({**model_cls._fields, name: fields_[0]})
         else:
             Field = type(fields_[-1])
-            add_field(model, name, Field(_base_fields=fields_))
+            add_field(model, name, Field(_base_fields__=tuple(fields_)))
 
     # 2. add manual fields
     if model.pool._init_modules:
@@ -553,7 +554,7 @@ def _add_manual_fields(model: BaseModel):
             try:
                 attrs = IrModelFields._instanciate_attrs(field_data)
                 if attrs:
-                    field = fields.Field.by_type[field_data['ttype']](**attrs)
+                    field = fields.Field._by_type__[field_data['ttype']](**attrs)
                     add_field(model, name, field)
             except Exception:
                 _logger.exception("Failed to load field %s.%s: skipped", model._name, field_data['name'])
@@ -584,13 +585,15 @@ def add_field(model: BaseModel, name: str, field: Field):
     field._toplevel = True
     field.__set_name__(model_cls, name)
     # add field as an attribute and in model_cls._fields (for reflection)
-    model_cls._fields[name] = field
+    model_cls._fields = ReadonlyDict({**model_cls._fields, name: field})
 
 
 def pop_field(model: BaseModel, name: str) -> Field | None:
     """ Remove the field with the given ``name`` from the model class of ``model``. """
     model_cls = model.env.registry[model._name]
-    field = model_cls._fields.pop(name, None)
+    fields_dict = dict(model_cls._fields)
+    field = fields_dict.pop(name, None)
+    model_cls._fields = ReadonlyDict(fields_dict)
     discardattr(model_cls, name)
     if model_cls._rec_name == name:
         # fixup _rec_name and display_name's dependencies