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.

Sign up for GitHub

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

Jump to bottom

ContentFile with str content throws TypeError #708

Closed
radimsuckr opened this issue May 22, 2019 · 10 comments
Closed

ContentFile with str content throws TypeError #708

radimsuckr opened this issue May 22, 2019 · 10 comments

Comments

@radimsuckr
Copy link

Hello, I've run into problems when I tried saving Django ContentFile. When I save the file with, for example, 'hello', it throws following exception: TypeError: Unicode-objects must be encoded before hashing.

However everything works fine with bytes. Below is traceback from Django shell. I've tried it on Django 1.11 and 2.2 with latest versions of both boto3 and django-storages.

Traceback:

>>> S3Boto3Storage().save('test.txt', ContentFile('hello'))
Traceback (most recent call last):
  File "<console>", line 1, in <module>
  File "/home/grelek/projects/djstorages-test/venv/lib/python3.7/site-packages/django/core/files/storage.py", line 52, in save
    return self._save(name, content)
  File "/home/grelek/projects/djstorages-test/venv/lib/python3.7/site-packages/storages/backends/s3boto3.py", line 506, in _save
    self._save_content(obj, content, parameters=parameters)
  File "/home/grelek/projects/djstorages-test/venv/lib/python3.7/site-packages/storages/backends/s3boto3.py", line 521, in _save_content
    obj.upload_fileobj(content, ExtraArgs=put_parameters)
  File "/home/grelek/projects/djstorages-test/venv/lib/python3.7/site-packages/boto3/s3/inject.py", line 621, in object_upload_fileobj
    ExtraArgs=ExtraArgs, Callback=Callback, Config=Config)
  File "/home/grelek/projects/djstorages-test/venv/lib/python3.7/site-packages/boto3/s3/inject.py", line 539, in upload_fileobj
    return future.result()
  File "/home/grelek/projects/djstorages-test/venv/lib/python3.7/site-packages/s3transfer/futures.py", line 106, in result
    return self._coordinator.result()
  File "/home/grelek/projects/djstorages-test/venv/lib/python3.7/site-packages/s3transfer/futures.py", line 265, in result
    raise self._exception
  File "/home/grelek/projects/djstorages-test/venv/lib/python3.7/site-packages/s3transfer/tasks.py", line 126, in __call__
    return self._execute_main(kwargs)
  File "/home/grelek/projects/djstorages-test/venv/lib/python3.7/site-packages/s3transfer/tasks.py", line 150, in _execute_main
    return_value = self._main(**kwargs)
  File "/home/grelek/projects/djstorages-test/venv/lib/python3.7/site-packages/s3transfer/upload.py", line 692, in _main
    client.put_object(Bucket=bucket, Key=key, Body=body, **extra_args)
  File "/home/grelek/projects/djstorages-test/venv/lib/python3.7/site-packages/botocore/client.py", line 357, in _api_call
    return self._make_api_call(operation_name, kwargs)
  File "/home/grelek/projects/djstorages-test/venv/lib/python3.7/site-packages/botocore/client.py", line 642, in _make_api_call
    request_signer=self._request_signer, context=request_context)
  File "/home/grelek/projects/djstorages-test/venv/lib/python3.7/site-packages/botocore/hooks.py", line 360, in emit_until_response
    return self._emitter.emit_until_response(aliased_event_name, **kwargs)
  File "/home/grelek/projects/djstorages-test/venv/lib/python3.7/site-packages/botocore/hooks.py", line 243, in emit_until_response
    responses = self._emit(event_name, kwargs, stop_on_response=True)
  File "/home/grelek/projects/djstorages-test/venv/lib/python3.7/site-packages/botocore/hooks.py", line 211, in _emit
    response = handler(**kwargs)
  File "/home/grelek/projects/djstorages-test/venv/lib/python3.7/site-packages/botocore/handlers.py", line 212, in conditionally_calculate_md5
    calculate_md5(params, **kwargs)
  File "/home/grelek/projects/djstorages-test/venv/lib/python3.7/site-packages/botocore/handlers.py", line 190, in calculate_md5
    binary_md5 = _calculate_md5_from_file(body)
  File "/home/grelek/projects/djstorages-test/venv/lib/python3.7/site-packages/botocore/handlers.py", line 204, in _calculate_md5_from_file
    md5.update(chunk)
