[Django] #36945: .values() and .order_by() miss FilteredRelation when resolving aliases shadowing relationship traversals

[Django]

#36945: .values() and .order_by() miss FilteredRelation when resolving aliases
shadowing relationship traversals
-------------------------------------+-------------------------------------
Reporter: Jacob | Owner: Sarah Boyce
Walls |
Type: Bug | Status: assigned
Component: Database | Version: 6.0
layer (models, ORM) | Keywords: FilteredRelation,
Severity: Normal | alias
Triage Stage: | Has patch: 0
Unreviewed |
Needs documentation: 0 | Needs tests: 0
Patch needs improvement: 0 | Easy pickings: 0
UI/UX: 0 |
-------------------------------------+-------------------------------------
Here is a [https://dryorm.xterm.info/filtered-relation-values-shadowing
fiddle] on DryORM demonstrating that when `.values()`/`.values_list()`
resolve aliases, they appropriately give priority to most aliases supplied
by `.annotate()`/`.alias()` before trying to parse `__` as relationship
traversals, but fail to account for annotations consisting of
`FilteredRelation`, which are stored in a different internal data
structure and therefore missed.

This leads to resolving an alias that happens to contain `__` as a
relationship traversal instead of the alias that was requested.

Here is the reproducer:
{{{#!py
from django.db import models
from django.contrib.auth.models import User
from django.contrib.contenttypes.models import ContentType


class Person(models.Model):
name = models.CharField(max_length=100)
creator = models.ForeignKey(User, models.CASCADE, null=True)
ct = models.ForeignKey(ContentType, models.CASCADE)


def run():
admin = User.objects.create(username='admin')
person_ct = ContentType.objects.get_for_model(Person)
Person.objects.create(creator=admin, name="Prince", ct=person_ct)
alias = "ct__model"
# This does not resolve to ContentType.model. Returns
Person.creator_id
qs = Person.objects.annotate(**{alias:
models.F("creator")}).values(alias)
print(qs)
# This does resolve to ContentType.model. Returns Person.ct.model
qs = Person.objects.annotate(
**{alias: models.FilteredRelation("creator",
condition=models.Q(creator__isnull=False))}
).values(alias)
print(qs)
}}}
Output:
{{{
<QuerySet [{'ct__model': 1}]>
<QuerySet [{'ct__model': 'person'}]>
}}}

In this example, since `alias` is a variable, and could be user-controlled
in the case of user-configurable reports, it is unexpected that for some
values of `alias`, the column it maps to might vary.
----
`.order_by()` is affected in the same way.
----
While this is not regarded as a security vulnerability, we thank
sw0rdl1ght for bringing the `.order_by()` case to the attention of the
security team.
--
Ticket URL: <https://code.djangoproject.com/ticket/36945>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.

[Django]

#36945: .values() and .order_by() miss FilteredRelation when resolving aliases
shadowing relationship traversals
-------------------------------------+-------------------------------------

Reporter: Jacob Walls | Owner: Sarah
| Boyce
Type: Bug | Status: assigned
Component: Database layer | Version: 6.0
(models, ORM) |
Severity: Normal | Resolution:
Keywords: FilteredRelation, | Triage Stage:
alias | Unreviewed
Has patch: 1 | Needs documentation: 0

Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------

Changes (by Dhruvan Gnanadhandayuthapani):

* has_patch: 0 => 1

Comment:

PR -> https://github.com/django/django/pull/20802
--
Ticket URL: <https://code.djangoproject.com/ticket/36945#comment:1>

[Django]

#36945: .values() and .order_by() miss FilteredRelation when resolving aliases
shadowing relationship traversals
-------------------------------------+-------------------------------------
Reporter: Jacob Walls | Owner: Sarah
| Boyce
Type: Bug | Status: assigned
Component: Database layer | Version: 6.0
(models, ORM) |
Severity: Normal | Resolution:

Keywords: FilteredRelation, | Triage Stage: Accepted
alias, order_by, values |

Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------

Changes (by Natalia Bidart):

* keywords: FilteredRelation, alias => FilteredRelation, alias, order_by,
values
* stage: Unreviewed => Accepted

Comment:

Thank you!
--
Ticket URL: <https://code.djangoproject.com/ticket/36945#comment:2>

[Django]

#36945: .values() and .order_by() miss FilteredRelation when resolving aliases
shadowing relationship traversals
-------------------------------------+-------------------------------------
Reporter: Jacob Walls | Owner: Sarah
| Boyce
Type: Bug | Status: assigned
Component: Database layer | Version: 6.0
(models, ORM) |
Severity: Normal | Resolution:
Keywords: FilteredRelation, | Triage Stage: Accepted
alias, order_by, values |
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------

Comment (by Annabelle Wiegart):

As discussed on Djangonaut Space Discord, I'm happy to work on this
ticket.
--
Ticket URL: <https://code.djangoproject.com/ticket/36945#comment:3>

[Django]

#36945: .values() and .order_by() miss FilteredRelation when resolving aliases
shadowing relationship traversals
-------------------------------------+-------------------------------------

Reporter: Jacob Walls | Owner: Annabelle
| Wiegart

Type: Bug | Status: assigned
Component: Database layer | Version: 6.0
(models, ORM) |
Severity: Normal | Resolution:
Keywords: FilteredRelation, | Triage Stage: Accepted
alias, order_by, values |
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------

Changes (by Annabelle Wiegart):

