Opened 5 years ago

Closed 5 years ago

Last modified 3 years ago

#30294 closed New feature (fixed)

HttpResponse doesn't handle memoryview objects

Reported by: Roger Hunwicks Owned by: Sage Abdullah
Component: HTTP handling Version: dev
Severity: Normal Keywords:
Cc: Triage Stage: Ready for checkin
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: yes UI/UX: no

Description

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:

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'

Change History (13)

comment:1 by Simon Charette, 5 years ago

Triage Stage: UnreviewedAccepted

I guess HttpResponseBase.make_bytes 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)).

comment:2 by Simon Charette, 5 years ago

Type: BugNew feature

comment:3 by Simon Charette, 5 years ago

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.

comment:4 by Sage Abdullah, 5 years ago

Owner: changed from nobody to Sage Abdullah
Status: newassigned

I'll try to work on this.

comment:5 by Sage Abdullah, 5 years ago

Does this look good?

comment:6 by Mariusz Felisiak, 5 years ago

Has patch: set

comment:7 by Carlton Gibson, 5 years ago

Needs tests: set

comment:8 by Carlton Gibson, 5 years ago

Needs tests: unset
Triage Stage: AcceptedReady for checkin

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)

comment:9 by Roger Hunwicks, 5 years ago

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.

comment:10 by Simon Charette, 5 years ago

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.

Version 0, edited 5 years ago by Simon Charette (next)

comment:11 by Carlton Gibson, 5 years ago

OK, thanks for the follow-up. I think Simon is correct. We'll need a release note... will comment on the PR.

comment:12 by Tim Graham <timograham@…>, 5 years ago

Resolution: fixed
Status: assignedclosed

In 9aa56cb:

Fixed #30294 -- Allowed HttpResponse to accept memoryview content.

comment:13 by Carlton Gibson <carlton@…>, 3 years ago

In 1fd9b44:

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

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