Opened 16 months ago

Last modified 11 months ago

#22224 new Bug

Non-nullable blank string-based model field validation doesn't prevent or clean `None`

Reported by: charettes Owned by: nobody
Component: Database layer (models, ORM) Version: master
Severity: Normal Keywords:
Cc: Triage Stage: Accepted
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: yes
Easy pickings: no UI/UX: no

Description (last modified by charettes)

Since we discourage the use of NULLable string-based fields the assumed way of holding text that might be empty would be to use the blank option.

In this case I would expect full_clean to raise a ValidationError when an empty non-nullable field is assigned None:

from django.db import models

class A(models.Model):
    b = models.CharField(blank=True)

a = A(b=None)
a.full_clean()  # Passes
a.save()  # Integrity error

Unfortunately specifying blank=True disable all validation against the b field, leading to database integrity error upon calling save(). The obvious solution here would be to override A.clean to convert b to an empty string when it equals None but it strikes me as a non trivial change for a quite common use case: correctly allowing a string-based field to be empty.

This is not an issue when cleaning data retrieved from the usual application/form-url-encoded request.GET or request.POST since an empty value is always represented by the empty string. However, in the case of an application/json encoded payload one may either provide None or '' (null or '') to express emptiness. Which will trigger the issue described above.

Attaching a patch that special case the empty_values of non-nullable string based fields based on the empty_strings_allowed flag. All tests pass on Py2.7/SQLite3.

Change History (6)

comment:1 Changed 16 months ago by charettes

  • Component changed from Uncategorized to Database layer (models, ORM)
  • Has patch set

comment:2 Changed 16 months ago by charettes

  • Description modified (diff)

comment:3 Changed 16 months ago by claudep

  • Triage Stage changed from Unreviewed to Accepted

comment:4 Changed 16 months ago by charettes

As pointed out by Loïc this issue is related to #20205 which propose to clean '' to None for fields disallowing empty strings.

Maybe the correct solution here would be to clean None to '' for non-null fields allowing empty strings?

comment:5 Changed 16 months ago by charettes

Another solution might be to special case get_db_prep_value to convert None to '' when the field is not NULL'able and empty_strings_allowed.

This would be completely backward compatible in the case of a model specific clean() with a self.non_null_string_based_field is None conditional.

comment:6 Changed 11 months ago by akaariai

  • Patch needs improvement set

It seems the problem here is that for HTML forms, "" is the natural None value, so for example IntegerField="" should validate and set the field to Zero. But for model validation this isn't the case. In that use case providing "" for IntegerField doesn't make sense. We could maybe make forms treat "" specially (convert to None unless the underlying field accepts empty strings), and then remove "" from empty_values for practically every field. Unfortunately changes here seem problematic as the problem isn't that large currently, but the backwards compatibility issues will be large.

Also, maybe we could just remove "" from empty values for all fields that accept "" directly as a value. The logic is that while "" is kinda special value for CharField, it is just one value in its domain. However, for IntegerField (and other fields that do not accept "" directly) the empty string is not in their domain, and thus it can be treated as None.

I'm marking patch needs improvement for lack of better marker. What is needed here is a design decision of how we want to treat "" in CharFields.

Note: See TracTickets for help on using tickets.
Back to Top