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

Upgrade django-storages to support URLs with more http methods #6771

Merged
merged 4 commits into from Mar 11, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
13 changes: 13 additions & 0 deletions readthedocs/builds/storage.py
Expand Up @@ -186,3 +186,16 @@ def listdir(self, path):
if not self.exists(path):
return [], []
return super().listdir(path)

def url(self, name, *args, **kwargs): # noqa
"""
Override to accept extra arguments and ignore them all.

This method helps us to bring compatibility between Azure Blob Storage
(which does not use the HTTP method) and Amazon S3 (who requires HTTP
method to build the signed URL).

``FileSystemStorage`` does not support any other argument than ``name``.
https://docs.djangoproject.com/en/2.2/ref/files/storage/#django.core.files.storage.Storage.url
"""
return super().url(name)
13 changes: 2 additions & 11 deletions readthedocs/proxito/views/serve.py
Expand Up @@ -133,18 +133,9 @@ def get(self,
# Signature and Expire time is calculated per file.
path += 'index.html'

if request.method == 'HEAD':
# When request method is HEAD we can't use ``storage.url`` because
# the signature calculated will consider GET as method.
# django-storages does not support passing the HTTP method into
# ``storage.url`` yet. Because of this, the response won't be
# exactly the same between GET and HEAD since we are not passing
# the headers returned by the storage itself.
if storage.exists(path):
return HttpResponse()
raise Http404
# NOTE: calling ``.url`` will remove the trailing slash
storage_url = storage.url(path, http_method=request.method)

storage_url = storage.url(path) # this will remove the trailing slash
# URL without scheme and domain to perform an NGINX internal redirect
parsed_url = urlparse(storage_url)._replace(scheme='', netloc='')
final_url = parsed_url.geturl()
Expand Down
10 changes: 10 additions & 0 deletions readthedocs/storage/azure_storage.py
Expand Up @@ -20,6 +20,16 @@ class AzureBuildMediaStorage(BuildMediaStorageMixin, OverrideHostnameMixin, Azur
azure_container = getattr(settings, 'AZURE_MEDIA_STORAGE_CONTAINER', None) or 'media'
override_hostname = getattr(settings, 'AZURE_MEDIA_STORAGE_HOSTNAME', None)

def url(self, name, expire=None, http_method=None): # noqa
"""
Override to accept ``http_method`` and ignore it.

This method helps us to bring compatibility between Azure Blob Storage
(which does not use the HTTP method) and Amazon S3 (who requires HTTP
method to build the signed URL).
"""
return super().url(name, expire)


class AzureBuildStorage(AzureStorage):

Expand Down
7 changes: 4 additions & 3 deletions requirements/pip.txt
Expand Up @@ -106,9 +106,10 @@ django-cors-middleware==1.4.0
user-agents==2.0

# Utilities used to upload build media to cloud storage
django-storages[azure]==1.7.2
azure-storage-blob==1.5.0
azure-storage-common==1.4.2
# django-storages is pinned to this particular commit because it
# supports generating URLs with other method than GET when using
# private buckets
git+https://github.com/jschneier/django-storages@d0f027c98a877f75615cfc42b4d51c038fa41bf6#egg=django-storages[azure]==1.9.1

# Required only in development and linting
django-debug-toolbar==2.0
Expand Down