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

s3boto3 backend throws exception when attempting to close file opened in read mode since V1.9 #831

Closed
mark4prime opened this issue Feb 3, 2020 · 4 comments · Fixed by #833

Comments

@mark4prime
Copy link

Since V1.9 it is now impossible to close a file opened in read mode using s3boto3 backend.

The culprit appears to be the _create_empty_on_close() method that has been added in this version which is called when no data has been written to the file. This asserts that 'w' is contained in the mode of the file which is obviously not present when opening a file for reading.

def _create_empty_on_close(self):
        """
        Attempt to create an empty file for this key when this File is closed if no bytes
        have been written and no object already exists on S3 for this key.
        This behavior is meant to mimic the behavior of Django's builtin FileSystemStorage,
        where files are always created after they are opened in write mode:
            f = storage.open("file.txt", mode="w")
            f.close()
        """
        assert "w" in self._mode
        assert self._raw_bytes_written == 0

        try:
            # Check if the object exists on the server; if so, don't do anything
            self.obj.load()
        except ClientError as err:
            if err.response["ResponseMetadata"]["HTTPStatusCode"] == 404:
                self.obj.put(
                    Body=b"", **self._storage._get_write_parameters(self.obj.key)
                )
            else:
                raise

Example:

settings.py

"""
Django settings for django_storages_test project.

Generated by 'django-admin startproject' using Django 3.0.3.

For more information on this file, see
https://docs.djangoproject.com/en/3.0/topics/settings/

For the full list of settings and their values, see
https://docs.djangoproject.com/en/3.0/ref/settings/
"""

import os

# Build paths inside the project like this: os.path.join(BASE_DIR, ...)
BASE_DIR = os.path.dirname(os.path.dirname(os.path.abspath(__file__)))


# Quick-start development settings - unsuitable for production
# See https://docs.djangoproject.com/en/3.0/howto/deployment/checklist/

# SECURITY WARNING: keep the secret key used in production secret!
SECRET_KEY = '=nc0!k1%0b(v7k!s27no)yo+gf=awni74)&crh(ngd1(v3-zjl'

# SECURITY WARNING: don't run with debug turned on in production!
DEBUG = True

ALLOWED_HOSTS = []


# Application definition

INSTALLED_APPS = [
    'django.contrib.admin',
    'django.contrib.auth',
    'django.contrib.contenttypes',
    'django.contrib.sessions',
    'django.contrib.messages',
    'django.contrib.staticfiles',
    'storages',
    'django_storages_test',
    'boto3'
]

MIDDLEWARE = [
    'django.middleware.security.SecurityMiddleware',
    'django.contrib.sessions.middleware.SessionMiddleware',
    'django.middleware.common.CommonMiddleware',
    'django.middleware.csrf.CsrfViewMiddleware',
    'django.contrib.auth.middleware.AuthenticationMiddleware',
    'django.contrib.messages.middleware.MessageMiddleware',
    'django.middleware.clickjacking.XFrameOptionsMiddleware',
]

ROOT_URLCONF = 'django_storages_test.urls'

TEMPLATES = [
    {
        'BACKEND': 'django.template.backends.django.DjangoTemplates',
        'DIRS': [os.path.join(BASE_DIR, 'templates')]
        ,
        'APP_DIRS': True,
        'OPTIONS': {
            'context_processors': [
                'django.template.context_processors.debug',
                'django.template.context_processors.request',
                'django.contrib.auth.context_processors.auth',
                'django.contrib.messages.context_processors.messages',
            ],
        },
    },
]

WSGI_APPLICATION = 'django_storages_test.wsgi.application'


# Database
# https://docs.djangoproject.com/en/3.0/ref/settings/#databases

DATABASES = {
    'default': {
        'ENGINE': 'django.db.backends.sqlite3',
        'NAME': os.path.join(BASE_DIR, 'db.sqlite3'),
    }
}


# Password validation
# https://docs.djangoproject.com/en/3.0/ref/settings/#auth-password-validators

AUTH_PASSWORD_VALIDATORS = [
    {
        'NAME': 'django.contrib.auth.password_validation.UserAttributeSimilarityValidator',
    },
    {
        'NAME': 'django.contrib.auth.password_validation.MinimumLengthValidator',
    },
    {
        'NAME': 'django.contrib.auth.password_validation.CommonPasswordValidator',
    },
    {
        'NAME': 'django.contrib.auth.password_validation.NumericPasswordValidator',
    },
]


# Internationalization
# https://docs.djangoproject.com/en/3.0/topics/i18n/

LANGUAGE_CODE = 'en-us'

TIME_ZONE = 'UTC'

USE_I18N = True

USE_L10N = True

USE_TZ = True


# Static files (CSS, JavaScript, Images)
# https://docs.djangoproject.com/en/3.0/howto/static-files/

STATIC_URL = '/static/'

# storages settings
DEFAULT_FILE_STORAGE = 'storages.backends.s3boto3.S3Boto3Storage'
AWS_ACCESS_KEY_ID = os.environ['AWS_ACCESS_KEY_ID']
AWS_SECRET_ACCESS_KEY = os.environ['AWS_SECRET_ACCESS_KEY']
AWS_STORAGE_BUCKET_NAME = 'django-storages-test'
AWS_S3_REGION_NAME = 'eu-west-1'

models.py

from django.db import models


class Foo(models.Model):
    foo_file = models.FileField()

usage

from django_storages_test import models
from django.core.files.base import ContentFile
foo = models.Foo.objects.create()
foo.foo_file.save('foo.txt', ContentFile(b'some text'))
/Pycharm/Envs/django_storages_test/lib/python3.7/site-packages/storages/backends/s3boto3.py:341: UserWarning: The default behavior of S3Boto3Storage is insecure and will change in django-storages 1.10. By default files and new buckets are saved with an ACL of 'public-read' (globally publicly readable). Version 1.10 will default to using the bucket's ACL. To opt into the new behavior set AWS_DEFAULT_ACL = None, otherwise to silence this warning explicitly set AWS_DEFAULT_ACL.
  "The default behavior of S3Boto3Storage is insecure and will change "
foo.foo_file.open()
<FieldFile: foo.txt>
foo.foo_file.close()
Traceback (most recent call last):
  File "<input>", line 1, in <module>
  File "/Pycharm/Envs/django_storages_test/lib/python3.7/site-packages/django/db/models/fields/files.py", line 123, in close
    file.close()
  File "/Pycharm/Envs/django_storages_test/lib/python3.7/site-packages/storages/backends/s3boto3.py", line 194, in close
    self._create_empty_on_close()
  File "/Pycharm/Envs/django_storages_test/lib/python3.7/site-packages/storages/backends/s3boto3.py", line 166, in _create_empty_on_close
    assert "w" in self._mode
AssertionError
@jschneier
Copy link
Owner

Yup can you open a PR removing the assert?

bpepple added a commit to Metron-Project/metron that referenced this issue Feb 3, 2020
Due to this bug causing thumbnails to generate:

jschneier/django-storages#831
@jschneier
Copy link
Owner

Just released 1.9.1 to address this. Thanks for the report.

@tonial
Copy link

tonial commented Feb 4, 2020

I was having this same issue migrating my Django project to 3.0.
Thank you for the fix !

@mark4prime
Copy link
Author

Thanks for the quick fix. I was just about to open a PR to do it myself this morning but you beat me to it!

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 a pull request may close this issue.

3 participants