HttpResponse and non-string content values
Paul McMillan
In short, the issue here is that the existing code for setting the
content of an HTTPResponse behaves inconsistently when given
non-strings. Setting content during __init__ behaves differently for
iterators than setting it later via the content property. If a
non-string, non-iterator is set for content, the code eventually
throws an error during string operations. If content is an iterator,
it is iterated over and (crucially) the content is converted to a
string.
My initial response was to fix the logic error for non-strings and
return an explicit error stating that this function requires a string
for content. However, since the existing iterator code does convert to
a string, the most consistent thing we can do here seems to be
converting everything (including non-iterators) right before output.
I added some test cases and cleaned the code up a bit. I believe sure
this solution is backwards compatible (we're adding the string
conversion, something that shouldn't have been possible before), but
I'd like input from some other people.
-Paul
Anssi Kääriäinen
> I added some test cases and cleaned the code up a bit. I believe sure
> this solution is backwards compatible (we're adding the string
> conversion, something that shouldn't have been possible before), but
> I'd like input from some other people.
>
> -Paul
>
> [1]https://code.djangoproject.com/ticket/16494
problem is that I don't think the current implementation is correct.
It consumes the iterator multiple times. See the comments in the
ticket for details.
- Anssi
Paul McMillan
consuming the iterator multiple times is tracked in #7581. I don't
think that issue needs to be a blocker for this one, since it has
existed for quite some time now.
https://code.djangoproject.com/ticket/7581
-Paul
> --
> You received this message because you are subscribed to the Google Groups "Django developers" group.
> To post to this group, send email to django-d...@googlegroups.com.
> To unsubscribe from this group, send email to django-develop...@googlegroups.com.
> For more options, visit this group at http://groups.google.com/group/django-developers?hl=en.
>
>
Jacob Kaplan-Moss
> I added some test cases and cleaned the code up a bit. I believe sure
> this solution is backwards compatible (we're adding the string
> conversion, something that shouldn't have been possible before), but
> I'd like input from some other people.
Looks right to me. I tried to come up with corner cases that wouldn't
be backwards-compatible, but couldn't. Even if someone's got one, the
previous behavior was arguably a bug anyway: a property set in
__init__ shouldn't behave differently than one set later.
Jacob
Anssi Kääriäinen
> I replied on the ticket, but for the record here as well, the issue of
> consuming the iterator multiple times is tracked in #7581. I don't
> think that issue needs to be a blocker for this one, since it has
> existed for quite some time now.
>
> https://code.djangoproject.com/ticket/7581
state of response.content (you change it by measuring it) is a bit
surprising to say the least. For example, try this in IPython:
>>> r = HttpResponse(iter(['a', 'b']))
>>> r.content
''
>>> r2 = HttpResponse(iter(['a', 'b']))
>>> print r2.content
'ab'
>>> r3 = HttpResponse(iter(['a', 'b']))
>>> r3.content == r3.content
False
That might be breaking some Python conventions.
Currently, a third party Middleware writer has zero chance to get the
logic right on first try if he doesn't know about this gotcha. If he
needs the content of a response, he will supposedly use the
response.content attribute. And not test what happens when the content
is an iterator. There is no mention of this issue in "writing your own
middleware" section of Django documentation.
- Anssi