Make bool(AnonymousUser) evaluate to false

82 views
Skip to first unread message

li...@lew21.net

unread,
May 31, 2017, 12:44:51 PM5/31/17
to Django developers (Contributions to Django itself)
I suggest adding __bool__() method returning False to the AnonymousUser class.

This way it'll be possible to check if the user is authenticated by simply writing "if request.user:"

It's a frequent source of bugs (at least for me, but probably I'm not alone) that right now this code returns True for anonymous users. If unnoticed, such bugs can also lead to security issues.

Adam Johnson

unread,
May 31, 2017, 12:55:06 PM5/31/17
to django-d...@googlegroups.com
I'm afraid I'm -1 on this. We already have if request.user.is_authenticated and request.user.is_anonymous which are both more explicit and pythonic. Additionally all python classes, and thus instances of User atm, are by default truthy, so implementing this custom __bool__ introduces space for more subtle bugs, e.g. the result of filter([request.user]) would change.

--
You received this message because you are subscribed to the Google Groups "Django developers (Contributions to Django itself)" group.
To unsubscribe from this group and stop receiving emails from it, send an email to django-developers+unsubscribe@googlegroups.com.
To post to this group, send email to django-developers@googlegroups.com.
Visit this group at https://groups.google.com/group/django-developers.
To view this discussion on the web visit https://groups.google.com/d/msgid/django-developers/065eb9a5-d935-466d-a301-d7de87e6fbb0%40googlegroups.com.
For more options, visit https://groups.google.com/d/optout.



--
Adam

Tim Graham

unread,
May 31, 2017, 12:56:37 PM5/31/17
to Django developers (Contributions to Django itself)
My thoughts from the ticket, "The Django test suite passes with the change but I feel like that could have some backwards compatibility concerns. Also "explicit is better than implicit"?

Tobias McNulty

unread,
May 31, 2017, 1:28:56 PM5/31/17
to django-developers
I second the objections; my assumption when reading the line 'if request.user:' is that it's shorthand for 'if request.user is None', which is not the case.

Grepping a project's code for incorrect usage of 'request.user' is simple enough, so hopefully that will suffice. I don't recommend this because I think existing Django developers would find it confusing, but technically it would be possible to change or swap out instances of AnonymousUser via middleware, too.

Lastly, this related ticket might be of interest: https://code.djangoproject.com/ticket/20313

Tobias

Tobias McNulty
Chief Executive Officer

tob...@caktusgroup.com
www.caktusgroup.com


--
You received this message because you are subscribed to the Google Groups "Django developers (Contributions to Django itself)" group.
To unsubscribe from this group and stop receiving emails from it, send an email to django-developers+unsubscribe@googlegroups.com.
To post to this group, send email to django-developers@googlegroups.com.
Visit this group at https://groups.google.com/group/django-developers.
To view this discussion on the web visit https://groups.google.com/d/msgid/django-developers/0c20c042-cc34-4707-a3e0-eed3ce2cf83d%40googlegroups.com.

Brice PARENT

unread,
Jun 1, 2017, 3:34:25 AM6/1/17
to django-d...@googlegroups.com

Wouldn't that be better placed in a linter library (such as django_linter)? The syntax is right, it really does what it describes, but it may be wrongly understood, so the syntax could be pointed out as possible cause of bug, but the behaviour we have now seems totally logic and the change would probably have side effects on many projects.

I'm not using specific linters other than the one included in PyCharm, but it would probably be a good idea, when you write "if request.user:" to have it propose to rewrite it as "if request.user.is_authenticated:" if it's what we meant.

-- Brice


Le 31/05/17 à 18:44, li...@lew21.net a écrit :
--
You received this message because you are subscribed to the Google Groups "Django developers (Contributions to Django itself)" group.
To unsubscribe from this group and stop receiving emails from it, send an email to django-develop...@googlegroups.com.
To post to this group, send email to django-d...@googlegroups.com.

Linus Lewandowski

unread,
Jun 1, 2017, 5:22:50 AM6/1/17
to django-d...@googlegroups.com
In my mental model, the request either was sent by a user (that has a User object) or not (and in this case it should be None, or at least something that works like None). However, looks like the majority of you have a different model, so I'm not going to press for that.

On the other hand, maybe it's a good idea to report a warning in the __bool__ method? In most cases, bool(AnonymousUser) is a programming error, and not a valid code, so this might be helpful.

We could make it a warning forever, or use RemovedInDjango30Warning, and then switch to return False OR an exception in 3.0.

Linus


