[Django] #28992: Admin search over GenericIPAddressField with badly formatted IP address causes a DataError
Django
causes a DataError
-----------------------------------------+------------------------
Reporter: Vlada Macek | Owner: nobody
Type: Bug | Status: new
Component: contrib.admin | Version: 1.11
Severity: Normal | Keywords:
Triage Stage: Unreviewed | Has patch: 0
Needs documentation: 0 | Needs tests: 0
Patch needs improvement: 0 | Easy pickings: 0
UI/UX: 0 |
-----------------------------------------+------------------------
When I have a @GenericIPAddressField@ of the model in @search_fields@ of a
@ModelAdmin@ and then search for example a space, the admin dies with 500
and
@DataError: invalid input syntax for type inet: " "@
exception is e-mailed.
--
Ticket URL: <https://code.djangoproject.com/ticket/28992>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.
Django
causes a DataError
Reporter: Vlada Macek | Owner: nobody
Type: Bug | Status: new
Component: contrib.admin | Version: 1.11
Keywords: | Triage Stage: Unreviewed
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Description changed by Vlada Macek:
Old description:
> When I have a @GenericIPAddressField@ of the model in @search_fields@ of
> a @ModelAdmin@ and then search for example a space, the admin dies with
> 500 and
>
> @DataError: invalid input syntax for type inet: " "@
>
> exception is e-mailed.
New description:
When I have a `GenericIPAddressField` of the model in `search_fields` of a
`ModelAdmin` and then search for example a space, the admin dies with 500
and
`DataError: invalid input syntax for type inet: " "`
exception is e-mailed.
--
--
Ticket URL: <https://code.djangoproject.com/ticket/28992#comment:1>
Django
PostgreSQL
-------------------------------------+-------------------------------------
Reporter: Vlada Macek | Owner: nobody
Type: Bug | Status: new
(models, ORM) |
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 Tim Graham):
* component: contrib.admin => Database layer (models, ORM)
* stage: Unreviewed => Accepted
Comment:
The problem isn't in the admin. For example,
`GenericIPAddress.objects.filter(ip=' ')` also crashes (using the model in
`tests/model_fields/test_genericipaddressfield.py`).
--
Ticket URL: <https://code.djangoproject.com/ticket/28992#comment:2>
Django
PostgreSQL
-------------------------------------+-------------------------------------
Type: Bug | Status: new
(models, ORM) |
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Comment (by Vlada Macek):
I was't this much specific in the Summary as not only space causes the
DataError, but apparently a syntactically wrong IPv4/IPv6 address, any of
`'string'`, `'10.10.10.300'`, `'fffg::'`.
I selected admin component as I considered the DataError from ORM as part
of correct "user interface" to the developer. What is clearly wrong IMHO
is 500 from using the admin.
Thanks.
--
Ticket URL: <https://code.djangoproject.com/ticket/28992#comment:3>
Django
PostgreSQL
-------------------------------------+-------------------------------------
Type: Bug | Status: new
(models, ORM) |
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Comment (by Claude Paroz):
A possible solution would be to validate the value in
`DatabaseOperations.adapt_ipaddressfield_value`. Possibly trying first
with our own validator, then round-tripping to the database as last
resort. Maybe a `ValidationError` before running the query may be catched
easier than the error coming from the database.
--
Ticket URL: <https://code.djangoproject.com/ticket/28992#comment:4>
Django
PostgreSQL
-------------------------------------+-------------------------------------
| Sarıgöl
Type: Bug | Status: assigned
Component: Database layer | Version: master
(models, ORM) |
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Changes (by Can Sarıgöl):
* owner: nobody => Can Sarıgöl
* status: new => assigned
* has_patch: 0 => 1
* version: 1.11 => master
Comment:
[https://github.com/django/django/pull/11682 PR]
--
Ticket URL: <https://code.djangoproject.com/ticket/28992#comment:5>
Django
PostgreSQL
-------------------------------------+-------------------------------------
Reporter: Vlada Macek | Owner: Can
| Sarıgöl
Type: Bug | Status: assigned
Component: Database layer | Version: master
(models, ORM) |
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Comment (by Simon Charette):
I know some lookup raise `ValueError` or `TypeError` when an invalid value
is provided but none raise `ValidationError` AFAIK. IMO this is an issue
similar to #29915 where a specialized `__contains` lookup must be
implemented for `GenericIPAddressField`.
--
Ticket URL: <https://code.djangoproject.com/ticket/28992#comment:6>
Django
PostgreSQL
-------------------------------------+-------------------------------------
Reporter: Vlada Macek | Owner: Can
| Sarıgöl
Type: Bug | Status: assigned
Component: Database layer | Version: master
(models, ORM) |
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Comment (by Carlton Gibson):
> What is clearly wrong IMHO is 500 from using the admin.
#30064 adds a form to `ChangeList` to validate the search query input.
You'll be able to subclass `ChangeList`, and override `get_changelist()`,
to customise that form (adding an additional validator for IP addresses,
say).
If you have multiple search fields, you'll likely need to override
`get_search_results()`
[https://docs.djangoproject.com/en/2.2/ref/contrib/admin/#django.contrib.admin.ModelAdmin.get_search_results
see the docs for that], since the same query value is used to filter on
all the fields. (Something like `django-filter` would allow per field
validation and filtering, but not from a single search box normally...)
From the UI perspective, I think that's more or less all we can really do
here. [https://code.djangoproject.com/ticket/30064#comment:3 Simon's
comment from #30064] sums it up:
> In my opinion the issue is that the admin is relying on unsanitized
request.GET passing to the ORM, that looks like the same class of issues
as Model.objects.get(int_field='foo'). The admin should use a form to
sanitize the input...
(See too the "User input should be sanitized before feeding it to the
ORM." below that.)
Validating against null characters is something we can apply to all input,
but IP addresses, say, are only going to apply sometimes, so the developer
needs to adjust the form themselves. (Short of a django-filter like
mechanism that will generate a full model-form, which is out of scope
here.)
As per the rest of the discussion here, what that leaves for this ticket
is the ORM behaviour: to raise a ValueError before hitting the DB in this
case.
--
Ticket URL: <https://code.djangoproject.com/ticket/28992#comment:7>
Django
PostgreSQL
-------------------------------------+-------------------------------------
Reporter: Vlada Macek | Owner: Can
| Sarıgöl
Type: Bug | Status: assigned
Component: Database layer | Version: master
(models, ORM) |
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 1
Easy pickings: 0 | UI/UX: 0
Changes (by felixxm):
* needs_better_patch: 0 => 1
--
Ticket URL: <https://code.djangoproject.com/ticket/28992#comment:8>
Django
PostgreSQL
-------------------------------------+-------------------------------------
Type: Bug | Status: assigned
Component: Database layer | Version: dev
(models, ORM) |
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 1
Easy pickings: 0 | UI/UX: 0
Changes (by Can Sarıgöl):
* owner: Can Sarıgöl => (none)
--
Ticket URL: <https://code.djangoproject.com/ticket/28992#comment:9>