Opened 15 months ago

Closed 12 months ago

Last modified 5 months ago

#28201 closed Cleanup/optimization (fixed)

Make CharField form field prohibit null characters

Reported by: CM Lubinski Owned by: Alejandro Zamora Fonseca
Component: Forms Version: 1.11
Severity: Normal Keywords:
Cc: Vlada Macek Triage Stage: Ready for checkin
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no

Description

Input with null characters cause Django to crash when saving data.

Reproduction recipe:

  1. Setup Django 1.11.1, Postgres (I don't think the version matters) using Python 3.5 (though this may apply to any 3.x).
  2. Create an admin account & login.
  3. Navigate to /admin/auth/group/add/
  4. Using the JavaScript console, execute
    $('#id_name').val("\x00something")
    
  5. Submit the form. See the error

Result will be similar to:

dev-api_1           | Internal Server Error: /admin/auth/group/add/
dev-api_1           | Traceback (most recent call last):
dev-api_1           |   File "/usr/src/app/.venv-dev/lib/python3.5/site-packages/django/core/handlers/exception.py", line 41, in inner
dev-api_1           |     response = get_response(request)
dev-api_1           |   File "/usr/src/app/.venv-dev/lib/python3.5/site-packages/django/core/handlers/base.py", line 187, in _get_response
dev-api_1           |     response = self.process_exception_by_middleware(e, request)
dev-api_1           |   File "/usr/src/app/.venv-dev/lib/python3.5/site-packages/django/core/handlers/base.py", line 185, in _get_response
dev-api_1           |     response = wrapped_callback(request, *callback_args, **callback_kwargs)
dev-api_1           |   File "/usr/src/app/.venv-dev/lib/python3.5/site-packages/django/contrib/admin/options.py", line 551, in wrapper
dev-api_1           |     return self.admin_site.admin_view(view)(*args, **kwargs)
dev-api_1           |   File "/usr/src/app/.venv-dev/lib/python3.5/site-packages/django/utils/decorators.py", line 149, in _wrapped_view
dev-api_1           |     response = view_func(request, *args, **kwargs)
dev-api_1           |   File "/usr/src/app/.venv-dev/lib/python3.5/site-packages/django/views/decorators/cache.py", line 57, in _wrapped_view_func
dev-api_1           |     response = view_func(request, *args, **kwargs)
dev-api_1           |   File "/usr/src/app/.venv-dev/lib/python3.5/site-packages/django/contrib/admin/sites.py", line 224, in inner
dev-api_1           |     return view(request, *args, **kwargs)
dev-api_1           |   File "/usr/src/app/.venv-dev/lib/python3.5/site-packages/django/contrib/admin/options.py", line 1508, in add_view
dev-api_1           |     return self.changeform_view(request, None, form_url, extra_context)
dev-api_1           |   File "/usr/src/app/.venv-dev/lib/python3.5/site-packages/django/utils/decorators.py", line 67, in _wrapper
dev-api_1           |     return bound_func(*args, **kwargs)
dev-api_1           |   File "/usr/src/app/.venv-dev/lib/python3.5/site-packages/django/utils/decorators.py", line 149, in _wrapped_view
dev-api_1           |     response = view_func(request, *args, **kwargs)
dev-api_1           |   File "/usr/src/app/.venv-dev/lib/python3.5/site-packages/django/utils/decorators.py", line 63, in bound_func
dev-api_1           |     return func.__get__(self, type(self))(*args2, **kwargs2)
dev-api_1           |   File "/usr/src/app/.venv-dev/lib/python3.5/site-packages/django/contrib/admin/options.py", line 1408, in changeform_view
dev-api_1           |     return self._changeform_view(request, object_id, form_url, extra_context)
dev-api_1           |   File "/usr/src/app/.venv-dev/lib/python3.5/site-packages/django/contrib/admin/options.py", line 1440, in _changeform_view
dev-api_1           |     if form.is_valid():
dev-api_1           |   File "/usr/src/app/.venv-dev/lib/python3.5/site-packages/django/forms/forms.py", line 183, in is_valid
dev-api_1           |     return self.is_bound and not self.errors
dev-api_1           |   File "/usr/src/app/.venv-dev/lib/python3.5/site-packages/django/forms/forms.py", line 175, in errors
dev-api_1           |     self.full_clean()
dev-api_1           |   File "/usr/src/app/.venv-dev/lib/python3.5/site-packages/django/forms/forms.py", line 386, in full_clean
dev-api_1           |     self._post_clean()
dev-api_1           |   File "/usr/src/app/.venv-dev/lib/python3.5/site-packages/django/forms/models.py", line 402, in _post_clean
dev-api_1           |     self.validate_unique()
dev-api_1           |   File "/usr/src/app/.venv-dev/lib/python3.5/site-packages/django/forms/models.py", line 411, in validate_unique
dev-api_1           |     self.instance.validate_unique(exclude=exclude)
dev-api_1           |   File "/usr/src/app/.venv-dev/lib/python3.5/site-packages/django/db/models/base.py", line 1032, in validate_unique
dev-api_1           |     errors = self._perform_unique_checks(unique_checks)
dev-api_1           |   File "/usr/src/app/.venv-dev/lib/python3.5/site-packages/django/db/models/base.py", line 1129, in _perform_unique_checks
dev-api_1           |     if qs.exists():
dev-api_1           |   File "/usr/src/app/.venv-dev/lib/python3.5/site-packages/django/db/models/query.py", line 668, in exists
dev-api_1           |     return self.query.has_results(using=self.db)
dev-api_1           |   File "/usr/src/app/.venv-dev/lib/python3.5/site-packages/django/db/models/sql/query.py", line 517, in has_results
dev-api_1           |     return compiler.has_results()
dev-api_1           |   File "/usr/src/app/.venv-dev/lib/python3.5/site-packages/django/db/models/sql/compiler.py", line 845, in has_results
dev-api_1           |     return bool(self.execute_sql(SINGLE))
dev-api_1           |   File "/usr/src/app/.venv-dev/lib/python3.5/site-packages/django/db/models/sql/compiler.py", line 886, in execute_sql
dev-api_1           |     raise original_exception
dev-api_1           |   File "/usr/src/app/.venv-dev/lib/python3.5/site-packages/django/db/models/sql/compiler.py", line 876, in execute_sql
dev-api_1           |     cursor.execute(sql, params)
dev-api_1           |   File "/usr/src/app/.venv-dev/lib/python3.5/site-packages/debug_toolbar/panels/sql/tracking.py", line 165, in execute
dev-api_1           |     return self._record(self.cursor.execute, sql, params)
dev-api_1           |   File "/usr/src/app/.venv-dev/lib/python3.5/site-packages/debug_toolbar/panels/sql/tracking.py", line 107, in _record
dev-api_1           |     return method(sql, params)
dev-api_1           |   File "/usr/src/app/.venv-dev/lib/python3.5/site-packages/django/db/backends/utils.py", line 80, in execute
dev-api_1           |     return super(CursorDebugWrapper, self).execute(sql, params)
dev-api_1           |   File "/usr/src/app/.venv-dev/lib/python3.5/site-packages/django/db/backends/utils.py", line 65, in execute
dev-api_1           |     return self.cursor.execute(sql, params)
dev-api_1           | ValueError: A string literal cannot contain NUL (0x00) characters.

Change History (20)

comment:1 Changed 15 months ago by Tim Graham

Component: contrib.adminDatabase layer (models, ORM)
Severity: NormalRelease blocker
Summary: 500 when using a null character, Postgres + Python 3.5.3Saving a Char/TextField with psycopg2 2.7+ raises ValueError: A string literal cannot contain NUL (0x00) characters is unhandled
Triage Stage: UnreviewedAccepted

This exception was introduced in psycopg2 2.7+. With psycopg2 < 2.7, there is no exception and null bytes are silently truncated by PostgreSQL. Other databases that I tested (SQLite, MySQL, Oracle) allow saving null bytes. This creates possible cross-database compatibility problems when moving data from those databases to PostgreSQL, e.g.#28117. I've tentatively closed that ticket as a duplicate but will reopen it if the solution for this ticket doesn't address it.

I propose having CharField and TextField strip null bytes from the value either a) only on PostgreSQL or b) on all databases. I raised a django-developers thread to ask for feedback.

