[Django] #21564: Generic base view http_method_not_allowed method references self.request and should use locally scoped request
Django
and should use locally scoped request
-------------------------------+--------------------
Reporter: adepue | Owner: nobody
Type: Uncategorized | Status: new
Component: Generic views | Version: 1.6
Severity: Normal | Keywords:
Triage Stage: Unreviewed | Has patch: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------+--------------------
The current method in django/views/generic/base.py is:
def http_method_not_allowed(self, request, *args, **kwargs):
logger.warning('Method Not Allowed (%s): %s', request.method,
request.path,
extra={
'status_code': 405,
'request': self.request
}
)
return http.HttpResponseNotAllowed(self._allowed_methods())
The 'extra' dictionary should reference the locally scoped 'request'
object instead of the class scoped member.
There is a PR open with this fix:
https://github.com/django/django/pull/2035
The side effect is that any unit test written that uses the view and
manually calls the dispatch method to validate non-allowed-methods will
crash with attribute not found error for request
--
Ticket URL: <https://code.djangoproject.com/ticket/21564>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.
Django
and should use locally scoped request
Reporter: adepue | Owner: nobody
Type: Bug | Status: new
Component: Generic views | Version: 1.6
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 adepue):
* needs_better_patch: => 0
* type: Uncategorized => Bug
* needs_tests: => 0
* needs_docs: => 0
--
Ticket URL: <https://code.djangoproject.com/ticket/21564#comment:1>
Django
and should use locally scoped request
Reporter: adepue | Owner: nobody
Type: Bug | Status: new
Component: Generic views | Version: 1.6
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Needs tests: 1 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Changes (by claudep):
* has_patch: 0 => 1
* needs_tests: 0 => 1
* stage: Unreviewed => Accepted
--
Ticket URL: <https://code.djangoproject.com/ticket/21564#comment:2>
Django
and should use locally scoped request
Reporter: adepue | Owner: nobody
Type: Bug | Status: new
Component: Generic views | Version: 1.6
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Comment (by adepue):
Test added to PR.
--
Ticket URL: <https://code.djangoproject.com/ticket/21564#comment:3>
Django
and should use locally scoped request
Reporter: adepue | Owner: nobody
Type: Bug | Status: new
Component: Generic views | Version: 1.6
Keywords: | Triage Stage: Ready for
Has patch: 1 | checkin
Needs tests: 0 | Needs documentation: 0
Easy pickings: 0 | Patch needs improvement: 0
| UI/UX: 0
-------------------------------------+-------------------------------------
Changes (by timo):
* needs_tests: 1 => 0
* stage: Accepted => Ready for checkin
Old description:
> The current method in django/views/generic/base.py is:
> def http_method_not_allowed(self, request, *args, **kwargs):
> logger.warning('Method Not Allowed (%s): %s', request.method,
> request.path,
> extra={
> 'status_code': 405,
> 'request': self.request
> }
> )
> return http.HttpResponseNotAllowed(self._allowed_methods())
>
> The 'extra' dictionary should reference the locally scoped 'request'
> object instead of the class scoped member.
>
> There is a PR open with this fix:
> https://github.com/django/django/pull/2035
>
> The side effect is that any unit test written that uses the view and
> manually calls the dispatch method to validate non-allowed-methods will
> crash with attribute not found error for request
New description:
The current method in django/views/generic/base.py is:
{{{
def http_method_not_allowed(self, request, *args, **kwargs):
logger.warning('Method Not Allowed (%s): %s', request.method,
request.path,
extra={
'status_code': 405,
'request': self.request
}
)
return http.HttpResponseNotAllowed(self._allowed_methods())
}}}
The 'extra' dictionary should reference the locally scoped 'request'
object instead of the class scoped member.
There is a PR open with this fix:
https://github.com/django/django/pull/2035
The side effect is that any unit test written that uses the view and
manually calls the dispatch method to validate non-allowed-methods will
crash with attribute not found error for request
--
--
Ticket URL: <https://code.djangoproject.com/ticket/21564#comment:4>
Django
and should use locally scoped request
Reporter: adepue | Owner: nobody
Type: Bug | Status: new
Component: Generic views | Version: 1.6
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Changes (by bmispelon):
* needs_better_patch: 0 => 1
* stage: Ready for checkin => Accepted
Comment:
There's another instance of a similar behavior in `RedirectView.get`:
https://github.com/django/django/blob/master/django/views/generic/base.py#L199
Would you mind fixing that one too and adding a test for it?
Thanks.
--
Ticket URL: <https://code.djangoproject.com/ticket/21564#comment:5>
Django
and should use locally scoped request
Reporter: adepue | Owner: nobody
Type: Bug | Status: new
Component: Generic views | Version: 1.6
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 1
Comment (by timo):
In addition, I was going to backport this to 1.6.X since it's a crashing
issue. Could you add the following to docs/releases/1.6.1.txt as well?
"Fixed a typo in
:meth:`django.views.generic.base.View.http_method_not_allowed` that caused
it to crash in some cases (#21564)." (feel free to amend as you see fit
and to account for the request from bmispelon)?
--
Ticket URL: <https://code.djangoproject.com/ticket/21564#comment:6>
Django
and should use locally scoped request
Reporter: adepue | Owner: nobody
Type: Bug | Status: new
Component: Generic views | Version: 1.6
Keywords: | Triage Stage: Ready for
Has patch: 1 | checkin
Needs tests: 0 | Needs documentation: 0
Easy pickings: 0 | Patch needs improvement: 0
| UI/UX: 0
-------------------------------------+-------------------------------------
Changes (by timo):
* needs_better_patch: 1 => 0
* stage: Accepted => Ready for checkin
--
Ticket URL: <https://code.djangoproject.com/ticket/21564#comment:7>
Django
and should use locally scoped request
Reporter: adepue | Owner: nobody
Type: Bug | Status: closed
Component: Generic views | Version: 1.6
Keywords: | Triage Stage: Ready for
Has patch: 1 | checkin
Needs tests: 0 | Needs documentation: 0
Easy pickings: 0 | Patch needs improvement: 0
| UI/UX: 0
-------------------------------------+-------------------------------------
* status: new => closed
* resolution: => fixed
Comment:
In [changeset:"e2f142030b81a37e1c3187f5d336dcb6014fd1c0"]:
{{{
#!CommitTicketReference repository=""
revision="e2f142030b81a37e1c3187f5d336dcb6014fd1c0"
Fixed #21564 -- Use local request object when possible in generic views.
Thanks to trac user adepue for the report and original patch.
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/21564#comment:8>