diff --git a/zerver/tests/test_upload.py b/zerver/tests/test_upload.py index 58a4c81b1f..9cfa35b4bf 100644 --- a/zerver/tests/test_upload.py +++ b/zerver/tests/test_upload.py @@ -543,7 +543,8 @@ class FileUploadTest(UploadSerializeMixin, ZulipTestCase): self.logout() def test_serve_local(self) -> None: - def check_xsend_links(name: Text, name_str_for_test: Text) -> None: + def check_xsend_links(name: Text, name_str_for_test: Text, + content_disposition: Text='') -> None: with self.settings(SENDFILE_BACKEND='sendfile.backends.nginx'): _get_sendfile.clear() # To clearout cached version of backend from djangosendfile self.login(self.example_email("hamlet")) @@ -559,9 +560,20 @@ class FileUploadTest(UploadSerializeMixin, ZulipTestCase): self.assertEqual(response['X-Accel-Redirect'], '/serve_uploads/../../' + test_upload_dir + '/files/' + fp_path + '/' + name_str_for_test) + if content_disposition != '': + self.assertIn('attachment;', response['Content-disposition']) + self.assertIn(content_disposition, response['Content-disposition']) + else: + self.assertEqual(response.get('Content-disposition'), None) - check_xsend_links('zulip.txt', 'zulip.txt') - check_xsend_links('áéБД.txt', '%C3%A1%C3%A9%D0%91%D0%94.txt') + check_xsend_links('zulip.txt', 'zulip.txt', "filename*=UTF-8''zulip.txt") + check_xsend_links('áéБД.txt', '%C3%A1%C3%A9%D0%91%D0%94.txt', + "filename*=UTF-8''%C3%A1%C3%A9%D0%91%D0%94.txt") + check_xsend_links('zulip.html', 'zulip.html', "filename*=UTF-8''zulip.html") + check_xsend_links('zulip.sh', 'zulip.sh', "filename*=UTF-8''zulip.sh") + check_xsend_links('zulip.jpeg', 'zulip.jpeg') + check_xsend_links('áéБД.pdf', '%C3%A1%C3%A9%D0%91%D0%94.pdf') + check_xsend_links('zulip', 'zulip', "filename*=UTF-8''zulip") def tearDown(self) -> None: destroy_uploads() diff --git a/zerver/views/upload.py b/zerver/views/upload.py index cfcd95dc76..a5d5eb7c74 100644 --- a/zerver/views/upload.py +++ b/zerver/views/upload.py @@ -13,6 +13,7 @@ from zerver.lib.validator import check_bool from zerver.models import UserProfile, validate_attachment_request from django.conf import settings from sendfile import sendfile +from mimetypes import guess_type def serve_s3(request: HttpRequest, url_path: str) -> HttpResponse: uri = get_signed_upload_url(url_path) @@ -22,7 +23,30 @@ def serve_local(request: HttpRequest, path_id: str) -> HttpResponse: local_path = get_local_file_path(path_id) if local_path is None: return HttpResponseNotFound('

File not found

') - return sendfile(request, local_path) + + # Here we determine whether a browser should treat the file like + # an attachment (and thus clicking a link to it should download) + # or like a link (and thus clicking a link to it should display it + # in a browser tab). This is controlled by the + # Content-Disposition header; `django-sendfile` sends the + # attachment-style version of that header if and only if the + # attachment argument is passed to it. For attachments, + # django-sendfile sets the response['Content-disposition'] like + # this: `attachment; filename="b'zulip.txt'"; filename*=UTF-8''zulip.txt`. + # + # The "filename" field (used to name the file when downloaded) is + # unreliable because it doesn't have a well-defined encoding; the + # newer filename* field takes precedence, since it uses a + # consistent format (urlquoted). For more details on filename* + # and filename, see the below docs: + # https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/Content-Disposition + attachment = True + file_type = guess_type(local_path)[0] + if file_type is not None and (file_type.startswith("image/") or + file_type == "application/pdf"): + attachment = False + + return sendfile(request, local_path, attachment=attachment) @has_request_variables def serve_file_backend(request: HttpRequest, user_profile: UserProfile,