[Django] #23004: Cleanse entries from request.META in debug views

41 views
Skip to first unread message

Django

unread,
Jul 11, 2014, 11:11:33 AM7/11/14
to django-...@googlegroups.com
#23004: Cleanse entries from request.META in debug views
------------------------------+--------------------
Reporter: blueyed | Owner: nobody
Type: New feature | Status: new
Component: Core (Other) | Version: master
Severity: Normal | Keywords:
Triage Stage: Unreviewed | Has patch: 0
Easy pickings: 0 | UI/UX: 0
------------------------------+--------------------
In the debug views `settings` is cleansed, which hides e.g. `SECRET_KEY`.

But a lot of sensible information might also be present / come from
`request.META`, e.g. in the form of `DJANGO_SECRET_KEY` or `DATABASE_URL`.

It might be sensible to apply a filter in `TECHNICAL_500_TEMPLATE` (source
code reference:
https://github.com/django/django/blob/master/django/views/debug.py#L972-977).

I see that this can be quite specific, but I think it would be sensible to
apply `HIDDEN_SETTINGS` to all entries starting with `DJANGO_` and have a
setting for additional entries, which might default to `DATABASE_URL` and
`SENTRY_DSN`.

--
Ticket URL: <https://code.djangoproject.com/ticket/23004>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.

Django

unread,
Jul 11, 2014, 12:27:03 PM7/11/14
to django-...@googlegroups.com
#23004: Cleanse entries from request.META in debug views
------------------------------+--------------------------------------

Reporter: blueyed | Owner: nobody
Type: New feature | Status: new
Component: Core (Other) | Version: master
Severity: Normal | Resolution:
Keywords: | Triage Stage: Unreviewed
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0

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

* needs_better_patch: => 0
* needs_tests: => 0
* needs_docs: => 0


Comment:

I have noticed that the environment variables do not appear to be present
when using Django's test.Client / live_server.

Shouldn't the test client's request.meta also include os.environ?

--
Ticket URL: <https://code.djangoproject.com/ticket/23004#comment:1>

Django

unread,
Jul 11, 2014, 12:56:30 PM7/11/14
to django-...@googlegroups.com
#23004: Cleanse entries from request.META in debug views
------------------------------+------------------------------------

Reporter: blueyed | Owner: nobody
Type: New feature | Status: new
Component: Core (Other) | Version: master
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted

Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0

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

* stage: Unreviewed => Accepted


Comment:

I would do something like move the `cleanse_setting()` function to a
method on `ExceptionReporterFilter` and make things like `HIDDEN_SETTINGS`
attributes. You could then easily override them in a subclass to avoid
introducing more global settings.

Regarding your comment, it's a separate issue but I don't think the test
client should include `os.environ`. You shouldn't rely on environment
variables in your views.

--
Ticket URL: <https://code.djangoproject.com/ticket/23004#comment:2>

Django

unread,
Jul 11, 2014, 3:43:31 PM7/11/14
to django-...@googlegroups.com
#23004: Cleanse entries from request.META in debug views
------------------------------+------------------------------------

Reporter: blueyed | Owner: nobody
Type: New feature | Status: new
Component: Core (Other) | Version: master
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0

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

Comment (by blueyed):

Just answering to the separate issue from the comment about `request.META`
- I do not have time to provide a patch for this issue myself, but thanks
for accepting it and outlining how it could be done!

> Regarding your comment, it's a separate issue but I don't think the test

client should include os.environ. You shouldn't rely on environment
variables in your views.

It's more that I want to test for e.g. `assert settings.SECRET_KEY not in
response.content` (for a "500" page), and was surprised that
`request.META` in the test client is different from runserver/uwsgi. I
have created a new issue for it:
https://code.djangoproject.com/ticket/23006

--
Ticket URL: <https://code.djangoproject.com/ticket/23004#comment:3>

Django

unread,
Aug 3, 2014, 7:35:13 AM8/3/14
to django-...@googlegroups.com
#23004: Cleanse entries from request.META in debug views
------------------------------+------------------------------------

Reporter: blueyed | Owner: nobody
Type: New feature | Status: new
Component: Core (Other) | Version: master
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0

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

Comment (by sthzg):

I am interested in this topic and started experimenting with @timo's
suggestions. I hope it is okay to put some questions here, because I am
not completely sure about the scope of it and would be interested in your
opinion.

---

After reading through the code, the {{{cleanse_setting()}}} method seems
to only be relevant to parsing values from the settings. Cleansing POST
for example (which like META is part of the request instance) is done as
part of {{{SafeExceptionReporterFilter}}}. What I am experimenting with is
to provide similar behavior for request.META as there already is for POST.

I implemented a {{{get_meta_parameters()}}} on
{{{SafeExceptionReporterFilter}}} that cleanses all values in META that
match the {{{HIDDEN_SETTINGS}}} (that are now an attribute of
{{{ExceptionReporterFilter}}}).


{{{
#!python
def get_meta_parameters(self, request):
"""
Replaces the values of META parameters that match defined patterns
from HIDDEN_SETTINGS with stars (*********).
"""
if request is None:
return {}
else:
cleansed = request.META.copy()
# Cleanse all values that match the regexp in HIDDEN_SETTINGS.
for k, v in cleansed.items():
if self.HIDDEN_SETTINGS.search(k):
cleansed[k] = CLEANSED_SUBSTITUTE
return cleansed
}}}

Now my idea would be to extend the Context instance in
{{{get_traceback_data()}}} to get a {{{filtered_META}}}, analog to what it
does to get the {{{filtered_POST}}}

{{{
#!python
c = {
# ...
'filtered_POST': self.filter.get_post_parameters(self.request),
'filtered_META': self.filter.get_meta_parameters(self.request)
# ...
}
}}}

Then, if {{{filtered_META}}} is not empty, I thought about changing the
{{{TECHNICAL_500_TEMPLATE}}} to loop over that.


----


Before I go on I would be interested if this was still accepted in terms
of behavior and scope or if a solution in that direction would be unlikely
to make its way to core. If yes I would be happy trying to code it and
backing it up by tests and then come back here to discuss the possible
patch.

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

Django

unread,
Mar 13, 2015, 9:25:59 PM3/13/15
to django-...@googlegroups.com
#23004: Cleanse entries from request.META in debug views
------------------------------+------------------------------------

Reporter: blueyed | Owner: nobody
Type: New feature | Status: new
Component: Core (Other) | Version: master
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0

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

Comment (by blueyed):

@sthzg
Your proposed changes look good to me.

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

Django

unread,
Jun 13, 2015, 8:30:18 PM6/13/15
to django-...@googlegroups.com
#23004: Cleanse entries from request.META in debug views
------------------------------+------------------------------------

Reporter: blueyed | Owner: nobody
Type: New feature | Status: new
Component: Core (Other) | Version: master
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0

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

* cc: jrabbit (added)


--
Ticket URL: <https://code.djangoproject.com/ticket/23004#comment:6>

Django

unread,
Jul 23, 2015, 1:23:37 PM7/23/15
to django-...@googlegroups.com
#23004: Cleanse entries from request.META in debug views
---------------------------------+------------------------------------

Reporter: blueyed | Owner: nobody
Type: New feature | Status: new
Component: Error reporting | Version: master

Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0

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

* component: Core (Other) => Error reporting


--
Ticket URL: <https://code.djangoproject.com/ticket/23004#comment:7>

Django

unread,
Mar 24, 2016, 5:26:12 PM3/24/16
to django-...@googlegroups.com
#23004: Cleanse entries from request.META in debug views
---------------------------------+------------------------------------

Reporter: blueyed | Owner: nobody
Type: New feature | Status: new
Component: Error reporting | Version: master
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 1
Needs tests: 1 | Patch needs improvement: 0

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

* needs_docs: 0 => 1
* has_patch: 0 => 1
* needs_tests: 0 => 1


Comment:

[https://github.com/django/django/pull/6331 PR] (currently without tests
and docs and probably developed independent from the discussion in this
ticket).

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

Django

unread,
Jan 29, 2017, 10:25:29 PM1/29/17
to django-...@googlegroups.com
#23004: Cleanse entries from request.META in debug views
---------------------------------+----------------------------------------
Reporter: Daniel Hahler | Owner: Ryan Castner
Type: New feature | Status: assigned

Component: Error reporting | Version: master
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 1
Needs tests: 1 | Patch needs improvement: 0

Easy pickings: 0 | UI/UX: 0
---------------------------------+----------------------------------------
Changes (by Ryan Castner):

* owner: nobody => Ryan Castner
* status: new => assigned


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

Django

unread,
Jan 31, 2017, 8:29:31 AM1/31/17
to django-...@googlegroups.com
#23004: Cleanse entries from request.META in debug views
---------------------------------+----------------------------------------
Reporter: Daniel Hahler | Owner: Ryan Castner
Type: New feature | Status: assigned
Component: Error reporting | Version: master
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 1

Easy pickings: 0 | UI/UX: 0
---------------------------------+----------------------------------------
Changes (by Tim Graham):

* needs_better_patch: 0 => 1
* needs_tests: 1 => 0
* needs_docs: 1 => 0


Comment:

[https://github.com/django/django/pull/7996 PR] with comments for
improvement.

--
Ticket URL: <https://code.djangoproject.com/ticket/23004#comment:10>

Django

unread,
Jan 31, 2017, 2:43:36 PM1/31/17
to django-...@googlegroups.com
#23004: Cleanse entries from request.META in debug views
---------------------------------+----------------------------------------
Reporter: Daniel Hahler | Owner: Ryan Castner
Type: New feature | Status: assigned
Component: Error reporting | Version: master
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0

Easy pickings: 0 | UI/UX: 0
---------------------------------+----------------------------------------
Changes (by Ryan Castner):

* needs_better_patch: 1 => 0


Comment:

Ok I made the updates you requested. Only thing I am unsure of is how I
did the `.. versionchanged 2.0` annotation.

--
Ticket URL: <https://code.djangoproject.com/ticket/23004#comment:11>

Django

unread,
Mar 6, 2017, 12:08:17 PM3/6/17
to django-...@googlegroups.com
#23004: Cleanse entries from request.META in debug views
---------------------------------+----------------------------------------
Reporter: Daniel Hahler | Owner: Ryan Castner
Type: New feature | Status: assigned
Component: Error reporting | Version: master
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 1

Easy pickings: 0 | UI/UX: 0
---------------------------------+----------------------------------------
Changes (by Tim Graham):

* needs_better_patch: 0 => 1


--
Ticket URL: <https://code.djangoproject.com/ticket/23004#comment:12>

Django

unread,
Dec 12, 2018, 2:22:13 PM12/12/18
to django-...@googlegroups.com
#23004: Cleanse entries from request.META in debug views
---------------------------------+------------------------------------
Reporter: Daniel Hahler | Owner: maxsond

Type: New feature | Status: assigned
Component: Error reporting | Version: master
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 1
Easy pickings: 0 | UI/UX: 0
---------------------------------+------------------------------------
Changes (by maxsond):

* owner: Ryan Castner => maxsond


--
Ticket URL: <https://code.djangoproject.com/ticket/23004#comment:13>

Django

unread,
Dec 12, 2018, 3:13:16 PM12/12/18
to django-...@googlegroups.com
#23004: Cleanse entries from request.META in debug views
---------------------------------+------------------------------------
Reporter: Daniel Hahler | Owner: maxsond
Type: New feature | Status: assigned
Component: Error reporting | Version: master
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0

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

* needs_better_patch: 1 => 0


Comment:

[https://github.com/django/django/pull/10748|​PR] updated for the current
master branch and with a test for
ExceptionReporterFilter.get_safe_request_meta.

--
Ticket URL: <https://code.djangoproject.com/ticket/23004#comment:14>

Django

unread,
Feb 13, 2019, 10:15:28 AM2/13/19
to django-...@googlegroups.com
#23004: Cleanse entries from request.META in debug views
---------------------------------+-----------------------------------------
Reporter: Daniel Hahler | Owner: Daniel Maxson

Type: New feature | Status: assigned
Component: Error reporting | Version: master
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 1

Easy pickings: 0 | UI/UX: 0
---------------------------------+-----------------------------------------
Changes (by Carlton Gibson):

* needs_better_patch: 0 => 1


--
Ticket URL: <https://code.djangoproject.com/ticket/23004#comment:15>

Django

unread,
Mar 28, 2019, 11:36:00 AM3/28/19
to django-...@googlegroups.com
#23004: Cleanse entries from request.META in debug views
---------------------------------+-----------------------------------------
Reporter: Daniel Hahler | Owner: Daniel Maxson
Type: New feature | Status: assigned
Component: Error reporting | Version: master
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0

Easy pickings: 0 | UI/UX: 0
---------------------------------+-----------------------------------------
Changes (by Carlton Gibson):

* needs_better_patch: 1 => 0


Comment:

There have been updates since last review. Unchecking PNI to put it back
in the queue.

--
Ticket URL: <https://code.djangoproject.com/ticket/23004#comment:16>

Django

unread,
Mar 28, 2019, 5:42:55 PM3/28/19
to django-...@googlegroups.com
#23004: Cleanse entries from request.META in debug views
-------------------------------------+-------------------------------------

Reporter: Daniel Hahler | Owner: Daniel
| Maxson
Type: New feature | Status: assigned
Component: Error reporting | Version: master
Severity: Normal | Resolution:
Keywords: | Triage Stage: Ready for
| checkin

Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0

Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Changes (by Oskar Haller):

* stage: Accepted => Ready for checkin


--
Ticket URL: <https://code.djangoproject.com/ticket/23004#comment:17>

Django

unread,
Apr 13, 2019, 1:47:42 PM4/13/19
to django-...@googlegroups.com
#23004: Cleanse entries from request.META in debug views
-------------------------------------+-------------------------------------
Reporter: Daniel Hahler | Owner: Daniel
| Maxson
Type: New feature | Status: assigned
Component: Error reporting | Version: master
Severity: Normal | Resolution:
Keywords: | Triage Stage: Ready for
| checkin
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0

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

Comment (by Markus Holtermann):

I'm not entirely sure we need that big of a change for this ticket,
especially considering that this is a `DEBUG=True` page that should
_never_ show up in production in the first place. I'd like to propose a
significantly smaller PR instead:
https://github.com/django/django/pull/11224

--
Ticket URL: <https://code.djangoproject.com/ticket/23004#comment:18>

Django

unread,
Apr 13, 2019, 1:48:36 PM4/13/19
to django-...@googlegroups.com
#23004: Cleanse entries from request.META in debug views
---------------------------------+-----------------------------------------

Reporter: Daniel Hahler | Owner: Daniel Maxson
Type: New feature | Status: assigned
Component: Error reporting | Version: master
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted

Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0

Easy pickings: 0 | UI/UX: 0
---------------------------------+-----------------------------------------
Changes (by Markus Holtermann):

* stage: Ready for checkin => Accepted


--
Ticket URL: <https://code.djangoproject.com/ticket/23004#comment:19>

Django

unread,
Apr 13, 2019, 4:01:26 PM4/13/19
to django-...@googlegroups.com
#23004: Cleanse entries from request.META in debug views
---------------------------------+-----------------------------------------
Reporter: Daniel Hahler | Owner: Daniel Maxson
Type: New feature | Status: assigned
Component: Error reporting | Version: master
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0

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

Old description:

> In the debug views `settings` is cleansed, which hides e.g. `SECRET_KEY`.
>
> But a lot of sensible information might also be present / come from
> `request.META`, e.g. in the form of `DJANGO_SECRET_KEY` or
> `DATABASE_URL`.
>
> It might be sensible to apply a filter in `TECHNICAL_500_TEMPLATE`
> (source code reference:
> https://github.com/django/django/blob/master/django/views/debug.py#L972-977).
>
> I see that this can be quite specific, but I think it would be sensible
> to apply `HIDDEN_SETTINGS` to all entries starting with `DJANGO_` and
> have a setting for additional entries, which might default to
> `DATABASE_URL` and `SENTRY_DSN`.

New description:

--

Comment (by Ryan Castner):

> I'm not entirely sure we need that big of a change for this ticket,
especially considering that this is a DEBUG=True

That is sort of frustrating because this was my original PR

https://github.com/django/django/pull/7996/files

--
Ticket URL: <https://code.djangoproject.com/ticket/23004#comment:20>

Django

unread,
Apr 13, 2019, 4:07:00 PM4/13/19
to django-...@googlegroups.com
#23004: Cleanse entries from request.META in debug views
---------------------------------+-----------------------------------------
Reporter: Daniel Hahler | Owner: Daniel Maxson
Type: New feature | Status: assigned
Component: Error reporting | Version: master
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0

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

Comment (by Daniel Maxson):

My take is that we should assume that this page will end up on production
at some point. Or perhaps a developer is working on an internal Django
project and thinks it's safe to keep DEBUG=TRUE there, but someone else on
the inside maliciously leverages this. Or someone's reusing credentials
between staging and production.

I would prefer not to hear that Django was how someone pivoted from HTTP
access to a debug build to acquiring secrets. Even if some of the fault
would be on bad developer practices, some of the fault would also be on
Django for leaking secrets when it didn't need to (and if it does need to
in specific situations, that's configurable for people who know what
they're doing).

I think there's a strong security argument that we should assume this data
will leak, and so should be cleansed before it does.

--
Ticket URL: <https://code.djangoproject.com/ticket/23004#comment:21>

Django

unread,
Apr 13, 2019, 5:00:01 PM4/13/19
to django-...@googlegroups.com
#23004: Cleanse entries from request.META in debug views
---------------------------------+-----------------------------------------
Reporter: Daniel Hahler | Owner: Daniel Maxson
Type: New feature | Status: assigned
Component: Error reporting | Version: master
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0

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

Comment (by Markus Holtermann):

Replying to [comment:20 Ryan Castner]:


> > I'm not entirely sure we need that big of a change for this ticket,
especially considering that this is a DEBUG=True
>
> That is sort of frustrating because this was my original PR
>
> https://github.com/django/django/pull/7996/files

Yikes, I did not see your PR. I'm sorry! It does look indeed a lot like
mine. I'd be ok with re-opening your PR instead.

Replying to [comment:21 Daniel Maxson]:


> My take is that we should assume that this page will end up on
production at some point. Or perhaps a developer is working on an internal
Django project and thinks it's safe to keep DEBUG=TRUE there, but someone
else on the inside maliciously leverages this. Or someone's reusing
credentials between staging and production.
>
> I would prefer not to hear that Django was how someone pivoted from HTTP
access to a debug build to acquiring secrets. Even if some of the fault
would be on bad developer practices, some of the fault would also be on
Django for leaking secrets when it didn't need to (and if it does need to
in specific situations, that's configurable for people who know what
they're doing).
>
> I think there's a strong security argument that we should assume this
data will leak, and so should be cleansed before it does.

The difference between your and Ryan/mine PRs is that yours adds the
additional functionality to specify which variables to treat as sensitive.
Considering that the entire cleaning is a best-effort approach, I don't
think that warrants the additional complexity. Especially as variables
like `HTTP_PROXY` etc may still contain secrets (and I don't think we want
to hide everything that contains `HTTP`). The ''only'' safe approach is
not showing variables in the first place.

--
Ticket URL: <https://code.djangoproject.com/ticket/23004#comment:22>

Django

unread,
Apr 16, 2019, 5:56:06 AM4/16/19
to django-...@googlegroups.com
#23004: Cleanse entries from request.META in debug views
---------------------------------+-----------------------------------------
Reporter: Daniel Hahler | Owner: Daniel Maxson
Type: New feature | Status: assigned
Component: Error reporting | Version: master
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0

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

Comment (by Daniel Maxson):

Thanks for the clarification. I misunderstood your previous comment. I
think what you're saying gets to the point I mention in the PR here:

{{{
I added the values as optional construction arguments with the defaults
coming from constants defined on the class. The class definition looks a
little clunkier but my thinking was this may be more pleasant to use when
actually creating subclasses than needing to update the attributes only
after construction.

Still, I'm not married to this approach and can remove the arguments (and
make any other changes) if the approach in the new commits doesn't sit
right with people.
}}}

My PR was intended primarily as an effort to bring Ryan's PR up to date
just because there hadn't been work on it in a while and I wanted to get
this feature for my own use. I happened to throw in the arguments while
working on that because it felt strange to me at the time not to, but
having done that, I can see the case against the added complexity.

Regardless, adding the arguments is outside the scope of this ticket so in
retrospect I should have limited myself only to updating Ryan's PR.

--
Ticket URL: <https://code.djangoproject.com/ticket/23004#comment:23>

Django

unread,
Apr 23, 2019, 3:53:42 PM4/23/19
to django-...@googlegroups.com
#23004: Cleanse entries from request.META in debug views
---------------------------------+-----------------------------------------
Reporter: Daniel Hahler | Owner: Daniel Maxson
Type: New feature | Status: assigned
Component: Error reporting | Version: master
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0

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

Comment (by Oskar Haller):

I think Django Core Team should do a decision on how to proceed

--
Ticket URL: <https://code.djangoproject.com/ticket/23004#comment:24>

Django

unread,
Apr 30, 2019, 1:20:25 PM4/30/19
to django-...@googlegroups.com
#23004: Cleanse entries from request.META in debug views
---------------------------------+-----------------------------------------
Reporter: Daniel Hahler | Owner: Daniel Maxson
Type: New feature | Status: assigned
Component: Error reporting | Version: master
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0

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

Comment (by Daniel Maxson):

@Markus Holtermann: Out of curiosity, have you seen Carlton Gibson's
[https://github.com/django/django/pull/10748#pullrequestreview-203264592
comment] on the PR at Feb 13? Can you perhaps have that conversation and
arrive at an agreement on how to proceed?

--
Ticket URL: <https://code.djangoproject.com/ticket/23004#comment:25>

Django

unread,
May 23, 2019, 11:01:05 AM5/23/19
to django-...@googlegroups.com
#23004: Cleanse entries from request.META in debug views
---------------------------------+-----------------------------------------
Reporter: Daniel Hahler | Owner: Daniel Maxson
Type: New feature | Status: assigned
Component: Error reporting | Version: master
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
---------------------------------+-----------------------------------------

Comment (by Oskar Haller):

Is there any progress on this?

--
Ticket URL: <https://code.djangoproject.com/ticket/23004#comment:26>

Django

unread,
May 23, 2019, 11:54:07 AM5/23/19
to django-...@googlegroups.com
#23004: Cleanse entries from request.META in debug views
---------------------------------+-----------------------------------------
Reporter: Daniel Hahler | Owner: Daniel Maxson
Type: New feature | Status: assigned
Component: Error reporting | Version: master
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
---------------------------------+-----------------------------------------

Comment (by Carlton Gibson):

Yes. It’s just waiting for a re-review.

--
Ticket URL: <https://code.djangoproject.com/ticket/23004#comment:27>

Django

unread,
Jul 11, 2019, 4:29:12 AM7/11/19
to django-...@googlegroups.com
#23004: Cleanse entries from request.META in debug views
---------------------------------+-----------------------------------------
Reporter: Daniel Hahler | Owner: Daniel Maxson
Type: New feature | Status: assigned
Component: Error reporting | Version: master
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
---------------------------------+-----------------------------------------
Description changed by Carlton Gibson:

Old description:

New description:

In the debug views `settings` is cleansed, which hides e.g. `SECRET_KEY`.

But a lot of sensible information might also be present / come from
`request.META`, e.g. in the form of `DJANGO_SECRET_KEY` or `DATABASE_URL`.

It might be sensible to apply a filter in `TECHNICAL_500_TEMPLATE` (source
code reference:
https://github.com/django/django/blob/master/django/views/debug.py#L972-977).

I see that this can be quite specific, but I think it would be sensible to
apply `HIDDEN_SETTINGS` to all entries starting with `DJANGO_` and have a
setting for additional entries, which might default to `DATABASE_URL` and
`SENTRY_DSN`.

--

--
Ticket URL: <https://code.djangoproject.com/ticket/23004#comment:28>

Django

unread,
Jul 11, 2019, 7:03:59 AM7/11/19
to django-...@googlegroups.com
#23004: Cleanse entries from request.META in debug views
---------------------------------+-----------------------------------------
Reporter: Daniel Hahler | Owner: Daniel Maxson
Type: New feature | Status: assigned
Component: Error reporting | Version: master
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
---------------------------------+-----------------------------------------

Comment (by Carlton Gibson):

OK... Right...

First off, thank you everybody for input on this (and related tickets).

I say "and related tickets" because this kind of thing keeps coming up.
(e.g. #23951 but others) ''Can we filter this?'', ''Can we filter that?'',
and so on.

#29714 suggests making having a custom `ExceptionReporter` easier. In this
case you'd just override `get_traceback_data()` and job done. I'm hoping
to get that in.

There was [https://code.djangoproject.com/ticket/25167#comment:1 an idea
to get rid of the exception filter as the extension point] but I think
that's too quick for now. **As such**, I'd like to go for the more
ambitious of the two approaches here. I think
[https://code.djangoproject.com/ticket/23004#comment:2 Tim's original
comment, way back when] was the right one in terms of providing the right
flexibility without us having to add hooks for every possible use-case.

On that basis, I'm going to close Markus' PR (Thank you again Markus) and
then have one final look... I hope that makes sense.

> ...especially considering that this is a DEBUG=True.

It's not. `ExceptionReporter` is used `AdminEmailHandler` at the least.

--
Ticket URL: <https://code.djangoproject.com/ticket/23004#comment:29>

Django

unread,
Jul 12, 2019, 6:08:41 AM7/12/19
to django-...@googlegroups.com
#23004: Cleanse entries from request.META in debug views
---------------------------------+-----------------------------------------
Reporter: Daniel Hahler | Owner: Daniel Maxson
Type: New feature | Status: assigned
Component: Error reporting | Version: master
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 1

Easy pickings: 0 | UI/UX: 0
---------------------------------+-----------------------------------------
Changes (by Carlton Gibson):

* needs_better_patch: 0 => 1


Comment:

PR is looking good, but needs a little work on the docs, (as per review on
PR).

Daniel, I'd like to get this in: combined with #29714, it will clear up a
whole group of the "Error Reporting" tickets. Please let me know if you
need input or don't have capacity to finish it off.

(Thanks for your effort! And same to all!)

--
Ticket URL: <https://code.djangoproject.com/ticket/23004#comment:30>

Django

unread,
Jan 9, 2020, 6:04:51 AM1/9/20
to django-...@googlegroups.com
#23004: Cleanse entries from request.META in debug views
---------------------------------+-----------------------------------------
Reporter: Daniel Hahler | Owner: Daniel Maxson
Type: New feature | Status: assigned
Component: Error reporting | Version: master
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0

Easy pickings: 0 | UI/UX: 0
---------------------------------+-----------------------------------------
Changes (by Carlton Gibson):

* needs_better_patch: 1 => 0


Comment:

New PR adjusting for feedback https://github.com/django/django/pull/12300

--
Ticket URL: <https://code.djangoproject.com/ticket/23004#comment:31>

Django

unread,
Jan 10, 2020, 6:01:18 AM1/10/20
to django-...@googlegroups.com
#23004: Cleanse entries from request.META in debug views
---------------------------------+-----------------------------------------
Reporter: Daniel Hahler | Owner: Daniel Maxson
Type: New feature | Status: closed

Component: Error reporting | Version: master
Severity: Normal | Resolution: fixed

Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
---------------------------------+-----------------------------------------
Changes (by Mariusz Felisiak <felisiak.mariusz@…>):

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


Comment:

In [changeset:"e2d9d66a22f9004c0349f6aa9f8762fa558bdee8" e2d9d66a]:
{{{
#!CommitTicketReference repository=""
revision="e2d9d66a22f9004c0349f6aa9f8762fa558bdee8"
Fixed #23004 -- Added request.META filtering to
SafeExceptionReporterFilter.

Co-authored-by: Ryan Castner <castn...@gmail.com>
}}}

--
Ticket URL: <https://code.djangoproject.com/ticket/23004#comment:33>

Django

unread,
Jan 10, 2020, 6:01:19 AM1/10/20
to django-...@googlegroups.com
#23004: Cleanse entries from request.META in debug views
---------------------------------+-----------------------------------------
Reporter: Daniel Hahler | Owner: Daniel Maxson
Type: New feature | Status: assigned

Component: Error reporting | Version: master
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
---------------------------------+-----------------------------------------

Comment (by Mariusz Felisiak <felisiak.mariusz@…>):

In [changeset:"581ba5a9486ed73cb81031d85b3ce1b27a960109" 581ba5a9]:
{{{
#!CommitTicketReference repository=""
revision="581ba5a9486ed73cb81031d85b3ce1b27a960109"
Refs #23004 -- Allowed exception reporter filters to customize settings
filtering.

Thanks to Tim Graham for the original implementation idea.

Co-authored-by: Daniel Maxson <dma...@ccpgames.com>
}}}

--
Ticket URL: <https://code.djangoproject.com/ticket/23004#comment:32>

Reply all
Reply to author
Forward
0 new messages