Skip to content

[FIX] core: default_get can stole records - #52462

Closed
fmdl wants to merge 1 commit into
odoo:masterfrom
fmdl:patch-211
Closed

[FIX] core: default_get can stole records#52462
fmdl wants to merge 1 commit into
odoo:masterfrom
fmdl:patch-211

Conversation

@fmdl

@fmdl fmdl commented Jun 4, 2020

Copy link
Copy Markdown
Contributor

Description of the issue/feature this PR addresses:
Create those models

class Order(models.Model)
      _name = 'order'

     line_ids = fields.One2many(comodel='order.line', inverse='order_id')

class OrderLine(models.Model)
      _name = 'order.line'

     order_id = fields.Many2one(comodel='order')
  • create order(1), with line_ids = order.line(1).
  • execute self.env['order'].with_context(default_line_ids=[1]).create({}).

Result:
order(1), with line_ids = None.
order(2), with line_ids = order.line(1).

--> order(2) has stolen order.line(1)

@odony @rco-odoo

--
I confirm I have signed the CLA and read the PR guidelines at www.odoo.com/submit-pr

@C3POdoo C3POdoo added the ORM ORM, python Framework related label Jun 4, 2020
@odony

odony commented Jun 5, 2020

Copy link
Copy Markdown
Contributor

No, to me that's putting too much arbitrary rules in the framework, there may very well be cases where it's the expected result. And that's without even considering the extra useless SQL queries.

To me, not every possible user or developer mistake needs to be covered or blocked in the code. This way lies madness and bloat. Errors that are caused by unreasonable behavior or code, or are reasonably well handled by default are fine. Especially if the correction implies adding more code and queries that are useless for all other situations.
As the saying goes: garbage in, garbage out.

In other words, proposing a fix for the special case at hand in #52457 is ok. This particular case for expense sheets is relatively likely to happen, not reasonably well handled currently, and easy to fix by changing the order of operations rather than adding defensive code. But let's leave the framework alone ;-)

@fmdl

fmdl commented Jun 7, 2020

Copy link
Copy Markdown
Contributor Author

@odony the issue is more global :

@odony

odony commented Jun 8, 2020

Copy link
Copy Markdown
Contributor

@odony the issue is more global

The issues you link to are caused by some user or developer (with legitimate access to the data) writing RPC or custom code that clearly makes no business sense. That would be in the category of "unreasonable behavior or code", i.e. garbage in = garbage out.

In your examples you could replace record.write(..) with cr.execute(...) if you are in an automated action for example. Are we going to prevent all mistakes there? Why is it different?

Unless the issues represent an actual security risk (in which case we wouldn't be discussing them here ;-)), or are reproducible easily through a reasonable usage of the our features, we can't cover all mistakes. We need to focus on the important things.

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

Labels

ORM ORM, python Framework related

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants