From fa13582ffb75df88cf9689b9fc643ab49d22e116 Mon Sep 17 00:00:00 2001 From: rahuldeve Date: Thu, 9 Jun 2016 15:49:56 +0530 Subject: [PATCH] Serve uploaded files through get_uploaded_file in development. Previously, uploaded files were served: * With S3UploadBackend, via get_uploaded_file (redirects to S3) * With LocalUploadBackend in production, via nginx directly * With LocalUploadBackend in development, via Django's static file server This changes that last case to use get_uploaded_file in development, which is a key step towards being able to do proper access control authorization. Does not affect production. --- zerver/lib/upload.py | 7 +++++++ zerver/views/upload.py | 24 +++++++++++++++++++----- zproject/urls.py | 2 -- 3 files changed, 26 insertions(+), 7 deletions(-) diff --git a/zerver/lib/upload.py b/zerver/lib/upload.py index 0866bc4f82..4b9de8b5ca 100644 --- a/zerver/lib/upload.py +++ b/zerver/lib/upload.py @@ -240,6 +240,13 @@ def write_local_file(type, path, file_data): with open(file_path, 'wb') as f: f.write(file_data) +def get_local_file_path(path_id): + local_path = os.path.join(settings.LOCAL_UPLOADS_DIR, 'files', path_id) + if os.path.isfile(local_path): + return local_path + else: + return None + class LocalUploadBackend(ZulipUploadBackend): def upload_message_image(self, uploaded_file_name, content_type, file_data, user_profile, target_realm=None): # type: (str, str, str, UserProfile, Optional[Realm]) -> str diff --git a/zerver/views/upload.py b/zerver/views/upload.py index ea5e8b08e5..710fde0c23 100644 --- a/zerver/views/upload.py +++ b/zerver/views/upload.py @@ -1,13 +1,14 @@ from __future__ import absolute_import -from django.http import HttpRequest, HttpResponse, HttpResponseForbidden +from django.http import HttpRequest, HttpResponse, HttpResponseForbidden, FileResponse, \ + HttpResponseNotFound from django.shortcuts import redirect from django.utils.translation import ugettext as _ from zerver.decorator import authenticated_json_post_view, zulip_login_required from zerver.lib.request import has_request_variables, REQ from zerver.lib.response import json_success, json_error -from zerver.lib.upload import upload_message_image_from_request, \ +from zerver.lib.upload import upload_message_image_from_request, get_local_file_path, \ get_signed_upload_url, get_realm_for_filename from zerver.lib.validator import check_bool from zerver.models import UserProfile @@ -35,7 +36,7 @@ def serve_s3(request, user_profile, realm_id_str, filename, redir): realm_id = get_realm_for_filename(url_path) if realm_id is None: # File does not exist - return json_error(_("That file does not exist."), status=404) + return HttpResponseNotFound('

File not found

') else: realm_id = int(realm_id_str) @@ -49,10 +50,24 @@ def serve_s3(request, user_profile, realm_id_str, filename, redir): else: return HttpResponseForbidden() +# TODO: Rewrite this once we have django-sendfile +def serve_local(request, path_id): + # type: (HttpRequest, str) -> HttpResponse + import os + import mimetypes + local_path = get_local_file_path(path_id) + if local_path is None: + return HttpResponseNotFound('

File not found

') + filename = os.path.basename(local_path) + response = FileResponse(open(local_path, 'rb'), content_type = mimetypes.guess_type(filename)) + return response + response['Content-Disposition'] = 'attachment; filename=%s' % (filename) + def serve_file_backend(request, user_profile, realm_id_str, filename, redir): # type: (HttpRequest, UserProfile, str, str, bool) -> HttpResponse + path_id = "%s/%s" % (realm_id_str, filename) if settings.LOCAL_UPLOADS_DIR is not None: - return HttpResponseForbidden() # Should have been served by nginx + return serve_local(request, path_id) return serve_s3(request, user_profile, realm_id_str, filename, redir) @@ -69,4 +84,3 @@ def get_uploaded_file(request, realm_id_str, filename, # type: (HttpRequest, str, str, bool) -> HttpResponse user_profile = request.user return serve_file_backend(request, user_profile, realm_id_str, filename, redir) - diff --git a/zproject/urls.py b/zproject/urls.py index a7552618f6..f9f4269075 100644 --- a/zproject/urls.py +++ b/zproject/urls.py @@ -98,8 +98,6 @@ if settings.DEVELOPMENT and settings.LOCAL_UPLOADS_DIR is not None: urlpatterns += patterns('', url(r'^user_avatars/(?P.*)$', 'django.views.static.serve', {'document_root': os.path.join(settings.LOCAL_UPLOADS_DIR, "avatars")}), - url(r'^user_uploads/(?P.*)$', 'django.views.static.serve', - {'document_root': os.path.join(settings.LOCAL_UPLOADS_DIR, "files")}), ) urlpatterns += patterns('zerver.views',