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

wrong usage of LibCloudStorage._get_object in LibCloudStorage._read #1190

Closed
engAmirEng opened this issue Oct 27, 2022 · 0 comments · Fixed by #1191
Closed

wrong usage of LibCloudStorage._get_object in LibCloudStorage._read #1190

engAmirEng opened this issue Oct 27, 2022 · 0 comments · Fixed by #1191

Comments

@engAmirEng
Copy link
Contributor

I bumped into this bug when I was trying to make LibCloudStorage work with django's ManifestFilesMixin but that doesn't matter and it should be fixed regardless.

exact version of what it is now:

def _get_object(self, name):
    """Get object by its name. [Return None if object not found"""
    clean_name = self._clean_name(name)
    try:
        return self.driver.get_object(self.bucket, clean_name)
    except ObjectDoesNotExistError:
        return None

def _read(self, name):
    obj = self._get_object(name)
    # TOFIX : we should be able to read chunk by chunk
    return next(self.driver.download_object_as_stream(obj, obj.size))

my recommendation:

def _read(self, name):
    obj = self._get_object(name)
    if obj is None:
        raise FileNotFoundError(f"{name} does not exist.")
    # TOFIX : we should be able to read chunk by chunk
    return next(self.driver.download_object_as_stream(obj, obj.size))

and if you are curious about the exact trigger of the bug:

class ManifestFilesMixin(HashedFilesMixin):
    def read_manifest(self):
        try:
            with self.manifest_storage.open(self.manifest_name) as manifest:
                return manifest.read().decode()
        except FileNotFoundError:
            return None
engAmirEng added a commit to engAmirEng/django-storages that referenced this issue Oct 27, 2022
prevent accessing size attribute on None
engAmirEng added a commit to engAmirEng/django-storages that referenced this issue Feb 10, 2023
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.

1 participant