[Django] #29282: Admin Checks May Raise TypeError
Django
-----------------------------------------+------------------------
Reporter: David Sanders | Owner: nobody
Type: Bug | Status: new
Component: contrib.admin | Version: master
Severity: Normal | Keywords:
Triage Stage: Unreviewed | Has patch: 1
Needs documentation: 0 | Needs tests: 0
Patch needs improvement: 0 | Easy pickings: 0
UI/UX: 0 |
-----------------------------------------+------------------------
Admin checks include several calls to `issubclass` without a
`try`/`catch`, allowing a `TypeError` to escape if the user provides a
value that isn't a `class` for whatever is being checked. This seems like
a sane-enough edge case to ensure the user gets a useful check failed
message rather than an uncaught exception. Providing a PR that uses a
helper function that catches the `TypeError` and returns `False` instead
so that the half-dozen checks that use `issubclass` have minimal code
change.
--
Ticket URL: <https://code.djangoproject.com/ticket/29282>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.
Django
-------------------------------+------------------------------------
Reporter: David Sanders | Owner: nobody
Type: Bug | Status: new
Component: contrib.admin | Version: master
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Changes (by Tim Graham):
* needs_better_patch: 0 => 1
* stage: Unreviewed => Accepted
Old description:
> Admin checks include several calls to `issubclass` without a
> `try`/`catch`, allowing a `TypeError` to escape if the user provides a
> value that isn't a `class` for whatever is being checked. This seems like
> a sane-enough edge case to ensure the user gets a useful check failed
> message rather than an uncaught exception. Providing a PR that uses a
> helper function that catches the `TypeError` and returns `False` instead
> so that the half-dozen checks that use `issubclass` have minimal code
> change.
New description:
Admin checks include several calls to `issubclass` without a
`try`/`catch`, allowing a `TypeError` to escape if the user provides a
value that isn't a `class` for whatever is being checked. This seems like
a sane-enough edge case to ensure the user gets a useful check failed
message rather than an uncaught exception. My
[https://github.com/django/django/pull/9844 PR] uses a helper function
that catches the `TypeError` and returns `False` instead so that the half-
dozen checks that use `issubclass` have minimal code change.
--
--
Ticket URL: <https://code.djangoproject.com/ticket/29282#comment:1>
Django
-------------------------------+------------------------------------
Reporter: David Sanders | Owner: nobody
Type: Bug | Status: new
Component: contrib.admin | Version: master
Keywords: | Triage Stage: Accepted
Easy pickings: 0 | UI/UX: 0
--
Ticket URL: <https://code.djangoproject.com/ticket/29282#comment:2>
Django
-------------------------------+------------------------------------
Type: Bug | Status: new
Component: contrib.admin | Version: master
Keywords: | Triage Stage: Accepted
Easy pickings: 0 | UI/UX: 0
Changes (by Herbert Fortes):
* cc: Herbert Fortes (added)
Comment:
The ticket is quiet for three months.
I read the PR and, only a function name is in a discussion.
I think the ticket was forgotten.
--
Ticket URL: <https://code.djangoproject.com/ticket/29282#comment:3>
Django
-------------------------------+------------------------------------
Type: Bug | Status: new
Component: contrib.admin | Version: master
Keywords: | Triage Stage: Accepted
Easy pickings: 0 | UI/UX: 0
Comment (by Sanyam Khurana):
Hi David,
I see this ticket is pending for a while. I would help to like out if
you're not actively working on this one anymore.
--
Ticket URL: <https://code.djangoproject.com/ticket/29282#comment:4>
Django
Reporter: David Sanders | Owner: Sanyam Khurana
Type: Bug | Status: assigned
Component: contrib.admin | Version: master
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Easy pickings: 0 | UI/UX: 0
Changes (by Sanyam Khurana):
* owner: nobody => Sanyam Khurana
* status: new => assigned
--
Ticket URL: <https://code.djangoproject.com/ticket/29282#comment:5>
Django
-------------------------------+------------------------------------------
Reporter: David Sanders | Owner: Sanyam Khurana
Type: Bug | Status: assigned
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Changes (by Tim Graham):
* needs_better_patch: 1 => 0
Comment:
[https://github.com/django/django/pull/10652 PR]
--
Ticket URL: <https://code.djangoproject.com/ticket/29282#comment:6>
Django
Reporter: David Sanders | Owner: Sanyam
| Khurana
Type: Bug | Status: assigned
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 Carlton Gibson):
* stage: Accepted => Ready for checkin
Comment:
Yep. Updated patch looks good.
--
Ticket URL: <https://code.djangoproject.com/ticket/29282#comment:7>
Django
Reporter: David Sanders | Owner: Sanyam Khurana
Type: Bug | Status: assigned
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Changes (by Carlton Gibson):
* stage: Ready for checkin => Accepted
Comment:
Ooops, I misinterpreted the history from the previous PR. A bit more to do
here.
--
Ticket URL: <https://code.djangoproject.com/ticket/29282#comment:8>
Django
Reporter: David Sanders | Owner: Sanyam
| Khurana
Type: Bug | Status: assigned
Severity: Normal | Resolution:
Keywords: | Triage Stage: Ready for
| checkin
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Changes (by Sanyam Khurana):
* stage: Accepted => Ready for checkin
Comment:
PR updated addressing all the reviews.
https://github.com/django/django/pull/10652/files
--
Ticket URL: <https://code.djangoproject.com/ticket/29282#comment:9>
Django
-------------------------------------+-------------------------------------
Reporter: David Sanders | Owner: Sanyam
| Khurana
Component: contrib.admin | Version: master
Severity: Normal | Resolution: fixed
Keywords: | Triage Stage: Ready for
| checkin
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Changes (by Tim Graham <timograham@…>):
* status: assigned => closed
* resolution: => fixed
Comment:
In [changeset:"a7d6cab77138f23ef37ec14acb27cb263538e6a7" a7d6cab]:
{{{
#!CommitTicketReference repository=""
revision="a7d6cab77138f23ef37ec14acb27cb263538e6a7"
Fixed #29282 -- Prevented some admin checks from crashing with TypeError.
Co-authored-by: David Sanders <dsand...@ucsbalum.com>
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/29282#comment:10>