uploads: Make django-sendfile to force downloading attachments.

We start to force downloads for the attachment files. We do this
for all files except images or pdf's. We would like images or pdf's
to open up in browser itself.

Tweaked by tabbott for comment clarity and correctness.

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()

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('<p>File not found</p>')
-    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,