Admin changelist search crashes (500) on `__exact` search_fields with choices

[Adam Johnson]

Regression in 4cecf3039586ea738afafb9a28c946bff42c37c1 (#36865), which replaced Cast-based comparison of non-text __exact search fields with per-term validation via the model field's formfield().to_python().
That validation is insufficient for two kinds of fields:

1. Crash (HTTP 500) for fields with choices

For a model field with choices (e.g. IntegerField(choices=...)), formfield() returns a TypedChoiceField whose to_python() returns the raw string unvalidated.
The term then reaches the ORM and IntegerField.get_prep_value() raises ValueError: invalid literal for int() with base 10: 'john'.
Since ModelAdmin.get_search_results() is called outside the IncorrectLookupParameters handling in ChangeList.get_queryset() (django/contrib/admin/views/main.py), the error propagates as a server error.

Minimal repro:

class Client(models.Model):
    name = models.CharField(max_length=30)
    status = models.IntegerField(choices=[(1, "Active"), (2, "Archived")])

class ClientAdmin(admin.ModelAdmin):
    search_fields = ["name", "status__exact"]

Searching for john in the changelist returns HTTP 500 on 6.1 and main; on 6.0 it returned the rows whose name matches.

2. Over-matching for BooleanField

For BooleanField __exact entries, forms.BooleanField.to_python() maps almost any string to True (only "false"/"0" map to False; nothing raises), so any search term OR-matches every row with a True value.

class Account(models.Model):
    name = models.CharField(max_length=30)
    active = models.BooleanField(default=True)

class AccountAdmin(admin.ModelAdmin):
    search_fields = ["name", "active__exact"]

Searching for john returns every active account on 6.1 instead of just john (6.0 behavior).

[Jacob Walls]

Thanks. For each case:

• BooleanField: The commit message explains that there is an increase in search permissiveness for values that can be normalized by formfield.to_python(). So for me, the permissive matching on truthy values is fine. It would be unexpected for me for NullBooleanField to yield a search term None that will never match a not-null database column.

By the way, do you think we should have done this, to allow folks to customize? I'm not sure if that's overloading the responsibility for this hook:

django/contrib/admin/options.py

@@ -1363 +1363 @@
                 bit_lookups = []
                 for orm_lookup, validate_field in orm_lookups:
                     if validate_field is not None:
-                        formfield = validate_field.formfield()
+                        formfield = self.formfield_for_dbfield(validate_field, request)
                         try:
                             if formfield is not None:
                                 value = formfield.to_python(bit)

• TypedChoiceField: I think this use case gives us a reason to reopen #34156, which, when you connect the dots a little further, says that TypedChoiceField is violating the contract that to_python() should return the correct python types. The original ask in #21397 was just for coercion to produce values outside choices, not different python types. I'll post a draft patch on #34156.

---

The provided PR does this:

                                if isinstance(value, str):
                                    # Form fields such as TypedChoiceField may
                                    # return the string unconverted. Ensure
                                    # the model field accepts the value, so
                                    # that filtering on it cannot crash.
                                    value = validate_field.to_python(value)

... but I'm worried about a new can of worms/inconsistencies there.

I'll accept this for the ChoiceField issue. I think I'd rather special-case the problematic cases we know about (forms.TypedChoiceField) and let that light a fire under us to fix it in the right layer for the next cycle (and also avoid re-committing to problematic performance with casts).

[Adam Johnson]

BooleanField: The commit message explains that there is an increase in search permissiveness for values that can be normalized by formfield.to_python(). So for me, the permissive matching on truthy values is fine.

Did you understand that a search for an arbitrary string now matches every row where any boolean field is true? That’s a huge regression to me. Previously, you'd need to serach for true to match truthy rows only.

Try running the new tests on the current main: ​https://github.com/django/django/pull/21754/changes#diff-640602cc17dc3e9a26db47f58b40a1289d5c0f1b5b0fe547810b1d962f6cde8bR996

TypedChoiceField: I think this use case gives us a reason to reopen #34156, which, when you connect the dots a little further, says that TypedChoiceField is violating the contract that to_python() should return the correct python types.

Oh yeah, that’s a problem! Thanks for looking into it.

[Jacob Walls]

Replying to Adam Johnson:

Did you understand that a search for an arbitrary string now matches every row where any boolean field is true? That’s a huge regression to me. Previously, you'd need to serach for true to match truthy rows only.

Ah, no, I didn't contemplate the effect of all the terms being OR'd together, even though it's right there in your report, so thanks for connecting those dots for me. Agree we have to do something! Probably a distinct formfield for search. Mabye with an overridable datastructure to hold it so that it doesn't overload self.formfield_for_dbfield?

[Jacob Walls]

I pushed a couple proposed tweaks to reduce the types of exceptions caught, and to push the special cases out of inline logic and into custom form classes where possible. It's ready for opinions.

[Jacob Walls]

Should probably look into making ChangeList.search_form_class more field-granular.

[Jacob Walls]

Replying to Jacob Walls:

Should probably look into making ChangeList.search_form_class more field-granular.

Never mind, this idea turned out to be awkward, since def get_search_results is on ModelAdmin itself, so this would introduce a two-way reference back to the ChangeList.