Opened 3 years ago

Closed 3 years ago

Last modified 3 years ago

#18379 closed Bug (fixed)

sensitive_variables handling fails for methods

Reported by: gabrielhurley Owned by: julien
Component: Core (Other) Version: 1.4
Severity: Release blocker Keywords:
Cc: Triage Stage: Accepted
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no

Description

When attempting to decorate a class method with the "sensitive_variables" decorator (both with and without the "method_decorator" decorator) the expectation is that the variables will be properly filtered out.

However, that is not the case due to this line: https://github.com/django/django/blob/master/django/views/debug.py#L159

Since methods do not live in the global namespace, they're not found, and thereby the sensitive_variable stripping is bypassed.

This is non-obvious behavior and should either be documented or fixed to behave as expected.

Change History (7)

comment:1 Changed 3 years ago by gabrielhurley

  • Owner changed from nobody to gabrielhurley

It looks like checking to see if func is None, and then inspecting f_locals for "self" is a viable option to get at the decorated method, but I'm not 100% sure it's the best approach. Frame objects are painful.

That solution would look something like this: https://github.com/gabrielhurley/django/compare/sensitive_variables_for_methods

comment:2 Changed 3 years ago by gabrielhurley

  • Has patch set
  • Needs tests set

comment:3 Changed 3 years ago by julien

  • Owner changed from gabrielhurley to julien
  • Severity changed from Normal to Release blocker
  • Triage Stage changed from Unreviewed to Accepted

I can confirm this issue, thanks! I'm working on a patch now.

comment:4 Changed 3 years ago by julien

  • Needs tests unset

So I've got a patch here: https://github.com/django/django/pull/106

If we're good to merge it then we should also apply it to 1.4.1, as this is a bug in a security-related feature.

Any reviews welcome.

comment:5 Changed 3 years ago by gabrielhurley

Looks good to me, Julien.

I like that approach. Nice and thorough. +1 on applying it to 1.4.1.

Thanks for carrying this one through to the end.

comment:6 Changed 3 years ago by julien

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

Great, thanks for the review Gabriel!
This was merged in [f699641161a4ec8b6cbee938fd3a4379e7889ff2].

comment:7 Changed 3 years ago by Julien Phalip <jphalip@…>

In [4d2fdd41855e978ec5f3b8a03e8599994d7f5b70]:

[1.4.X] Fixed #18379 -- Made the sensitive_variables decorator work with object methods.

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