Serve PDFs inline in the browser

Fixes #1158
Add config options WAGTAILDOCS_CONTENT_TYPES and WAGTAILDOCS_INLINE_CONTENT_TYPES to determine the Content-Type and Content-Disposition headers returned for documents when using the Django serve view, and default to application/pdf being served inline.

CHANGELOG.txt

@@ -16,6 +16,8 @@ Changelog
  * Show user's full name in report views (Matt Westcott)
  * Improve Wagtail admin page load performance by caching SVG icons sprite in localstorage (Coen van der Kamp)
  * Support SVG icons in ModelAdmin menu items (Scott Cranfill)
+ * Serve PDFs inline in the browser when accessed from the edit document view (Matt Westcott)
+ * Make document `content-type` and `content-disposition` configurable via `WAGTAILDOCS_CONTENT_TYPES` and `WAGTAILDOCS_INLINE_CONTENT_TYPES` (Matt Westcott)
  * Fix: Make page-level actions accessible to keyboard users in page listing tables (Jesse Menn)
  * Fix: `WAGTAILFRONTENDCACHE_LANGUAGES` was being interpreted incorrectly. It now accepts a list of strings, as documented (Karl Hobley)
  * Fix: Update oEmbed endpoints to use https where available (Matt Westcott)

docs/reference/settings.rst

@@ -180,6 +180,26 @@ For this reason, Wagtail provides a number of serving methods which trade some o
 
 If ``WAGTAILDOCS_SERVE_METHOD`` is unspecified or set to ``None``, the default method is ``'redirect'`` when a remote storage backend is in use (i.e. one that exposes a URL but not a local filesystem path), and ``'serve_view'`` otherwise. Finally, some storage backends may not expose a URL at all; in this case, serving will proceed as for ``'serve_view'``.
 
+.. _wagtaildocs_content_types:
+
+.. code-block:: python
+
+  WAGTAILDOCS_CONTENT_TYPES = {
+      'pdf': 'application/pdf',
+      'txt': 'text/plain',
+  }
+
+Specifies the MIME content type that will be returned for the given file extension, when using the ``serve_view`` method. Content types not listed here will be guessed using the Python ``mimetypes.guess_type`` function, or ``application/octet-stream`` if unsuccessful.
+
+.. _wagtaildocs_inline_content_types:
+
+.. code-block:: python
+
+  WAGTAILDOCS_INLINE_CONTENT_TYPES = ['application/pdf', 'text/plain']
+
+A list of MIME content types that will be shown inline in the browser (by serving the HTTP header ``Content-Disposition: inline``) rather than served as a download, when using the ``serve_view`` method. Defaults to ``application/pdf``.
+
+
 Password Management
 ===================
 

docs/releases/2.11.rst

@@ -25,6 +25,8 @@ Other features
  * Show user's full name in report views (Matt Westcott)
  * Improve Wagtail admin page load performance by caching SVG icons sprite in localstorage (Coen van der Kamp)
  * Support SVG icons in ModelAdmin menu items (Scott Cranfill)
+ * Serve PDFs inline in the browser when accessed from the edit document view (Matt Westcott)
+ * Make document ``content-type`` and ``content-disposition`` configurable via ``WAGTAILDOCS_CONTENT_TYPES`` and ``WAGTAILDOCS_INLINE_CONTENT_TYPES`` (Matt Westcott)
 
 
 Bug fixes

wagtail/documents/models.py

@@ -1,6 +1,8 @@
 import hashlib
 import os.path
+import urllib
 from contextlib import contextmanager
+from mimetypes import guess_type
 
 from django.conf import settings
 from django.db import models
@@ -147,6 +149,27 @@ class AbstractDocument(CollectionMember, index.Indexed, models.Model):
         from wagtail.documents.permissions import permission_policy
         return permission_policy.user_has_permission_for_instance(user, 'change', self)
 
