Skip to content

Model.resolved mutates shared discriminator field across sibling models via dict.update reference copy #646

Open
gravithex wants to merge 1 commit intopython-restx:masterfrom
gravithex:fix/discriminator-default-shared-mutation-in-resolved
Open

Model.resolved mutates shared discriminator field across sibling models via dict.update reference copy #646
gravithex wants to merge 1 commit intopython-restx:masterfrom
gravithex:fix/discriminator-default-shared-mutation-in-resolved

Conversation

@gravithex
Copy link

@gravithex gravithex commented Mar 20, 2026

Describe the bug

When two or more models inherit from a common parent that defines a discriminator field, all sibling models end up sharing the same String field instance for the discriminator. Each model mutates that shared instance when its .resolved is computed, so whichever sibling is resolved last overwrites the default for all others.

This is distinct from (but related to) #314, which describes a stale value on the second call. This issue causes the discriminator to be wrong on subsequent calls, once both sibling models have had their .resolved computed

To reproduce

from dataclasses import dataclass

from flask import Flask
from flask_restx import Api, Namespace, fields, marshal

app = Flask(__name__)
api = Api(app)
ns = Namespace("test", path="/test")
api.add_namespace(ns)

model_signatory = ns.model("Signatory", {"class": fields.String(discriminator=True)})

model_private = ns.inherit("PrivatePerson", model_signatory, {
    "first_name": fields.String(),
})
model_legal = ns.inherit("LegalEntity", model_signatory, {
    "legal_name": fields.String(),
})


@dataclass
class PrivatePerson:
    first_name: str


@dataclass
class LegalEntity:
    legal_name: str


@dataclass
class Container:
    signatory: object


mapping = {PrivatePerson: model_private, LegalEntity: model_legal}
container_model = ns.model("Container", {"signatory": fields.Polymorph(mapping)})

with app.app_context():
    # First pass — both look correct because each model's .resolved is computed for the first time
    r1 = marshal(Container(LegalEntity("Acme")), container_model)
    r2 = marshal(Container(PrivatePerson("John")), container_model)
    print("Pass 1:")
    print("  LegalEntity  →", r1["signatory"])   # {'class': 'LegalEntity',  ...} ✓
    print("  PrivatePerson→", r2["signatory"])   # {'class': 'PrivatePerson', ...} ✓

    # Second pass — .resolved is cached, but the shared discriminator String instance
    # was last mutated by PrivatePerson, so LegalEntity now outputs the wrong class
    r3 = marshal(Container(LegalEntity("Acme")), container_model)
    r4 = marshal(Container(PrivatePerson("John")), container_model)
    print("\nPass 2:")
    print("  LegalEntity  →", r3["signatory"])   # {'class': 'PrivatePerson', ...} ← BUG
    print("  PrivatePerson→", r4["signatory"])   # {'class': 'PrivatePerson', ...} ✓

Expected behavior

class should be "LegalEntity" when marshalling a LegalEntity instance.

Root cause

The bug is in RawModel.resolved (model.py):

@cached_property                                                                                                                                                                                                               
def resolved(self):                                       
    resolved = copy.deepcopy(self)                                                                                                                                                                                             
    for parent in self.__parents__:
        resolved.update(parent.resolved)       # ← copies field references, NOT copies                                                                                                                                         
                                                                                                                                                                                                                                 
    candidates = [f for f in resolved.values() if getattr(f, "discriminator", None)]                                                                                                                                           
    elif len(candidates) == 1:                                                                                                                                                                                                 
        candidates[0].default = self.name      # ← mutates the SHARED String instance                                                                                                                                          

dict.update copies values by reference. Since parent.resolved is a @cached_property (always the same object), every child's resolved dict holds a reference to the same String(discriminator=True) instance from the parent.

Then candidates[0].default = self.name mutates that shared instance. The last sibling to call .resolved sets the default for all of them.

Proposed fix

Deep copy the parent fields before merging, so each child model owns a private copy before mutating it:

- resolved.update(parent.resolved)
+ resolved.update(copy.deepcopy(parent.resolved))

This is more targeted than the approach in PR #359 (replacing @cached_property with @property), which fixes the symptom by disabling caching entirely at the cost of recomputing a deepcopy on every marshal call. The proposed fix here preserves the @cached_property performance benefit while eliminating the shared mutation at its source.

Environment

  • flask-restx version: 1.3.2
  • Python version: 3.11

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants