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

S3BotoStorage fails when keys contain ":" #248

Closed
joharohl opened this issue Jan 16, 2017 · 6 comments
Closed

S3BotoStorage fails when keys contain ":" #248

joharohl opened this issue Jan 16, 2017 · 6 comments
Labels

Comments

@joharohl
Copy link

Trying to do this:
storage.delete("2017-01-13T12:55:32.497821+00:00/somekey")
Which fails like so:

  File "...", line 76, in ...
    storage.delete("2017-01-13T12:55:32.497821+00:00/somekey")
  File ".../local/lib/python2.7/site-packages/storages/backends/s3boto.py", line 438, in delete
    name = self._normalize_name(self._clean_name(name))
  File "/.../local/lib/python2.7/site-packages/storages/backends/s3boto.py", line 364, in _normalize_name
    name)
django.core.exceptions.SuspiciousOperation: Attempted access to "2017-01-13T12:55:32.497821+00:00/somekey" denied.

I have tracked this down to storages.backends.s3boto.safe_join's call to urlparse.urljoin. Having ":" confuses the join to skip the / at the beginning of final_path which then fails during the check that final_path is a sub path of base.

It's probably a small fix, but I unfortunately don't have enough knowledge of the urlparse lib to know how to do it.

PS.
And yes, AWS supports having ":" part of the keys. Running aws s3 rm s3://<bucket>/2017-01-13T12:55:32.497821+00:00/somekey works fine.

@jschneier
Copy link
Owner

Indeed. According to Amazon's Docs the colon (":") should probably be url-encoded. That is a bug in the library, probably we should make safe_join significantly smarter and ensure it complies with the docs.

@jschneier
Copy link
Owner

Out of curiosity how are you getting the file up to the server? If I use the normal Django fileuploader process then those characters are stripped out.

@jschneier
Copy link
Owner

Also, renaming a file to have : as you do via the aws cli (which I believe uses boto3 anyway) urlencodes the characters.

@joharohl
Copy link
Author

This isn't so much of an issue in for the user uploads if it strips annoying characters then.

Our use case is for a backup script that uses datetimes for the folder names to be able to prune backups later solely based on the directory names. So we are mostly using django-storages as a convenient way to reuse the storage definitions we already use. We could of course just use boto3 directly, but it would be nice if it worked directly with django-storages.

So don't feel like it's too much of a priority to fix.

@jschneier
Copy link
Owner

Ah, makes sense. What does the actual url that amazon shows you look like in S3? My hunch is it is already urlencoded which would make this a pretty easy fix.

@joharohl
Copy link
Author

Hi, sorry for late reply. Yes, the colons in the url are urlencoded.

So 2017-01-13T12:55:32.497821+00:00 turns into 2017-01-13T12%3A55%3A32.497821%2B00%3A00

jleclanche pushed a commit that referenced this issue Jun 5, 2017
Combine the identical s3boto3 and s3boto implementations of safe_join()
and its tests to reduce code duplication.

Fixes #248
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

3 participants