Opened 9 years ago

Closed 9 years ago

Last modified 8 years ago

#1488 closed defect (fixed)

[magic-removal][patch] request.user proxy object gives read-only attributes

Reported by: django@… Owned by: adrian
Component: Core (Other) Version:
Severity: normal Keywords:
Cc: Triage Stage: Unreviewed
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: UI/UX:

Description

The lazy loading request.user proxy object gives read-only access to user attributes. Attached patch allows setting attributes on request.user

Attachments (3)

auth_middleware_fix.diff (1.6 KB) - added by django@… 9 years ago.
auth middleware fix permits setting attributes on request.use
auth_middleware_fix.2.diff (1.8 KB) - added by django@… 9 years ago.
auth middleware fix permits setting attributes on request.use
auth_middleware_fix.3.diff (1.8 KB) - added by django@… 9 years ago.
_actual_ patch, ignore first two files

Download all attachments as: .zip

Change History (10)

Changed 9 years ago by django@…

auth middleware fix permits setting attributes on request.use

Changed 9 years ago by django@…

auth middleware fix permits setting attributes on request.use

Changed 9 years ago by django@…

_actual_ patch, ignore first two files

comment:1 Changed 9 years ago by jkocherhans

Reassigning request.__class__ attribute feels really wrong to me. However, this works, and the current implementation doesn't... at least not like it should. I don't have any ideas other than using properties on the request object like it was before. Is de-coupling worth the __class__ hack in this case? Is there another way to do this?

comment:2 Changed 9 years ago by adrian

Both the current magic-removal behavior *and* this patch are problematic in that they access request.session for every request. That should be lazy in itself. I'm working on it...

comment:3 Changed 9 years ago by adrian

  • Resolution set to fixed
  • Status changed from new to closed

(In [2590]) magic-removal: Fixed #1488 -- request.user is no longer read-only. Also improved it to only hit the session database table if a user is requested.

comment:4 Changed 9 years ago by adrian

Note that my solution accomplished it by doing this:

request.__class__.user = LazyUser(request.session)

That's hackish as well, because it changes the class instead of the instance. I couldn't get per-instance descriptors to work. Maybe this discussion can help us come up with a cleaner solution.

comment:5 Changed 9 years ago by adrian

I cleaned it up a bit in [2591].

comment:6 Changed 9 years ago by django@…

Hi Adrian,

Good point about session access needing to be lazy too - should have thought of that.

Note that in my patch the descriptor binds the user to the instance - this way the descriptor is only called the first time the user attribute is accessed because instance.__dict__ has priority over "non-data" descriptors.

The last version of my patch does actually do per-instance descriptors by dynamically creating a new subclass for each instance (this was an Alex Martelli trick I found somewhere in c.l.p.) but I have just timed it and unfortunately class creation is slow (it takes about 30 times as long).

Also note that there is a stray import of SESSION_KEY still in there.

comment:7 Changed 9 years ago by adrian

Thanks for the info -- and for the heads-up about the stray import. Fixed in [2592].

Note: See TracTickets for help on using tickets.
Back to Top