Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Import force_bytes on django >= 1.5 #1377

Merged
merged 3 commits into from
Jan 31, 2014
Merged

Conversation

LilyFoote
Copy link
Contributor

Testing file uploads with the APIRequestFactory fails with a django.utils.encoding.DjangoUnicodeDecodeError on python 3 with django 1.6.

I've fixed this by importing the correct function (force_bytes) from django >= 1.5.

@tomchristie
Copy link
Member

Thanks @Ian-Foote - Any chance we could get a test case that validates the file upload behaviour rather than the import behaviour?

@LilyFoote
Copy link
Contributor Author

@tomchristie I added a test for the file upload behaviour. I'm not sure what to assert at the end, but it does fail on django 1.5+ without the force_bytes fix. It also reveals a UnicodeDecodeError in django 1.4 and 1.3 that I can't see how to fix:

======================================================================
ERROR: test_upload_file (rest_framework.tests.test_testing.TestAPIRequestFactory)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/home/travis/build/tomchristie/django-rest-framework/rest_framework/tests/test_testing.py", line 154, in test_upload_file
    factory.post('/', data={'image': simple_png})
  File "/home/travis/build/tomchristie/django-rest-framework/rest_framework/test.py", line 75, in post
    data, content_type = self._encode_data(data, format, content_type)
  File "/home/travis/build/tomchristie/django-rest-framework/rest_framework/test.py", line 61, in _encode_data
    ret = renderer.render(data)
  File "/home/travis/build/tomchristie/django-rest-framework/rest_framework/renderers.py", line 603, in render
    return encode_multipart(self.BOUNDARY, data)
  File "/home/travis/virtualenv/python2.6/lib/python2.6/site-packages/django/test/client.py", line 145, in encode_multipart
    return '\r\n'.join(lines)
UnicodeDecodeError: 'ascii' codec can't decode byte 0x89 in position 0: ordinal not in range(128)

@LilyFoote
Copy link
Contributor Author

@tomchristie I looked into this some more and discovered that changing BOUNDARY = 'BoUnDaRyStRiNg' to BOUNDARY = b'BoUnDaRyStRiNg' in renderers makes the tests pass on python 2. Unfortunately it breaks python 3. I'll dig some more later.

Django's encode_multipart was updated in django 1.5 to work internally
with unicode and convert to bytes.

In django >= 1.5 we therefore need to pass the BOUNDARY as unicode. In
django < 1.5 we still need to pass it as bytes.
@LilyFoote
Copy link
Contributor Author

@tomchristie Could you review this please?

@tomchristie
Copy link
Member

This looks good to me.

Any chance you could also manually confirm that this works as expected both before and after Django 1.5 for actual uploads. I'm a little wary that some parts of the test client do not always exactly replicate exact behaviour - wouldn't want to get bitten by the tests passing but uploads failing when the code is actually run.

Once that's done this has the thumbs up from me.

@tomchristie
Copy link
Member

(And, thank you!)

@LilyFoote
Copy link
Contributor Author

@tomchristie The uploads themselves do work correctly. I have however found another issue: #1378.

tomchristie added a commit that referenced this pull request Jan 31, 2014
Import force_bytes on django >= 1.5
@tomchristie tomchristie merged commit 8f92116 into encode:master Jan 31, 2014
@LilyFoote LilyFoote deleted the force_bytes branch January 31, 2014 11:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants