[Django] #20889: HttpResponseBase._convert_to_charset complains about newline it inserted itself
Django
itself
-------------------------------+--------------------
Reporter: mjl@… | Owner: nobody
Type: Uncategorized | Status: new
Component: HTTP handling | Version: 1.5
Severity: Normal | Keywords:
Triage Stage: Unreviewed | Has patch: 1
Easy pickings: 1 | UI/UX: 0
-------------------------------+--------------------
Consider the following snippet:
{{{
# coding=utf-8
from django.http import HttpResponse
h = HttpResponse()
f = 'zzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzz a\xcc\x88'.decode('utf-8')
h['Content-Disposition'] = u'attachment; filename="%s"' % f
}}}
This contains a "OSX-Style" Umlaut (a + ¨), which triggers the "convert to
mime encoding" path of _convert_to_charset. However, this string is so
long that email.header.Header, which is used to mime-encode, will split
the line. Later _convert_to_charset explodes because the header now
contains a newline.
{{{
$ ./manage.py shell
Python 2.7.5 (default, Aug 1 2013, 01:01:17)
[GCC 4.2.1 Compatible Apple Clang 4.1 ((tags/Apple/clang-421.11.66))] on
darwin
Type "help", "copyright", "credits" or "license" for more information.
(InteractiveConsole)
>>> import ttt
Traceback (most recent call last):
File "<console>", line 1, in <module>
File "/Users/mjl/Projects/cms_sites/cc.intranet/cms/ttt.py", line 5, in
<module>
h['Content-Disposition'] = u'attachment; filename="%s"' % f
File "/Users/mjl/Projects/cms_sites/cc.intranet/lib/python2.7/site-
packages/django/http/response.py", line 110, in __setitem__
value = self._convert_to_charset(value, 'latin1', mime_encode=True)
File "/Users/mjl/Projects/cms_sites/cc.intranet/lib/python2.7/site-
packages/django/http/response.py", line 105, in _convert_to_charset
raise BadHeaderError("Header values can't contain newlines (got %r)" %
value)
BadHeaderError: Header values can't contain newlines (got
'=?utf-8?q?attachment=3B_filename=3D=22zzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzz_a?=\n
=?utf-8?b?zIgi?=')
}}}
Solution: django/http/response.py#L163 should probably use
{{{
Header(value, 'utf-8', maxlinelen=1000)
}}}
to avoid generating continuation lines. Agreed, the "1000" is somewhat
arbitrary, but realistically this fixes the problem.
--
Ticket URL: <https://code.djangoproject.com/ticket/20889>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.
Django
itself
Reporter: mjl@… | Owner: animan1
Type: Uncategorized | Status: assigned
Component: HTTP handling | Version: 1.5
Keywords: | Triage Stage: Unreviewed
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 1 | UI/UX: 0
Changes (by animan1):
* status: new => assigned
* needs_better_patch: => 0
* owner: nobody => animan1
* needs_tests: => 0
* needs_docs: => 0
--
Ticket URL: <https://code.djangoproject.com/ticket/20889#comment:1>
Django
itself
Reporter: mjl@… | Owner: animan1
Component: HTTP handling | Version: 1.5
Keywords: | Triage Stage: Unreviewed
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Changes (by Tim Graham <timograham@…>):
* status: assigned => closed
* resolution: => fixed
Comment:
In [changeset:"6dca603abb0eb164ba87657caf5cc65bca449719"]:
{{{
#!CommitTicketReference repository=""
revision="6dca603abb0eb164ba87657caf5cc65bca449719"
Fixed #20889 -- Prevented email.Header from inserting newlines
Passed large maxlinelen to email.Header to prevent newlines from being
inserted into value returned by _convert_to_charset
Thanks mjl at laubach.at for the report.
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/20889#comment:2>
Django
itself
Reporter: mjl@… | Owner: animan1
Type: Uncategorized | Status: closed
Keywords: | Triage Stage: Unreviewed
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Comment (by Tim Graham <timograham@…>):
In [changeset:"789d8f07487f80c5b90ec9cbb1fcf99837ef3ace"]:
{{{
#!CommitTicketReference repository=""
revision="789d8f07487f80c5b90ec9cbb1fcf99837ef3ace"
Fixed syntax error on Python 3.2; refs #20889.
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/20889#comment:3>
Django
itself
Reporter: mjl@… | Owner: animan1
Component: HTTP handling | Version: 1.5
Keywords: | Triage Stage: Unreviewed
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Changes (by poly):
* cc: poly (added)
* status: closed => new
* resolution: fixed =>
Comment:
I'm afraid this annoying bug hasn't gone, see my report on #21620.
--
Ticket URL: <https://code.djangoproject.com/ticket/20889#comment:4>
Django
itself
Reporter: mjl@… | Owner: rednaw
Type: Uncategorized | Status: assigned
Component: HTTP handling | Version: 1.5
Keywords: | Triage Stage: Unreviewed
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Changes (by rednaw):
* status: new => assigned
* owner: animan1 => rednaw
--
Ticket URL: <https://code.djangoproject.com/ticket/20889#comment:5>
Django
itself
Reporter: mjl@… | Owner: rednaw
Type: Uncategorized | Status: assigned
Keywords: nlsprint14 | Triage Stage: Unreviewed
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Changes (by rednaw):
* keywords: => nlsprint14
--
Ticket URL: <https://code.djangoproject.com/ticket/20889#comment:6>
Django
itself
Reporter: mjl@… | Owner: rednaw
Type: Uncategorized | Status: assigned
Keywords: nlsprint14 | Triage Stage: Unreviewed
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Comment (by rednaw):
The bug you are encountering poly, is (I think) because of a bug in the
Python standard library that has never been solved.
The bug was reported over here:
http://bugs.python.org/issue12649
However, it was closed for some reason. I can still reproduce the bug with
this snippet:
{{{
from email.header import Header
import sys
print Header(
u'attachment; filename="ajdsklfj klasdjfkl asdjfkl jadsfja sdflkads
fad fads adsf dasjfkl jadslkfj dlasf asd \u6211\u6211\u6211 jo
\u6211\u6211 jo \u6211\u6211"',
charset='utf-8',
maxlinelen=sys.maxsize
).encode()
}}}
which output is:
{{{
=?utf-8?b?YXR0YWNobWVudDsgZmlsZW5hbWU9ImFqZHNrbGZqIGtsYXNkamZrbCBhc2RqZmts?=
=?utf-8?b?IGphZHNmamEgc2RmbGthZHMgZmFkIGZhZHMgYWRzZiBkYXNqZmtsIGphZHNsa2Zq?=
=?utf-8?b?IGRsYXNmIGFzZCDmiJHmiJHmiJEgam8g5oiR5oiRIGpvIOaIkeaIkSI=?=
}}}
So it's because there's one character in the header that can't be encoded
to `latin-1` which raises a `UnicodeError`, which will be catched and will
be encoded using the `Header` class from the python standard library in
`email.header`.
So this happens over here:
https://github.com/django/django/blob/master/django/http/response.py#L169
Well, it correctly gives the `maxlinelen`. Now, what goes wrong in there?
If you dive in to the code, I tackled it down to the `_encode_chunks` the
`Header` class. This method takes the `maxlinelen` argument (which is
ultimately based on the value initially provided in the init method in the
code I posted above). However, it doesn't give it to the `header_encode`
method that is used in this piece of code in this `_encode_chunks` method:
{{{
if charset is None or charset.header_encoding is None:
s = header
else:
s = charset.header_encode(header)
}}}
So if `charset.header_encoding` is set it uses this method to encode the
header, but then the `maxlinelen` is lost. This is where it goes wrong.
I'll keep this ticket assigned to me as I'm probably going on with it
tomorrow.
--
Ticket URL: <https://code.djangoproject.com/ticket/20889#comment:7>
Django
itself
Reporter: mjl@… | Owner: rednaw
Type: Uncategorized | Status: assigned
Keywords: nlsprint14 | Triage Stage: Unreviewed
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Comment (by rednaw):
I just created a ticket in the Python bugtracker about this:
http://bugs.python.org/issue20747
--
Ticket URL: <https://code.djangoproject.com/ticket/20889#comment:8>
Django
itself
Reporter: mjl@… | Owner: rednaw
Type: Uncategorized | Status: assigned
Keywords: nlsprint14 | Triage Stage: Unreviewed
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
-------------------------------+--------------------------------------
Changes (by rednaw):
* easy: 1 => 0
--
Ticket URL: <https://code.djangoproject.com/ticket/20889#comment:9>
Django
itself
Reporter: mjl@… | Owner: rednaw
Type: Uncategorized | Status: assigned
Keywords: nlsprint14 | Triage Stage: Unreviewed
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------+--------------------------------------
Comment (by mjl):
Actually, I'm not sure that the check for newline is actually correct.
There is no problem with a newline in a header '''as long as it's followed
by white space''', which means it's a continuation line and will be folded
back at the other end.
I understand where that check comes from though, there have been too many
header injections out there.
--
Ticket URL: <https://code.djangoproject.com/ticket/20889#comment:10>
Django
itself
Reporter: mjl@… | Owner: rednaw
Component: HTTP handling | Version: 1.7-beta-2
Severity: Normal | Resolution:
Keywords: nlsprint14 | Triage Stage: Unreviewed
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------+--------------------------------------
* has_patch: 1 => 0
* version: 1.5 => 1.7-beta-2
* type: Uncategorized => Bug
Comment:
Is there any harm in leaving the original fix in 1.7? Should we leave this
ticket open or is it something that should by solved in Python itself?
--
Ticket URL: <https://code.djangoproject.com/ticket/20889#comment:11>
Django
itself
Reporter: mjl@… | Owner: rednaw
Component: HTTP handling | Version: 1.7-beta-2
Keywords: nlsprint14 | Triage Stage: Unreviewed
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------+--------------------------------------
Changes (by timo):
* status: assigned => closed
* resolution: => needsinfo
Comment:
Closing as needsinfo as it's not clear to me what we can do in Django to
solve this issue and I haven't been able to get anyway to follow up in 3
months.
--
Ticket URL: <https://code.djangoproject.com/ticket/20889#comment:12>
Django
itself
Reporter: mjl@… | Owner: rednaw
Type: Bug | Status: closed
Component: HTTP handling | Version: 1.7-beta-2
Severity: Normal | Resolution: needsinfo
Keywords: nlsprint14 | Triage Stage: Unreviewed
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------+--------------------------------------
* cc: gitaarik@… (added)
--
Ticket URL: <https://code.djangoproject.com/ticket/20889#comment:13>
Django
itself
Reporter: mjl@… | Owner: rednaw
Type: Bug | Status: new
Component: HTTP handling | Version: master
Severity: Normal | Resolution:
Keywords: nlsprint14 | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------+------------------------------------
* status: closed => new
* has_patch: 0 => 1
* version: 1.7-beta-2 => master
* resolution: needsinfo =>
* stage: Unreviewed => Accepted
Comment:
I've encountered this bug on a production site. I'm suggesting to check
for newlines in user-provided values, before formatting the header through
Python API to workaround this issue.
https://github.com/django/django/pull/4163
--
Ticket URL: <https://code.djangoproject.com/ticket/20889#comment:14>
Django
itself
Reporter: mjl@… | Owner: rednaw
Type: Bug | Status: new
Component: HTTP handling | Version: master
Severity: Normal | Resolution:
| checkin
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Changes (by timgraham):
* stage: Accepted => Ready for checkin
--
Ticket URL: <https://code.djangoproject.com/ticket/20889#comment:15>
Django
itself
Reporter: mjl@… | Owner: rednaw
Component: HTTP handling | Version: master
Keywords: nlsprint14 | Triage Stage: Ready for
| checkin
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
* status: new => closed
* resolution: => fixed
Comment:
In [changeset:"efb1f99f943b99624c78b91ff58502125490191e"]:
{{{
#!CommitTicketReference repository=""
revision="efb1f99f943b99624c78b91ff58502125490191e"
Fixed #20889 -- Prevented BadHeaderError when Python inserts newline
Workaround for http://bugs.python.org/issue20747.
In some corner cases, Python 2 inserts a newline in a header value
despite `maxlinelen` passed in Header constructor.
Thanks Tim Graham for the review.
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/20889#comment:16>
Django
itself
Reporter: mjl@… | Owner: rednaw
Type: Bug | Status: closed
Component: HTTP handling | Version: master
Severity: Normal | Resolution: fixed
Keywords: nlsprint14 | Triage Stage: Ready for
| checkin
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Comment (by Claude Paroz <claude@…>):
In [changeset:"84e7fec88ddfc4178500a80d640728226d77317a"]:
{{{
#!CommitTicketReference repository=""
revision="84e7fec88ddfc4178500a80d640728226d77317a"
[1.8.x] Fixed #20889 -- Prevented BadHeaderError when Python inserts
newline
Workaround for http://bugs.python.org/issue20747.
In some corner cases, Python 2 inserts a newline in a header value
despite `maxlinelen` passed in Header constructor.
Thanks Tim Graham for the review.
Backport of efb1f99f94 from master.
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/20889#comment:17>