Fields values are mutable after validation in constructor

[adriendelsalle]

Checks

• I added a descriptive title to this issue
• I have searched (google, github) for similar issues and couldn't find anything
• I have read and followed the docs and still think this is a bug

Bug

Output of python -c "import pydantic.utils; print(pydantic.utils.version_info())":

pydantic version: 1.9.0
pydantic compiled: False
install path: /home/adrien/codes/pydantic/pydantic/pydantic
python version: 3.10.5 | packaged by conda-forge | (main, Jun 14 2022, 07:06:46) [GCC 10.3.0]
platform: Linux-5.15.0-40-generic-x86_64-with-glibc2.35
optional deps. installed: ['typing-extensions']

Hi!
The fields values are mutable after validation in constructor and it may invalidate previsouly validated field:

from pydantic import BaseModel, ValidationError, validator
import pytest

class A(BaseModel):
    a: int
    b: int

    @validator("a")
    def v_a(cls, value, values):
        assert value < 10
        return value

    @validator("b")
    def v_b(cls, value, values):
        values["a"] = 50
        return value

with pytest.raises(ValidationError):
    A(a=50, b=1)

a = A(a=1, b=1)
assert a.a == 50  # that's wrong, should be 1
assert a.b == 1

I suggest to use a copy or similar approach on values in validated_model

https://github.com/samuelcolvin/pydantic/blob/eadfdbdbde3d4d8363b69196bbe4dded956982c3/pydantic/main.py#L1038

[samuelcolvin]

In the end, in every programming environment from excel upwards, if you try hard enough to break things, you can break things.

I would consider this to come into the "try hard enough and you can break things" category.

Either way, I don't think we should make fundamental changes like this in pydantic v1. If we make a change to this, it should be in pydantic-core. Partly because of performance and partly because some people might be using this "feature".

[adriendelsalle]

Hi @samuelcolvin , thanks for taking time to answer!

That's pretty annoying for a library targeting validation as a major feature to have such corner cases.

Do you have some microbenchmarks somewhere to run and see what are the real impacts on perfs?
Another approach could be to use a class wrapper to access the underlying object (dict for GetterDict) without exposing the __setitem__ to avoid such use. That way there is no copy construction but only taking a ref and an additional indirection, pretty cheap.

I'm not sure this kind of use should be consider as a feature. This much more looks like a rabbit hole for people having to debug such a case ;)

[adriendelsalle]

I can't make it as efficient as a dict, constness would help here :).
I opened an issue on pydantic-core as a reminder/todo if it makes sense.