dennisv / django-storage-swift

OpenStack Swift storage backend for Django
MIT License
86 stars 60 forks source link

401 Unauthorized: Temp URL invalid if path contains whitespaces #79

Closed dperetti closed 7 years ago

dperetti commented 7 years ago

Obviously a quoting issue. Suggestion : refactor url() using swiftclient's generate_temp_url().

FYI my quick and dirty workaround in my django app was as follow :

from swiftclient.utils import generate_temp_url
from six.moves.urllib import parse as urlparse

sw = SwiftStorage()
url = urlparse.urljoin(sw.base_url, urlparse.quote(sw.name_prefix + release_path))
path = urlparse.unquote(urlparse.urlsplit(url).path)
url = generate_temp_url(path=path, key=sw.temp_url_key, seconds=int(sw.temp_url_duration), method='GET')
return urlparse.urljoin(sw.base_url, url)
einarf commented 7 years ago

Got an example of an invalid url?

einarf commented 7 years ago

Please review the PR.

Also: Is this bug about file names containing spaces? It would be nice to get an example-url so we can write a test for this specific situation.

dperetti commented 7 years ago

Will let you know shortly !

einarf commented 7 years ago

Great. Not sure if the quoting matters or not. That's the only thing missing from your example.

dperetti commented 7 years ago

Trying to pip install your pull request like usual and this doesn't work:

pip install git+git://github.com/blacktorn/django-storage-swift.git@043fabe1af9532f9b5d649ef450abf0d75616802

Says it's not a tree. Any idea ?

einarf commented 7 years ago

Try from the repo/branch the pull requests came from: https://github.com/ZettaIO/django-storage-swift/tree/temp-url

einarf commented 7 years ago

To be more specific, use github's archive url: (tar.gz of branch temp-url) https://github.com/ZettaIO/django-storage-swift/archive/temp-url.tar.gz

pip install https://github.com/ZettaIO/django-storage-swift/archive/temp-url.tar.gz

.. or just clone the branch and run python setup.py install or python setup.py develop so you can easily modify the code while installed. The you don't have to mess around in site-packages etc.

dperetti commented 7 years ago

Using your fork works ... but I'm back to 401 ! The url seems OK but the temp_url_sig is probably wrong. It looks like you pass the sig as third argument of generate_temp_url, it should be an url_key as in my example above.

einarf commented 7 years ago

When do you get 401?

dperetti commented 7 years ago

When I go to the generated url.

einarf commented 7 years ago

Are you sure that added the SWIFT_TEMP_URL_KEY in your swift account as well?

dperetti commented 7 years ago

Of course, look at my example, I'm even using it and... it works. Again, I think you don't pass the correct arguments to generate_temp_url(). Read this : the third argument should precisely be the temp url key, not the hash of it !

einarf commented 7 years ago

ahh yes you are right. generate_temp_url is also generating the hmac.

Fixing.

dperetti commented 7 years ago

Side comment : the unit tests should have spotted that it didn't work ;-)

einarf commented 7 years ago

Yep they should ideally do that. Branch is updated 😄

Created issue #81

dperetti commented 7 years ago

Doesn't work : the path must be unquoted here because it was quoted here !

einarf commented 7 years ago

fixed

dperetti commented 7 years ago

Nope :

unquote() got an unexpected keyword argument 'encoding'

einarf commented 7 years ago

Must be a python 2 thing

einarf commented 7 years ago

Fixed. all good things are 5?

dperetti commented 7 years ago

That was the one ! 👍

einarf commented 7 years ago

.. and I added unit test to verify the generated temp-url key.

Please close this issue if there is nothing more to add.

einarf commented 7 years ago

@blacktorn @dperetti Can any one you close this issue?