comment:2 Changed 15 months ago by Biswajit Sahu

Owner: changed from nobody to Biswajit Sahu
Status: newassigned

comment:3 Changed 15 months ago by Tim Graham

Owner: Biswajit Sahu deleted
Status: assignednew

Please don't work on this issue until there's a consensus about the solution.

comment:4 Changed 15 months ago by Tim Graham

Component: Database layer (models, ORM)Forms
Severity: Release blockerNormal
Summary: Saving a Char/TextField with psycopg2 2.7+ raises ValueError: A string literal cannot contain NUL (0x00) characters is unhandledMake CharField/TextField prohibit null characters
Type: BugCleanup/optimization

The consensus on the mailing list is to make CharField and TextField prohibit null characters.

comment:5 Changed 15 months ago by Rajesh Veeranki

So the way to achieve this is by adding a default validator to CharField and TextField and raise a validation error. Am I right?

comment:6 Changed 15 months ago by Tim Graham

That seems reasonable.

comment:7 Changed 14 months ago by Alejandro Zamora Fonseca

Owner: set to Alejandro Zamora Fonseca
Status: newassigned

comment:8 Changed 14 months ago by Alejandro Zamora Fonseca

Added default null characters validation to CharField and TextField in
order to avoid raising errors on database layer. Added test for both
fields. Tested on PostgreSQL, MySQL, and SQLite backends.

comment:9 Changed 14 months ago by Alejandro Zamora Fonseca

Linked PR against master branch PR

Last edited 14 months ago by Claude Paroz (previous) (diff)

comment:10 Changed 14 months ago by Claude Paroz

Has patch: set
Patch needs improvement: set

comment:11 Changed 14 months ago by Alejandro Zamora Fonseca

Maybe only adding the null char validator to CharField would be enough?
Because TextField is converted to CharField when Model to Field translation occurs.

comment:12 Changed 14 months ago by Tim Graham

Summary: Make CharField/TextField prohibit null charactersMake CharField form field prohibit null characters

Correct, there's isn't a TextField form field.

comment:13 Changed 14 months ago by Claude Paroz

Patch needs improvement: unset

comment:14 Changed 14 months ago by Tim Graham

Patch needs improvement: set

comment:15 Changed 14 months ago by Alejandro Zamora Fonseca

I added documentation and unit tests for the validator.

comment:16 Changed 14 months ago by Alejandro Zamora Fonseca

Patch needs improvement: unset

comment:17 Changed 13 months ago by Alejandro Zamora Fonseca

Triage Stage: AcceptedReady for checkin

comment:18 Changed 12 months ago by Tim Graham <timograham@…>

Resolution: fixed
Status: assignedclosed

In 90d7b91:

Fixed #28201 -- Added ProhibitNullCharactersValidator and used it on CharField form field.

comment:19 Changed 5 months ago by Vlada Macek

Cc: Vlada Macek added

We just ran into this on 1.11 LTS just as the original author. May I ask whether it is usual to backport fixes like this?

Thank you.

comment:20 Changed 5 months ago by Tim Graham

Per our supported versions policy, 1.11 is only receiving data loss and security fixes.

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