Opened 3 years ago

Closed 3 years ago

Last modified 22 months 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 Changed 3 years ago by Simon Charette

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 Changed 3 years ago by Simon Charette

Type: BugNew feature

comment:3 Changed 3 years ago 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.

comment:4 Changed 3 years ago by Sage Abdullah

Owner: changed from nobody to Sage Abdullah
Status: newassigned

I'll try to work on this.

comment:5 Changed 3 years ago by Sage Abdullah

Does this look good?

comment:6 Changed 3 years ago by Mariusz Felisiak

Has patch: set

comment:7 Changed 3 years ago by Carlton Gibson

Needs tests: set

comment:8 Changed 3 years ago by Carlton Gibson

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 Changed 3 years ago 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.

comment:10 Changed 3 years ago 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. Required adjustments to string handling when moving a code base from Python 2 to 3 should be expected.

Last edited 3 years ago by Simon Charette (previous) (diff)

comment:11 Changed 3 years ago 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.

comment:12 Changed 3 years ago by Tim Graham <timograham@…>

Resolution: fixed
Status: assignedclosed

In 9aa56cb:

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

comment:13 Changed 22 months ago by Carlton Gibson <carlton@…>

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