[Django] #33567: Builtin csrf_failure() view uses wrong charset
Django
--------------------------------------+------------------------
Reporter: MrVichr | Owner: nobody
Type: Bug | Status: new
Component: CSRF | Version: 4.0
Severity: Normal | Keywords: csrf
Triage Stage: Unreviewed | Has patch: 0
Needs documentation: 0 | Needs tests: 0
Patch needs improvement: 0 | Easy pickings: 1
UI/UX: 0 |
--------------------------------------+------------------------
When Django detects wrong CSRF token, it shows an error using view
django.views.csrf.csrf_failure. That file ends with
{{{
return HttpResponseForbidden(t.render(c), content_type="text/html;")
}}}
When the template (CSRF_FAILURE_TEMPLATE_NAME) is written using '''UTF-8,
it is rendered incorrectly'''. I suggest changing that line to
{{{
return HttpResponseForbidden(t.render(c), content_type="text/html;"+
f"
charset={settings.DEFAULT_CHARSET};")
}}}
or perhaps leaving out the `content_type` entirely.
Currently I'm using a workaround, by adding
{{{
<meta http-equiv="Content-type" content="text/html; charset=utf-8" />
}}}
to the template's HEAD, but it seems to me that the suggested fix is a
better solution.
--
Ticket URL: <https://code.djangoproject.com/ticket/33567>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.
Django
Reporter: MrVichr | Owner: nobody
Type: Bug | Status: closed
Component: CSRF | Version: 4.0
Severity: Normal | Resolution: needsinfo
Keywords: csrf | Triage Stage: Unreviewed
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 1 | UI/UX: 0
Changes (by Carlton Gibson):
* status: new => closed
* resolution: => needsinfo
Comment:
Hi. Can you provide a working example of the issue please, and exactly
what you mean by " it is rendered incorrectly".
Creating a test project with a UTF8 403_csrf.html works as expected, so
I'm guessing there's something else going on.
Thanks.
--
Ticket URL: <https://code.djangoproject.com/ticket/33567#comment:1>
Django
-------------------------+--------------------------------------
Reporter: MrVichr | Owner: nobody
Type: Bug | Status: closed
Component: CSRF | Version: 4.0
Severity: Normal | Resolution: needsinfo
Keywords: csrf | Triage Stage: Unreviewed
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 1 | UI/UX: 0
-------------------------+--------------------------------------
Comment (by Claude Paroz):
I think I'd rather omit `content_type` when it is `text/html` (several
other locations are affected, too). I'll try a patch to see what's the
outcome in tests.
--
Ticket URL: <https://code.djangoproject.com/ticket/33567#comment:2>
Django
-------------------------+--------------------------------------
Reporter: MrVichr | Owner: nobody
Type: Bug | Status: closed
Component: CSRF | Version: 4.0
Severity: Normal | Resolution: needsinfo
Keywords: csrf | Triage Stage: Unreviewed
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 1 | UI/UX: 0
-------------------------+--------------------------------------
Comment (by Claude Paroz):
Here's the [https://github.com/django/django/pull/15491 PR].
--
Ticket URL: <https://code.djangoproject.com/ticket/33567#comment:3>
Django
-------------------------+--------------------------------------
Reporter: MrVichr | Owner: nobody
Type: Bug | Status: closed
Component: CSRF | Version: 4.0
Severity: Normal | Resolution: needsinfo
Keywords: csrf | Triage Stage: Unreviewed
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 1 | UI/UX: 0
-------------------------+--------------------------------------
Comment (by Carlton Gibson):
Hey Claude,
Yes, I took it out in that one place, with no effect.
But, if there is an error, it would be good to see how that could come up.
(It should be a no-op right? 🤔)
--
Ticket URL: <https://code.djangoproject.com/ticket/33567#comment:4>
Django
-------------------------+--------------------------------------
Reporter: MrVichr | Owner: nobody
Type: Bug | Status: closed
Component: CSRF | Version: 4.0
Severity: Normal | Resolution: needsinfo
Keywords: csrf | Triage Stage: Unreviewed
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 1 | UI/UX: 0
-------------------------+--------------------------------------
Comment (by Claude Paroz):
When the charset is missing from a response, the browser has to guess
(either using a default, a user preference, or sniffing the content). We
should avoid that. Read https://www.w3.org/International/articles/http-
charset/index.en
--
Ticket URL: <https://code.djangoproject.com/ticket/33567#comment:5>
Django
-------------------------+------------------------------------
Type: Bug | Status: new
Component: CSRF | Version: 4.0
Severity: Normal | Resolution:
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 1 | UI/UX: 0
-------------------------+------------------------------------
* status: closed => new
* has_patch: 0 => 1
* resolution: needsinfo =>
* stage: Unreviewed => Accepted
Comment:
Good link!
> It is very important to always label Web documents explicitly. HTTP 1.1
says that the default charset is ISO-8859-1. ...
So we can presume the doc is interpreted as ISO-8859-1, rather than UTF-8,
and so an error (although exact example...)
{{{
>>> r = HttpResponse("Hello 🎉", content_type="text/html")
>>> r.charset
'utf-8'
>>> r.serialize_headers()
b'Content-Type: text/html'
>>> r = HttpResponse("Hello 🎉")
>>> r.serialize_headers()
b'Content-Type: text/html; charset=utf-8'
}}}
Claude's, of removing the half-specified `content_type` automatically
picks up MrVichr's idea to add the explicit `charset`.
--
Ticket URL: <https://code.djangoproject.com/ticket/33567#comment:6>
Django
Reporter: MrVichr | Owner: Claude Paroz
Type: Bug | Status: assigned
Component: CSRF | Version: 4.0
Severity: Normal | Resolution:
Keywords: csrf | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 1 | UI/UX: 0
Changes (by Carlton Gibson):
* owner: nobody => Claude Paroz
* status: new => assigned
--
Ticket URL: <https://code.djangoproject.com/ticket/33567#comment:7>
Django
Reporter: MrVichr | Owner: Claude Paroz
Type: Bug | Status: assigned
Component: CSRF | Version: 4.0
Severity: Normal | Resolution:
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 1 | UI/UX: 0
Changes (by Carlton Gibson):
* stage: Accepted => Ready for checkin
--
Ticket URL: <https://code.djangoproject.com/ticket/33567#comment:8>
Django
-------------------------+---------------------------------------------
Reporter: MrVichr | Owner: Claude Paroz
Component: CSRF | Version: 4.0
Severity: Normal | Resolution: fixed
Keywords: csrf | Triage Stage: Ready for checkin
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 1 | UI/UX: 0
-------------------------+---------------------------------------------
* status: assigned => closed
* resolution: => fixed
Comment:
In [changeset:"93803a1b5f4a08eb064b4cc8b3834ff323be4065" 93803a1]:
{{{
#!CommitTicketReference repository=""
revision="93803a1b5f4a08eb064b4cc8b3834ff323be4065"
Fixed #33567 -- Avoided setting default text/html content type on
responses.
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/33567#comment:9>