+    @property
+    def content_type(self):
+        content_types_lookup = getattr(settings, 'WAGTAILDOCS_CONTENT_TYPES', {})
+        return (
+            content_types_lookup.get(self.file_extension.lower())
+            or guess_type(self.filename)[0]
+            or 'application/octet-stream'
+        )
+
+    @property
+    def content_disposition(self):
+        inline_content_types = getattr(
+            settings, 'WAGTAILDOCS_INLINE_CONTENT_TYPES', ['application/pdf']
+        )
+        if self.content_type in inline_content_types:
+            return 'inline'
+        else:
+            return "attachment; filename={0}; filename*=UTF-8''{0}".format(
+                urllib.parse.quote(self.filename)
+            )
+
     class Meta:
         abstract = True
         verbose_name = _('document')

wagtail/documents/tests/test_models.py

@@ -85,21 +85,43 @@ class TestDocumentFilenameProperties(TestCase):
         self.document = models.Document(title="Test document")
         self.document.file.save('example.doc', ContentFile("A boring example document"))
 
+        self.pdf_document = models.Document(title="Test document")
+        self.pdf_document.file.save('example.pdf', ContentFile("A boring example document"))
+
         self.extensionless_document = models.Document(title="Test document")
         self.extensionless_document.file.save('example', ContentFile("A boring example document"))
 
     def test_filename(self):
         self.assertEqual('example.doc', self.document.filename)
+        self.assertEqual('example.pdf', self.pdf_document.filename)
         self.assertEqual('example', self.extensionless_document.filename)
 
     def test_file_extension(self):
         self.assertEqual('doc', self.document.file_extension)
+        self.assertEqual('pdf', self.pdf_document.file_extension)
         self.assertEqual('', self.extensionless_document.file_extension)
 
+    def test_content_type(self):
+        self.assertEqual('application/msword', self.document.content_type)
+        self.assertEqual('application/pdf', self.pdf_document.content_type)
+        self.assertEqual('application/octet-stream', self.extensionless_document.content_type)
+
+    def test_content_disposition(self):
+        self.assertEqual(
+            '''attachment; filename=example.doc; filename*=UTF-8''example.doc''',
+            self.document.content_disposition
+        )
+        self.assertEqual('inline', self.pdf_document.content_disposition)
+        self.assertEqual(
+            '''attachment; filename=example; filename*=UTF-8''example''',
+            self.extensionless_document.content_disposition
+        )
+
     def tearDown(self):
         # delete the FieldFile directly because the TestCase does not commit
         # transactions to trigger transaction.on_commit() in the signal handler
         self.document.file.delete()
+        self.pdf_document.file.delete()
         self.extensionless_document.file.delete()
 
 

wagtail/documents/tests/test_views.py

@@ -18,6 +18,8 @@ class TestServeView(TestCase):
     def setUp(self):
         self.document = models.Document(title="Test document", file_hash="123456")
         self.document.file.save('example.doc', ContentFile("A boring example document"))
+        self.pdf_document = models.Document(title="Test document", file_hash="123456")
+        self.pdf_document.file.save('example.pdf', ContentFile("A boring example document"))
 
     def tearDown(self):
         if hasattr(self, 'response'):
@@ -30,9 +32,11 @@ class TestServeView(TestCase):
         # delete the FieldFile directly because the TestCase does not commit
         # transactions to trigger transaction.on_commit() in the signal handler
         self.document.file.delete()
+        self.pdf_document.file.delete()
 
-    def get(self):
-        self.response = self.client.get(reverse('wagtaildocs_serve', args=(self.document.id, self.document.filename)))
+    def get(self, document=None):
+        document = document or self.document
+        self.response = self.client.get(reverse('wagtaildocs_serve', args=(document.id, document.filename)))
         return self.response
 
     def test_response_code(self):
@@ -40,9 +44,14 @@ class TestServeView(TestCase):
 
     def test_content_disposition_header(self):
         self.assertEqual(
-            self.get()['Content-Disposition'],
+            self.get(self.document)['Content-Disposition'],
             'attachment; filename="{}"'.format(self.document.filename))
 
+    def test_inline_content_disposition_header(self):
+        self.assertEqual(
+            self.get(self.pdf_document)['Content-Disposition'],
+            'inline')
+
     @mock.patch('wagtail.documents.views.serve.hooks')
     @mock.patch('wagtail.documents.views.serve.get_object_or_404')
     def test_non_local_filesystem_content_disposition_header(
@@ -55,6 +64,8 @@ class TestServeView(TestCase):
         # Create a mock document with no local file to hit the correct code path
         mock_doc = mock.Mock()
         mock_doc.filename = self.document.filename
+        mock_doc.content_type = self.document.content_type
+        mock_doc.content_disposition = self.document.content_disposition
         mock_doc.file = StringIO('file-like object' * 10)
         mock_doc.file.path = None
         mock_doc.file.url = None
@@ -75,6 +86,38 @@ class TestServeView(TestCase):
             )
         )
 
+    @mock.patch('wagtail.documents.views.serve.hooks')
+    @mock.patch('wagtail.documents.views.serve.get_object_or_404')
+    def test_non_local_filesystem_inline_content_disposition_header(
+        self, mock_get_object_or_404, mock_hooks
+    ):
+        """
+        Tests the 'Content-Disposition' header in a response when using a
+        storage backend that doesn't expose filesystem paths.
+        """
+        # Create a mock document with no local file to hit the correct code path
+        mock_doc = mock.Mock()
+        mock_doc.filename = self.pdf_document.filename
+        mock_doc.content_type = self.pdf_document.content_type
+        mock_doc.content_disposition = self.pdf_document.content_disposition
+        mock_doc.file = StringIO('file-like object' * 10)
+        mock_doc.file.path = None
+        mock_doc.file.url = None
+        mock_doc.file.size = 30
+        mock_get_object_or_404.return_value = mock_doc
+
+        # Bypass 'before_serve_document' hooks
+        mock_hooks.get_hooks.return_value = []
+
+        response = self.get(self.pdf_document)
+
+        self.assertEqual(response.status_code, 200)
+
+        self.assertEqual(
+            response['Content-Disposition'],
+            "inline"
+        )
+
     def test_content_length_header(self):
         self.assertEqual(self.get()['Content-Length'], '25')
 

wagtail/documents/views/serve.py

@@ -1,4 +1,3 @@
-import urllib
 from wsgiref.util import FileWrapper
 
 from django.conf import settings
@@ -79,17 +78,16 @@ def serve(request, document_id, document_filename):
         # Use wagtail.utils.sendfile to serve the file;
         # this provides support for mimetypes, if-modified-since and django-sendfile backends
 
-        if hasattr(settings, 'SENDFILE_BACKEND'):
-            return sendfile(request, local_path, attachment=True, attachment_filename=doc.filename)
-        else:
+        sendfile_opts = {
+            'attachment': (doc.content_disposition != 'inline'),
+            'attachment_filename': doc.filename,
+            'mimetype': doc.content_type,
+        }
+        if not hasattr(settings, 'SENDFILE_BACKEND'):
             # Fallback to streaming backend if user hasn't specified SENDFILE_BACKEND
-            return sendfile(
-                request,
-                local_path,
-                attachment=True,
-                attachment_filename=doc.filename,
-                backend=sendfile_streaming_backend.sendfile
-            )
+            sendfile_opts['backend'] = sendfile_streaming_backend.sendfile
+
+        return sendfile(request, local_path, **sendfile_opts)
 
     else:
 
@@ -100,11 +98,11 @@ def serve(request, document_id, document_filename):
         # as a StreamingHttpResponse
 
         wrapper = FileWrapper(doc.file)
-        response = StreamingHttpResponse(wrapper, content_type='application/octet-stream')
+        response = StreamingHttpResponse(wrapper, doc.content_type)
 
         # set filename and filename* to handle non-ascii characters in filename
         # see https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/Content-Disposition
-        response['Content-Disposition'] = "attachment; filename={0}; filename*=UTF-8''{0}".format(urllib.parse.quote(doc.filename))
+        response['Content-Disposition'] = doc.content_disposition
 
         # FIXME: storage backends are not guaranteed to implement 'size'
         response['Content-Length'] = doc.file.size

wagtail/utils/sendfile.py

@@ -85,6 +85,8 @@ def sendfile(request, filename, attachment=False, attachment_filename=None, mime
                 parts.append('filename*=UTF-8\'\'%s' % quoted_filename)
 
         response['Content-Disposition'] = '; '.join(parts)
+    else:
+        response['Content-Disposition'] = 'inline'
 
     response['Content-length'] = os.path.getsize(filename)
     response['Content-Type'] = mimetype