Pull requests

#72 Merged
Repository
mmlin mmlin
Branch
default
Repository
david david
Branch
default

For the s3 and s3boto backends, the 'url' methods remove trailing slashes but should not. This fixes that.

Author
  1. Mike Lin
Reviewers
Description

Among other things, this is preventing the calendar widget from being rendered on the admin app. It's because of a lost slash when constructing the URL that ends up looking something like this: https://bucket.s3.amazonaws.com/static/adminimg/icon_clock.gif (notice the missing slash between "admin" and "img").

The problem stems from the url() method of the backend class, which is called in the admin base template (contrib/admin/templates/admin/base.html) from a line like this:

<script type="text/javascript">window.__admin_media_prefix__ = "{% filter escapejs %}{% static "admin/" %}{% endfilter %}";</script>

{% static "admin/" %} is removing the trailing slash. The static tag in this case is what calls the url method on the class that handles static storage. In my case that class was S3BotoStorage. I chose the patch the url method itself instead of the methods it calls that actually remove the slash (_clean_name or _normalize_name) in order to avoid unwanted side-effects where those other methods are called.

I also made an identical change in the S3Storage class, but, to be honest, I didn't test that change.

*Update: Rolled back changes to the deprecated S3 backend. Used string.endswith() to be more concise. Added test cases. For the test cases, I used a custom domain to avoid having to set the AWS bucket, key, and secret when testing.*

Comments (5)

  1. Ian Lewis

    I think we can go ahead an merge this but I would appreciate a test for this change.

    S3.py is deprecated so I'm not sure we should actually touch that. Did you actually use the S3 backend and encounter the problem you are describing?

  2. Mike Lin author

    No, I haven't used the S3 backend. I would prefer not to mess with it, so I'll pull that change out. I'll add the test case too.

  3. Mike Lin author

    Should be ready now, Ian. For the test cases, I used a custom domain to avoid having to set the AWS bucket, key, and secret when testing.

    Let me know if anything looks off to you. Thanks.