Opened 12 years ago

Closed 5 years ago

Last modified 5 years ago

#18929 closed Bug (invalid)

CachedFilesMixin is not compatible with S3BotoStorage.

Reported by: idanzalz@… Owned by: nobody
Component: contrib.staticfiles Version: 1.4
Severity: Normal Keywords:
Cc: Triage Stage: Accepted
Has patch: yes Needs documentation: no
Needs tests: yes Patch needs improvement: yes
Easy pickings: no UI/UX: no

Description

CachedFilesMixin breaks S3BotoStorage when used together because it applies "unquote" on the result of the storage url method.
see https://github.com/django/django/pull/321 for more details

Change History (4)

comment:1 by Jannis Leidel, 12 years ago

Please don't just point to a pull request, this ticket exists to discuss the issue.

comment:2 by anonymous, 12 years ago

There isn't really that much to add. The storage's url method result gets wrapped with unquote when using the CachedFilesMixin.
This sometimes produces a wrong url for the S3BotoStorage, since the Signature part in the url may contain %2B which is translated to '+' by unquote and then you get an authentication error from AWS when trying to remove it.

Calling unquote on the url seems like a side effect of the CachedFilesMixin that has no connection to what it's supposed to do so I think it should be removed.

comment:3 by Aymeric Augustin, 12 years ago

Needs tests: set
Patch needs improvement: set
Triage Stage: UnreviewedAccepted

This ticket looks valid. But the current patch isn't acceptable because it breaks tests (as mentioned by Karen on the pull request).

To move forward, the code should make it clearer which kind of path (filesystem, URL-unescaped, URL-escaped) in accepted and return by storage functions, and more tests must be added.

comment:4 by Mariusz Felisiak, 5 years ago

Resolution: invalid
Status: newclosed
Summary: CachedFilesMixin is not compatible with S3BotoStorageCachedFilesMixin is not compatible with S3BotoStorage.

This issue is not valid anymore because CachedFilesMixin and CachedStaticFilesStorage will be removed in Django 3.1 (see f1894bae3071da4ee577fc40ae61491f3e03d82c) and an eventual patch doesn't qualify for a backport.

Last edited 5 years ago by Mariusz Felisiak (previous) (diff)
Note: See TracTickets for help on using tickets.
Back to Top