TypeError: Unicode-objects must be encoded before hashing
@sww314
Copy link
Contributor

sww314 commented May 22, 2019

Try this:
S3Boto3Storage().save('test.txt', ContentFile(b'hello'))

@radimsuckr
Copy link
Author

Yeah, bytes work just fine, but the documentation states that str should work too. :( Or do I miss something?

https://django-storages.readthedocs.io/en/latest/backends/amazon-S3.html#model

@sww314
Copy link
Contributor

sww314 commented May 22, 2019

str works in Python 2. In Python 3, you need to encode it or use Bytes.

@sww314
Copy link
Contributor

sww314 commented May 22, 2019

See #707

@sww314 sww314 closed this as completed May 22, 2019
@IrinaAlexandra IrinaAlexandra mentioned this issue Feb 24, 2020
Closed
@LincolnPuzey
Copy link
Contributor

LincolnPuzey commented May 7, 2020 •
edited
Loading

@sww314 This issue should be re-opened.

It is perfectly valid Django to initialize a ContentFile with either a string or bytes in Python 3.

This can be seen in the Django docs: https://docs.djangoproject.com/en/dev/ref/files/file/#django.core.files.base.ContentFile
And in the source for ContentFile itself: https://github.com/django/django/blob/master/django/core/files/base.py#L126
Where __init__ checks if the content arg is a str.

So this is a bug in django-storages that should be fixed.

@LincolnPuzey
Copy link
Contributor

Note that the same error occurs when saving a Django File object initialized with a file object opened in text mode.

S3Boto3Storage().save("testfile.txt", File(StringIO("foo")))

@LincolnPuzey
Copy link
Contributor

@jschneier Can this issue be re-opened? As per my above comment I believe this is a valid bug in django-storages.

@radimsuckr
Copy link
Author

Hi @LincolnPuzey, I've implemented a workaround for this issue in module druids/django-chamber. You might want to take a look at it.

Nevertheless I agree with you that this behavior in django-storages is wrong and should be fixed.

@jschneier jschneier reopened this Jun 8, 2020
@hvitis hvitis mentioned this issue Jul 13, 2020
Closed
LincolnPuzey added a commit to LincolnPuzey/django-storages that referenced this issue Jul 27, 2020
@LincolnPuzey
Add test for saving files using bytes/strings and File/ContentFile.
add4d35 
2 of these tests currently fail due to issue jschneier#708.
They use moto to mock s3 at a lower level and actually get the right error to surface.
LincolnPuzey added a commit to LincolnPuzey/django-storages that referenced this issue Jul 27, 2020
@LincolnPuzey
Add test for saving files using bytes/strings and File/ContentFile.
edd441c 
2 of these tests currently fail due to issue jschneier#708.
They use moto to mock s3 at a lower level and actually get the right error to surface.
LincolnPuzey added a commit to LincolnPuzey/django-storages that referenced this issue Jul 27, 2020
@LincolnPuzey
Fix issue jschneier#708: Make _save() wrap content in a ReadBytesWrap…
9e4f043 
…per to handle file objects that are open in text/string mode.

This is done before gzip compressing, so also removed force_bytes() call in _compress_content(). Fix test that failed because of this.
@LincolnPuzey LincolnPuzey mentioned this issue Jul 27, 2020
Merged
LincolnPuzey added a commit to LincolnPuzey/django-storages that referenced this issue Aug 18, 2020
@LincolnPuzey
Add test for saving files using bytes/strings and File/ContentFile.
3625a67 
2 of these tests currently fail due to issue jschneier#708.
They use moto to mock s3 at a lower level and actually get the right error to surface.
LincolnPuzey added a commit to LincolnPuzey/django-storages that referenced this issue Aug 18, 2020
@LincolnPuzey
Fix issue jschneier#708: Make _save() wrap content in a ReadBytesWrap…
2ebd3c4 
…per to handle file objects that are open in text/string mode.

This is done before gzip compressing, so also removed force_bytes() call in _compress_content(). Fix test that failed because of this.
@bradenmacdonald bradenmacdonald mentioned this issue Aug 25, 2020
Merged
LincolnPuzey added a commit to LincolnPuzey/django-storages that referenced this issue Sep 19, 2020
@LincolnPuzey
Add test for saving files using bytes/strings and File/ContentFile.
f9e8c0c 
2 of these tests currently fail due to issue jschneier#708.
They use moto to mock s3 at a lower level and actually get the right error to surface.
LincolnPuzey added a commit to LincolnPuzey/django-storages that referenced this issue Sep 19, 2020
@LincolnPuzey
Fix issue jschneier#708: Make _save() wrap content in a ReadBytesWrap…
992d303 
…per to handle file objects that are open in text/string mode.

This is done before gzip compressing, so also removed force_bytes() call in _compress_content(). Fix test that failed because of this.
LincolnPuzey added a commit to LincolnPuzey/django-storages that referenced this issue Sep 19, 2020
@LincolnPuzey
Fix issue jschneier#708: Make _save() wrap content in a ReadBytesWrap…
7699236 
…per.

This correctly handles file-like-objects that are open in text/string mode by converting to strings returned by read() to bytes, which is what upload_fileobj() requires.
This is done before gzip compressing, so also removed force_bytes() call in _compress_content().
@LincolnPuzey LincolnPuzey mentioned this issue Dec 18, 2020
Closed
@sambles sambles mentioned this issue Apr 22, 2021
Merged
@dvf
Copy link

dvf commented Jul 19, 2021

Any update on this?

@phuocdn phuocdn mentioned this issue Oct 1, 2021
Open
@kdeggelman kdeggelman mentioned this issue Jan 28, 2022
Closed
jschneier pushed a commit to LincolnPuzey/django-storages that referenced this issue Sep 4, 2023
@LincolnPuzey @jschneier
Fix issue jschneier#708: Make _save() wrap content in a ReadBytesWrap…
5f742cd 
…per.

This correctly handles file-like-objects that are open in text/string mode by converting to strings returned by read() to bytes, which is what upload_fileobj() requires.
This is done before gzip compressing, so also removed force_bytes() call in _compress_content().
jschneier added a commit that referenced this issue Sep 4, 2023
@LincolnPuzey @jschneier
[s3] fix saving files with string content (#911)
19a15c2 
* Add alternate .venv to gitignore

* Add moto as test dependency.

* Add ReadBytesWrapper utility class for wrapping a file-like object.

This makes .read() always return bytes.
If .read() returns a string, it will be encoded to bytes before being returned.
The encoding to use can be specified, otherwise will use the .encoding property of the original file, otherwise will use utf-8.

* Fix issue #708: Make _save() wrap content in a ReadBytesWrapper.

This correctly handles file-like-objects that are open in text/string mode by converting to strings returned by read() to bytes, which is what upload_fileobj() requires.
This is done before gzip compressing, so also removed force_bytes() call in _compress_content().

* Add tests for saving both File/ContentFile with string/bytes.

Add these tests in a new test class that uses moto. Remove old test for saving ContentFile
Move test for detecting content-type to this new class. Add some more tests around this.
Fix tests that fail because settings.AWS_STORAGE_BUCKET_NAME is now defined.
Fix tests that fail because content is always wrapped.
Fix test for gzipped file since that now only takes bytes.

* tweaks

* add close to ReadBytesWrapper

* add readable to ReadBytesWrapper

---------

Co-authored-by: Josh Schneier <josh.schneier@gmail.com>
@jschneier
Copy link
Owner

Fixed by #911

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
5 participants
@dvf @sww314 @jschneier @radimsuckr @LincolnPuzey