* owner: Sarah Boyce => Annabelle Wiegart

--
Ticket URL: <https://code.djangoproject.com/ticket/36945#comment:4>

[Django]

#36945: .values() and .order_by() miss FilteredRelation when resolving aliases
shadowing relationship traversals
-------------------------------------+-------------------------------------
Reporter: Jacob Walls | Owner: Annabelle
| Wiegart
Type: Bug | Status: assigned
Component: Database layer | Version: 6.0
(models, ORM) |
Severity: Normal | Resolution:
Keywords: FilteredRelation, | Triage Stage: Accepted
alias, order_by, values |

Has patch: 0 | Needs documentation: 0

Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------

Changes (by Jacob Walls):

* has_patch: 1 => 0

--
Ticket URL: <https://code.djangoproject.com/ticket/36945#comment:5>

[Django]

#36945: .values() and .order_by() miss FilteredRelation when resolving aliases
shadowing relationship traversals
-------------------------------------+-------------------------------------
Reporter: Jacob Walls | Owner: Annabelle
| Wiegart
Type: Bug | Status: assigned
Component: Database layer | Version: 6.0
(models, ORM) |
Severity: Normal | Resolution:
Keywords: FilteredRelation, | Triage Stage: Accepted
alias, order_by, values |
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------

Comment (by Annabelle Wiegart):

It took me some time to understand the reproducer example and identify the
relevant methods. Looking at .values() first, it seems like the culprit is
in django.db.models.sql.query.Query::add_fields: When self.setup_joins()
is called, the alias is split without checking for a filtered relation
with that name first:

{{{
join_info = self.setup_joins(name.split(LOOKUP_SEP), opts, alias,
allow_many=allow_m2m)
}}}

That could be a place to fix the problem. Next, I am going to look into
.values_list() and .order_by(). I'll try to provide a patch shortly.
--
Ticket URL: <https://code.djangoproject.com/ticket/36945#comment:6>

[Django]

#36945: .values() and .order_by() miss FilteredRelation when resolving aliases
shadowing relationship traversals
-------------------------------------+-------------------------------------
Reporter: Jacob Walls | Owner: Annabelle
| Wiegart
Type: Bug | Status: assigned
Component: Database layer | Version: 6.0
(models, ORM) |
Severity: Normal | Resolution:
Keywords: FilteredRelation, | Triage Stage: Accepted
alias, order_by, values |

Has patch: 1 | Needs documentation: 0

Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------

Changes (by Annabelle Wiegart):

* has_patch: 0 => 1

Comment:

I just created which solves the problem.
--
Ticket URL: <https://code.djangoproject.com/ticket/36945#comment:7>

[Django]

#36945: .values() and .order_by() miss FilteredRelation when resolving aliases
shadowing relationship traversals
-------------------------------------+-------------------------------------
Reporter: Jacob Walls | Owner: Annabelle
| Wiegart
Type: Bug | Status: assigned
Component: Database layer | Version: 6.0
(models, ORM) |
Severity: Normal | Resolution:
Keywords: FilteredRelation, | Triage Stage: Accepted
alias, order_by, values |
Has patch: 1 | Needs documentation: 0

Needs tests: 0 | Patch needs improvement: 1

Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------

Changes (by Jacob Walls):

* needs_better_patch: 0 => 1

--
Ticket URL: <https://code.djangoproject.com/ticket/36945#comment:8>

[Django]

#36945: .values() and .order_by() miss FilteredRelation when resolving aliases
shadowing relationship traversals
-------------------------------------+-------------------------------------
Reporter: Jacob Walls | Owner: Annabelle
| Wiegart
Type: Bug | Status: assigned
Component: Database layer | Version: 6.0
(models, ORM) |
Severity: Normal | Resolution:

Keywords: FilteredRelation, | Triage Stage: Ready for
alias, order_by, values | checkin

Has patch: 1 | Needs documentation: 0

Needs tests: 0 | Patch needs improvement: 0

Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Changes (by Jacob Walls):

* needs_better_patch: 1 => 0
* stage: Accepted => Ready for checkin

--
Ticket URL: <https://code.djangoproject.com/ticket/36945#comment:9>

[Django]

#36945: .values() and .order_by() miss FilteredRelation when resolving aliases
shadowing relationship traversals
-------------------------------------+-------------------------------------
Reporter: Jacob Walls | Owner: Annabelle
| Wiegart

Type: Bug | Status: closed

Component: Database layer | Version: 6.0
(models, ORM) |

Severity: Normal | Resolution: fixed

Keywords: FilteredRelation, | Triage Stage: Ready for
alias, order_by, values | checkin
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------

Changes (by Jacob Walls <jacobtylerwalls@…>):

* resolution: => fixed
* status: assigned => closed

Comment:

In [changeset:"73cc09f14f13fedddc14d6ba5b287cb33c24e4a4" 73cc09f]:
{{{#!CommitTicketReference repository=""
revision="73cc09f14f13fedddc14d6ba5b287cb33c24e4a4"
Fixed #36945 -- Made .values() and .order_by() resolve FilteredRelation
aliases correctly.

Made Query.add_fields(), Query.add_ordering() and
SQLCompiler.find_ordering_name() look up FilteredRelation aliases
before splitting field names to relationship traversals. Added
.test_alias_values() and .test_alias_order_by() to
FilteredRelationTests.

Thanks Pravin Kamble for the review.
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/36945#comment:10>