[Django] #30294: HttpResponse doesn't handle memoryview objects
Django
-----------------------------------------+------------------------
Reporter: rhunwicks | Owner: nobody
Type: Bug | Status: new
Component: HTTP handling | Version: master
Severity: Normal | Keywords:
Triage Stage: Unreviewed | Has patch: 0
Needs documentation: 0 | Needs tests: 0
Patch needs improvement: 0 | Easy pickings: 1
UI/UX: 0 |
-----------------------------------------+------------------------
I am trying to write a `BinaryField` retrieved from the database into a
`HttpResponse`. When the database is Sqlite this works correctly, but
Postgresql returns the contents of the field as a `memoryview` object and
it seems like current Django doesn't like this combination:
{{{
#!python
from django.http import HttpResponse
# String content
response = HttpResponse("My Content")
response.content
# Out: b'My Content'
# This is correct
# Bytes content
response = HttpResponse(b"My Content")
response.content
# Out: b'My Content'
# This is also correct
# memoryview content
response = HttpResponse(memoryview(b"My Content"))
response.content
# Out: b'<memory at 0x7fcc47ab2648>'
# This is not correct, I am expecting b'My Content'
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/30294>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.
Django
Reporter: Roger Hunwicks | Owner: nobody
Type: Bug | Status: new
Component: HTTP handling | Version: master
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 1 | UI/UX: 0
Changes (by Simon Charette):
* stage: Unreviewed => Accepted
Comment:
I guess `HttpResponseBase.make_bytes`
[https://github.com/django/django/blob/9caa3e0a29c54939cc923c1693f491ff6c8e77e2/django/http/response.py#L232-L233
could be adapted] to deal with `memoryview` objects by casting them to
`bytes`.
In all cases simply wrapping the `memoryview` in `bytes` works as a
workaround `HttpResponse(bytes(model.binary_field))`.
--
Ticket URL: <https://code.djangoproject.com/ticket/30294#comment:1>
Django
Reporter: Roger Hunwicks | Owner: nobody
Component: HTTP handling | Version: master
Keywords: | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 1 | UI/UX: 0
Changes (by Simon Charette):
* type: Bug => New feature
--
Ticket URL: <https://code.djangoproject.com/ticket/30294#comment:2>
Django
Reporter: Roger Hunwicks | Owner: nobody
Component: HTTP handling | Version: master
Keywords: | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 1 | UI/UX: 0
Comment (by Simon Charette):
The fact `make_bytes` would still use `force_bytes` if
da56e1bac6449daef9aeab8d076d2594d9fd5b44 didn't refactor it and that
d680a3f4477056c69629b0421db4bb254b8c69d0 added `memoryview` support to
`force_bytes` strengthen my assumption that `make_bytes` should be
adjusted as well.
--
Ticket URL: <https://code.djangoproject.com/ticket/30294#comment:3>
Django
Reporter: Roger Hunwicks | Owner: sage
Type: New feature | Status: assigned
Component: HTTP handling | Version: master
Keywords: | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 1 | UI/UX: 0
Changes (by sage):
* status: new => assigned
* owner: nobody => sage
Comment:
I'll try to work on this.
--
Ticket URL: <https://code.djangoproject.com/ticket/30294#comment:4>
Django
Reporter: Roger Hunwicks | Owner: sage
Type: New feature | Status: assigned
Keywords: | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 1 | UI/UX: 0
Comment (by sage):
Does
[https://github.com/laymonage/django/commit/6f57880bb3f4b1deaad5dc0949b1369a006f51e4
this] look good?
--
Ticket URL: <https://code.djangoproject.com/ticket/30294#comment:5>
Django
Reporter: Roger Hunwicks | Owner: sage
Type: New feature | Status: assigned
Keywords: | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 1 | UI/UX: 0
Changes (by felixxm):
* has_patch: 0 => 1
Comment:
[https://github.com/django/django/pull/11133 PR]
--
Ticket URL: <https://code.djangoproject.com/ticket/30294#comment:6>
Django
Reporter: Roger Hunwicks | Owner: sage
Type: New feature | Status: assigned
Keywords: | Triage Stage: Accepted
Needs tests: 1 | Patch needs improvement: 0
Easy pickings: 1 | UI/UX: 0
Changes (by Carlton Gibson):
* needs_tests: 0 => 1
--
Ticket URL: <https://code.djangoproject.com/ticket/30294#comment:7>
Django
Reporter: Roger Hunwicks | Owner: sage
Type: New feature | Status: assigned
Keywords: | Triage Stage: Ready for
| checkin
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 1 | UI/UX: 0
Changes (by Carlton Gibson):
* needs_tests: 1 => 0
* stage: Accepted => Ready for checkin
Comment:
I've just asked for the test case to be moved (from `tests/responses` to
`tests/httpwrappers`, since that looks like the more suitable home) but
assuming that's in place this looks ready to go.
So, question: are we sure this is a New Feature, rather than a Bug fix?
(Just thinking re release notes and/or backports)
--
Ticket URL: <https://code.djangoproject.com/ticket/30294#comment:8>
Django
Reporter: Roger Hunwicks | Owner: sage
Type: New feature | Status: assigned
Keywords: | Triage Stage: Ready for
| checkin
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 1 | UI/UX: 0
Comment (by Roger Hunwicks):
Personally, I think it is a Bug Fix because this used to work on Django
1.8 and Python 2.7 and it no longer works on Django 2.2 and Python 3.x.
The difference is that in Python 2.7 `BinaryField` returned a `buffer`
which was handled correctly, but on Python 3.6 it returns a `memoryview`.
I think this change is because Psycopg2 returns different on objects on
Py2 vs Py3 and `BinaryField` just passes them on. It is not Django's fault
that the Py2 vs Py3 differences in the upstream library caused a
regression, but it still looks like a regression that Django code that
used to work (`BinaryField` -> `HttpResponse`) no longer does.
--
Ticket URL: <https://code.djangoproject.com/ticket/30294#comment:9>
Django
Reporter: Roger Hunwicks | Owner: sage
Type: New feature | Status: assigned
Keywords: | Triage Stage: Ready for
| checkin
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 1 | UI/UX: 0
Comment (by Simon Charette):
`BinaryField` was introduced in 1.8 and has been returning `memoryview` on
Python 3 `psycopg2` since then so this isn't a regression nor a bug in a
newly introduced feature; this has been happening since the feature was
introduced.
I considered this was a ''new feature'' because `HttpResponse` never
handled `memoryview` objects in the first place. Interactions with
`BinaryField` is red herring here IMO.
--
Ticket URL: <https://code.djangoproject.com/ticket/30294#comment:10>
Django
Reporter: Roger Hunwicks | Owner: sage
Type: New feature | Status: assigned
Keywords: | Triage Stage: Ready for
| checkin
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 1 | UI/UX: 0
Comment (by Carlton Gibson):
OK, thanks for the follow-up. I think Simon is correct. We'll need a
release note... will comment on the PR.
--
Ticket URL: <https://code.djangoproject.com/ticket/30294#comment:11>
Django
Reporter: Roger Hunwicks | Owner: sage
Component: HTTP handling | Version: master
Keywords: | Triage Stage: Ready for
| checkin
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 1 | UI/UX: 0
Changes (by Tim Graham <timograham@…>):
* status: assigned => closed
* resolution: => fixed
Comment:
In [changeset:"9aa56cb0d5dede7fc176a46c745dfd3dacdad773" 9aa56cb]:
{{{
#!CommitTicketReference repository=""
revision="9aa56cb0d5dede7fc176a46c745dfd3dacdad773"
Fixed #30294 -- Allowed HttpResponse to accept memoryview content.
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/30294#comment:12>
Django
Reporter: Roger Hunwicks | Owner: sage
Type: New feature | Status: closed
Keywords: | Triage Stage: Ready for
| checkin
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 1 | UI/UX: 0
Comment (by Carlton Gibson <carlton@…>):
In [changeset:"1fd9b44a6b1bde29fb1f6746e007787505608974" 1fd9b44]:
{{{
#!CommitTicketReference repository=""
revision="1fd9b44a6b1bde29fb1f6746e007787505608974"
Refs #32074 -- Fixed handling memoryview content by HttpResponse on Python
3.10+.
An iterator was added to memoryview in Python 3.10,
see https://bugs.python.org/issue41732
Refs #30294
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/30294#comment:13>