You received this message because you are subscribed to a topic in the Google Groups "Django developers (Contributions to Django itself)" group.
To unsubscribe from this topic, visit https://groups.google.com/d/topic/django-developers/rGWdttZO06g/unsubscribe.
To unsubscribe from this group and all its topics, send an email to django-develop...@googlegroups.com.

To post to this group, send email to django-d...@googlegroups.com.
Visit this group at https://groups.google.com/group/django-developers.

Marten Kenbeek

unread,
Jun 1, 2017, 5:39:34 AM6/1/17
to Django developers (Contributions to Django itself)
{% if request.user %} is a perfectly valid way to check if the user object is available in the template. I don't think we should change that or give a warning.

Adam Johnson

unread,
Jun 1, 2017, 5:40:52 AM6/1/17
to django-d...@googlegroups.com
On the other hand, maybe it's a good idea to report a warning in the __bool__ method? In most cases, bool(AnonymousUser) is a programming error, and not a valid code, so this might be helpful.

bool(AnonymousUser) is very pythonic, not a "programming error". The rules for python's truthy/falsey are mostly "empty string, 0 (int/float/fraction...), False and None are falsey, the rest are truthy". Making AnonymousUser anything other than truthy just seems wrong to me.


On 1 June 2017 at 10:22, Linus Lewandowski <li...@lew21.net> wrote:
In my mental model, the request either was sent by a user (that has a User object) or not (and in this case it should be None, or at least something that works like None). However, looks like the majority of you have a different model, so I'm not going to press for that.

On the other hand, maybe it's a good idea to report a warning in the __bool__ method? In most cases, bool(AnonymousUser) is a programming error, and not a valid code, so this might be helpful.

We could make it a warning forever, or use RemovedInDjango30Warning, and then switch to return False OR an exception in 3.0.

Linus


On Thu, Jun 1, 2017 at 9:34 AM Brice PARENT <con...@brice.xyz> wrote:

Wouldn't that be better placed in a linter library (such as django_linter)? The syntax is right, it really does what it describes, but it may be wrongly understood, so the syntax could be pointed out as possible cause of bug, but the behaviour we have now seems totally logic and the change would probably have side effects on many projects.

I'm not using specific linters other than the one included in PyCharm, but it would probably be a good idea, when you write "if request.user:" to have it propose to rewrite it as "if request.user.is_authenticated:" if it's what we meant.

-- Brice


Le 31/05/17 à 18:44, li...@lew21.net a écrit :
I suggest adding __bool__() method returning False to the AnonymousUser class.

This way it'll be possible to check if the user is authenticated by simply writing "if request.user:"

It's a frequent source of bugs (at least for me, but probably I'm not alone) that right now this code returns True for anonymous users. If unnoticed, such bugs can also lead to security issues.

--
You received this message because you are subscribed to the Google Groups "Django developers (Contributions to Django itself)" group.
To unsubscribe from this group and stop receiving emails from it, send an email to django-developers+unsubscribe@googlegroups.com.

To post to this group, send email to django-developers@googlegroups.com.

--
You received this message because you are subscribed to a topic in the Google Groups "Django developers (Contributions to Django itself)" group.
To unsubscribe from this topic, visit https://groups.google.com/d/topic/django-developers/rGWdttZO06g/unsubscribe.
To unsubscribe from this group and all its topics, send an email to django-developers+unsubscribe@googlegroups.com.
To post to this group, send email to django-developers@googlegroups.com.

--
You received this message because you are subscribed to the Google Groups "Django developers (Contributions to Django itself)" group.
To unsubscribe from this group and stop receiving emails from it, send an email to django-developers+unsubscribe@googlegroups.com.
To post to this group, send email to django-developers@googlegroups.com.

Visit this group at https://groups.google.com/group/django-developers.
To view this discussion on the web visit https://groups.google.com/d/msgid/django-developers/CAB96mebXAH5SLN3p2wDCtvQs4POVD6Xrh-NHOLVM31Q%3DUvX_NQ%40mail.gmail.com.

For more options, visit https://groups.google.com/d/optout.



--
Adam

Brice PARENT

unread,
Jun 1, 2017, 6:11:30 AM6/1/17
to django-d...@googlegroups.com
Le 01/06/17 à 11:40, Adam Johnson a écrit :
bool(AnonymousUser) is very pythonic, not a "programming error". The rules for python's truthy/falsey are mostly "empty string, 0 (int/float/fraction...), False and None are falsey, the rest are truthy". Making AnonymousUser anything other than truthy just seems wrong to me.
Exactly!
`bool(request.user) == False` gives the impression we haven't yet discovered if there is a user or not, like before Django assigns an AnonymousUser or some kind of User instance to it.
`bool(request.user) == True` is explicitely telling us that we know the information is there.
Reply all
Reply to author
Forward
0 new messages