default='' (non-bytestring) on BinaryField crashes some migration operations

[James]

Description

Initial migration has a default value '' for BinaryField.
Later, change default value to b'' and migrate.
Trying to undo this migration fails. It seems like '' is allowed during migration, but not in reverse migration.

Related issue

#22851 Default value for BinaryField

Reproduce

Python 3.6.0, Django 1.10.6, Postgres 9.5.4

1. startproject djangoproject
2. startapp firstapp
3. firstapp/models.py:

class TableOne(models.Model):
    field1 = models.BinaryField(default = '')

4. makemigrations firstapp
5. migrate firstapp 0001
6. Modify firstapp/models.py

class TableOne(models.Model):
    field1 = models.BinaryField(default = b'')

6. migrate firstapp 0002
7. migrate firstapp 0001

Error: TypeError: can't escape str to binary

Traceback (most recent call last):
  File "manage.py", line 22, in <module>
    execute_from_command_line(sys.argv)
  File "C:\Py\py3_64\lib\site-packages\django\core\management\__init__.py", line 367, in execute_from_command_line
    utility.execute()
  File "C:\Py\py3_64\lib\site-packages\django\core\management\__init__.py", line 359, in execute
    self.fetch_command(subcommand).run_from_argv(self.argv)
  File "C:\Py\py3_64\lib\site-packages\django\core\management\base.py", line 294, in run_from_argv
    self.execute(*args, **cmd_options)
  File "C:\Py\py3_64\lib\site-packages\django\core\management\base.py", line 345, in execute
    output = self.handle(*args, **options)
  File "C:\Py\py3_64\lib\site-packages\django\core\management\commands\migrate.py", line 204, in handle
    fake_initial=fake_initial,
  File "C:\Py\py3_64\lib\site-packages\django\db\migrations\executor.py", line 119, in migrate
    state = self._migrate_all_backwards(plan, full_plan, fake=fake)
  File "C:\Py\py3_64\lib\site-packages\django\db\migrations\executor.py", line 194, in _migrate_all_backwards
    self.unapply_migration(states[migration], migration, fake=fake)
  File "C:\Py\py3_64\lib\site-packages\django\db\migrations\executor.py", line 264, in unapply_migration
    state = migration.unapply(state, schema_editor)
  File "C:\Py\py3_64\lib\site-packages\django\db\migrations\migration.py", line 178, in unapply
    operation.database_backwards(self.app_label, schema_editor, from_state, to_state)
  File "C:\Py\py3_64\lib\site-packages\django\db\migrations\operations\fields.py", line 210, in database_backwards
    self.database_forwards(app_label, schema_editor, from_state, to_state)
  File "C:\Py\py3_64\lib\site-packages\django\db\migrations\operations\fields.py", line 205, in database_forwards
    schema_editor.alter_field(from_model, from_field, to_field)
  File "C:\Py\py3_64\lib\site-packages\django\db\backends\base\schema.py", line 506, in alter_field
    old_db_params, new_db_params, strict)
  File "C:\Py\py3_64\lib\site-packages\django\db\backends\postgresql\schema.py", line 118, in _alter_field
    new_db_params, strict,
  File "C:\Py\py3_64\lib\site-packages\django\db\backends\base\schema.py", line 660, in _alter_field
    params,
  File "C:\Py\py3_64\lib\site-packages\django\db\backends\base\schema.py", line 112, in execute
    cursor.execute(sql, params)
  File "C:\Py\py3_64\lib\site-packages\django\db\backends\utils.py", line 80, in execute
    return super(CursorDebugWrapper, self).execute(sql, params)
  File "C:\Py\py3_64\lib\site-packages\django\db\backends\utils.py", line 65, in execute
    return self.cursor.execute(sql, params)
TypeError: can't escape str to binary

Notes

site-packages\django\db\backends\base\shema.py def effective_default(self, field):

determines default as an empty <class 'str'>, when (default = '')

Possible Fix?

site-packages\django\db\backends\base\shema.py ~line 197

def effective_default(self, field):
        if field.has_default():
            default = field.get_default()
            if field.get_internal_type() == "BinaryField" and not default:
                default = six.binary_type()
        elif not field.null and field.blank and field.empty_strings_allowed:
            if field.get_internal_type() == "BinaryField":
                default = six.binary_type()
            else:
                default = six.text_type()
        elif getattr(field, 'auto_now', False)

[Tim Graham]

While doing a quick test, I noticed that a non-bytestring also crashes with AddField (forward). I'm not sure about the proper resolution (perhaps a system check error for an invalid default?) but the inconsistency is certainly unexpected.

[Windson yang]

It didn't crash when I using sqlite3, maybe related to Postgres, right?

[Claude Paroz]

I would also suggest a system check to prevent default strings in the first place. Something like:

diff --git a/django/db/models/fields/__init__.py b/django/db/models/fields/__init__.py
index b2e9b18351..af4671c0f6 100644
--- a/django/db/models/fields/__init__.py
+++ b/django/db/models/fields/__init__.py
@@ -2292,6 +2292,22 @@ class BinaryField(Field):
         if self.max_length is not None:
             self.validators.append(validators.MaxLengthValidator(self.max_length))
 
+    def check(self, **kwargs):
+        errors = super().check(**kwargs)
+        errors.extend(self._check_default_is_not_str(**kwargs))
+        return errors
+
+    def _check_default_is_not_str(self, **kwargs):
+        if self.has_default() and isinstance(self.default, str):
+            return [
+                checks.Error(
+                    "BinaryField 'default' cannot be a string, use bytes content instead.",
+                    obj=self,
+                    id='fields.E170',
+                )
+            ]
+        return []
+
     def deconstruct(self):
         name, path, args, kwargs = super().deconstruct()
         del kwargs['editable']

[Hasan Ramezani]

System check for preventing default string value for BinaryField added based on @Claude Paroz suggestion

[Mariusz Felisiak <felisiak.mariusz@…>]

In 981dd6dd:

Fixed #28431 -- Added a system check for BinaryField to prevent strings defaults.

Thanks Claude Paroz for the initial patch.