Opened 2 years ago

Last modified 2 years ago

#18929 new Bug

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 (3)

comment:1 Changed 2 years ago by jezdez

  • Needs documentation unset
  • Needs tests unset
  • Patch needs improvement unset

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

comment:2 Changed 2 years ago by anonymous

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 Changed 2 years ago by aaugustin

  • Needs tests set
  • Patch needs improvement set
  • Triage Stage changed from Unreviewed to Accepted

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.

Note: See TracTickets for help on using tickets.
Back to Top