diff --git a/CHANGELOG.rst b/CHANGELOG.rst index 37399b342..f3f4a1530 100644 --- a/CHANGELOG.rst +++ b/CHANGELOG.rst @@ -12,6 +12,12 @@ CHANGELOG * Changed the preferred way to do model registration via model inheritance and ``mptt.AlreadyRegistered``, which is deprecated since django-mptt 0.4 * Use dashed name for django-polymorphic dependency in setup.py +* In ``models.File``, add field ``mime_type`` to store the Content-Type as set by + the browser during file upload +* For extended Django Filer models, adopt the classmethod ``matches_file_type`` to its + new signature, this is a breaking change +* Add attribute ``download`` to the download link in order to offer the file + under its original name 1.7.1 (2020-04-29) diff --git a/docs/extending_filer.rst b/docs/extending_filer.rst index 6e8a72861..755a5f2df 100644 --- a/docs/extending_filer.rst +++ b/docs/extending_filer.rst @@ -47,11 +47,12 @@ So let's add a ``matches_file_type()`` method to the ``Video`` model: .. code-block:: python @classmethod - def matches_file_type(cls, iname, ifile, request): - # the extensions we'll recognise for this file type - filename_extensions = ['.dv', '.mov', '.mp4', '.avi', '.wmv',] - ext = os.path.splitext(iname)[1].lower() - return ext in filename_extensions + def matches_file_type(cls, iname, ifile, mime_type): + video_types = ['application/vnd.dvb.ait', 'video/x-sgi-movie', 'video/mp4', 'video/mpeg', + 'video/x-msvideo', 'video/x-ms-wmv', 'video/ogg', 'video/webm', 'video/quicktime'] + return mime_type in video_types + +.. note:: The signature of this classmethod changed in version 2.0. Now you can upload files of those types into the Filer. diff --git a/filer/admin/clipboardadmin.py b/filer/admin/clipboardadmin.py index c4644dabf..f4f973fbd 100644 --- a/filer/admin/clipboardadmin.py +++ b/filer/admin/clipboardadmin.py @@ -85,10 +85,10 @@ def ajax_upload(request, folder_id=None): try: if len(request.FILES) == 1: # dont check if request is ajax or not, just grab the file - upload, filename, is_raw = handle_request_files_upload(request) + upload, filename, is_raw, mime_type = handle_request_files_upload(request) else: # else process the request as usual - upload, filename, is_raw = handle_upload(request) + upload, filename, is_raw, mime_type = handle_upload(request) # TODO: Deprecated/refactor # Get clipboad # clipboard = Clipboard.objects.get_or_create(user=request.user)[0] @@ -97,7 +97,7 @@ def ajax_upload(request, folder_id=None): for filer_class in filer_settings.FILER_FILE_MODELS: FileSubClass = load_model(filer_class) # TODO: What if there are more than one that qualify? - if FileSubClass.matches_file_type(filename, upload, request): + if FileSubClass.matches_file_type(filename, upload, mime_type): FileForm = modelform_factory( model=FileSubClass, fields=('original_filename', 'owner', 'file') @@ -111,6 +111,7 @@ def ajax_upload(request, folder_id=None): # Enforce the FILER_IS_PUBLIC_DEFAULT file_obj.is_public = filer_settings.FILER_IS_PUBLIC_DEFAULT file_obj.folder = folder + file_obj.mime_type = mime_type file_obj.save() # TODO: Deprecated/refactor # clipboard_item = ClipboardItem( diff --git a/filer/migrations/0012_file_mime_type.py b/filer/migrations/0012_file_mime_type.py new file mode 100644 index 000000000..4d6889f5c --- /dev/null +++ b/filer/migrations/0012_file_mime_type.py @@ -0,0 +1,30 @@ +# Generated by Django 2.1.9 on 2019-06-25 19:12 + +import mimetypes + +from django.db import migrations, models + +import filer.models.filemodels + + +def guess_mimetypes(apps, schema_editor): + FileModel = apps.get_model('filer', 'File') + for file_obj in FileModel.objects.all(): + file_obj.mime_type, _ = mimetypes.guess_type(file_obj.file.url) + file_obj.save(update_fields=['mime_type']) + + +class Migration(migrations.Migration): + + dependencies = [ + ('filer', '0011_auto_20190418_0137'), + ] + + operations = [ + migrations.AddField( + model_name='file', + name='mime_type', + field=models.CharField(default='application/octet-stream', help_text='MIME type of uploaded content', max_length=255, validators=[filer.models.filemodels.mimetype_validator]), + ), + migrations.RunPython(guess_mimetypes, reverse_code=migrations.RunPython.noop), + ] diff --git a/filer/models/abstract.py b/filer/models/abstract.py index 531214a0e..e9b5f9bdf 100644 --- a/filer/models/abstract.py +++ b/filer/models/abstract.py @@ -1,5 +1,4 @@ import logging -import os from django.db import models from django.utils.translation import gettext_lazy as _ @@ -42,14 +41,11 @@ class BaseImage(File): ) @classmethod - def matches_file_type(cls, iname, ifile, request): - # This was originally in admin/clipboardadmin.py it was inside of a try - # except, I have moved it here outside of a try except because I can't - # figure out just what kind of exception this could generate... all it was - # doing for me was obscuring errors... - # --Dave Butler - iext = os.path.splitext(iname)[1].lower() - return iext in ['.jpg', '.jpeg', '.png', '.gif'] + def matches_file_type(cls, iname, ifile, mime_type): + # source: https://www.freeformatter.com/mime-types-list.html + image_subtypes = ['gif', 'jpeg', 'png', 'x-png'] + maintype, subtype = mime_type.split('/') + return maintype == 'image' and subtype in image_subtypes def file_data_changed(self, post_init=False): attrs_updated = super().file_data_changed(post_init=post_init) diff --git a/filer/models/filemodels.py b/filer/models/filemodels.py index 457f869a3..fd9fbbc5d 100644 --- a/filer/models/filemodels.py +++ b/filer/models/filemodels.py @@ -1,8 +1,10 @@ import hashlib +import mimetypes import os from datetime import datetime from django.conf import settings +from django.core.exceptions import ValidationError from django.core.files.base import ContentFile from django.db import models from django.urls import NoReverseMatch, reverse @@ -38,6 +40,12 @@ def is_public_default(): return filer_settings.FILER_IS_PUBLIC_DEFAULT +def mimetype_validator(value): + if not mimetypes.guess_extension(value): + msg = "'{mimetype}' is not a recognized MIME-Type." + raise ValidationError(msg.format(mimetype=value)) + + class File(PolymorphicModel, mixins.IconsMixin): file_type = 'File' _icon = "file" @@ -81,10 +89,17 @@ class File(PolymorphicModel, mixins.IconsMixin): 'file. File will be publicly accessible ' 'to anyone.')) + mime_type = models.CharField( + max_length=255, + help_text='MIME type of uploaded content', + validators=[mimetype_validator], + default='application/octet-stream', + ) + objects = FileManager() @classmethod - def matches_file_type(cls, iname, ifile, request): + def matches_file_type(cls, iname, ifile, mime_type): return True # I match all files... def __init__(self, *args, **kwargs): diff --git a/filer/server/backends/base.py b/filer/server/backends/base.py index 777a98f48..5e811ee4e 100644 --- a/filer/server/backends/base.py +++ b/filer/server/backends/base.py @@ -1,4 +1,3 @@ -import mimetypes import os from django.utils.encoding import smart_str @@ -10,9 +9,6 @@ class ServerBase: Warning: this API is EXPERIMENTAL and may change at any time. """ - def get_mimetype(self, path): - return mimetypes.guess_type(path)[0] or 'application/octet-stream' - def default_headers(self, **kwargs): self.save_as_header(**kwargs) self.size_header(**kwargs) @@ -44,3 +40,6 @@ def size_header(self, response, **kwargs): # be an expensive operation. # elif file and file.size is not None: # response['Content-Length'] = file.size + + def serve(self, request, filer_file, **kwargs): + raise NotImplementedError(".serve() must be overridden") diff --git a/filer/server/backends/default.py b/filer/server/backends/default.py index abcb300b7..264a6ab10 100644 --- a/filer/server/backends/default.py +++ b/filer/server/backends/default.py @@ -15,19 +15,19 @@ class DefaultServer(ServerBase): This will only work for files that can be accessed in the local filesystem. """ - def serve(self, request, file_obj, **kwargs): - fullpath = file_obj.path + def serve(self, request, filer_file, **kwargs): + fullpath = filer_file.path # the following code is largely borrowed from `django.views.static.serve` # and django-filetransfers: filetransfers.backends.default if not os.path.exists(fullpath): raise Http404('"%s" does not exist' % fullpath) # Respect the If-Modified-Since header. statobj = os.stat(fullpath) - response_params = {'content_type': self.get_mimetype(fullpath)} + response_params = {'content_type': filer_file.mime_type} if not was_modified_since(request.META.get('HTTP_IF_MODIFIED_SINCE'), statobj[stat.ST_MTIME], statobj[stat.ST_SIZE]): return HttpResponseNotModified(**response_params) response = HttpResponse(open(fullpath, 'rb').read(), **response_params) response["Last-Modified"] = http_date(statobj[stat.ST_MTIME]) - self.default_headers(request=request, response=response, file_obj=file_obj, **kwargs) + self.default_headers(request=request, response=response, file_obj=filer_file.file, **kwargs) return response diff --git a/filer/server/backends/nginx.py b/filer/server/backends/nginx.py index c2ebb4431..03a7ff17f 100644 --- a/filer/server/backends/nginx.py +++ b/filer/server/backends/nginx.py @@ -18,12 +18,10 @@ def __init__(self, location, nginx_location): def get_nginx_location(self, path): return path.replace(self.location, self.nginx_location) - def serve(self, request, file_obj, **kwargs): - # we should not use get_mimetype() here, because it tries to access the - # file in the filesystem. + def serve(self, request, filer_file, **kwargs): response = HttpResponse() - del response['Content-Type'] - nginx_path = self.get_nginx_location(file_obj.path) + response['Content-Type'] = filer_file.mime_type + nginx_path = self.get_nginx_location(filer_file.path) response['X-Accel-Redirect'] = nginx_path - self.default_headers(request=request, response=response, file_obj=file_obj, **kwargs) + self.default_headers(request=request, response=response, file_obj=filer_file.file, **kwargs) return response diff --git a/filer/server/backends/xsendfile.py b/filer/server/backends/xsendfile.py index 4e52fe352..476a87f7b 100644 --- a/filer/server/backends/xsendfile.py +++ b/filer/server/backends/xsendfile.py @@ -4,14 +4,14 @@ class ApacheXSendfileServer(ServerBase): - def serve(self, request, file_obj, **kwargs): + def serve(self, request, filer_file, **kwargs): response = HttpResponse() - response['X-Sendfile'] = file_obj.path + response['X-Sendfile'] = filer_file.path # This is needed for lighttpd, hopefully this will # not be needed after this is fixed: # http://redmine.lighttpd.net/issues/2076 - response['Content-Type'] = self.get_mimetype(file_obj.path) + response['Content-Type'] = filer_file.mime_type - self.default_headers(request=request, response=response, file_obj=file_obj, **kwargs) + self.default_headers(request=request, response=response, file_obj=filer_file.file, **kwargs) return response diff --git a/filer/server/views.py b/filer/server/views.py index a8b671ae8..5b87abb6d 100644 --- a/filer/server/views.py +++ b/filer/server/views.py @@ -28,7 +28,7 @@ def serve_protected_file(request, path): raise PermissionDenied else: raise Http404('File not found') - return server.serve(request, file_obj=file_obj.file, save_as=False) + return server.serve(request, file_obj=file_obj, save_as=False) @never_cache @@ -51,6 +51,7 @@ def serve_protected_thumbnail(request, path): raise Http404('File not found') try: thumbnail = ThumbnailFile(name=path, storage=file_obj.file.thumbnail_storage) - return thumbnail_server.serve(request, thumbnail, save_as=False) + thumbnail_temp_file = File(file=thumbnail, mime_type=file_obj.mime_type) + return thumbnail_server.serve(request, thumbnail_temp_file, save_as=False) except Exception: raise Http404('File not found') diff --git a/filer/utils/files.py b/filer/utils/files.py index f850d00b9..d3f7c8c2c 100644 --- a/filer/utils/files.py +++ b/filer/utils/files.py @@ -1,3 +1,4 @@ +import mimetypes import os from django.http.multipartparser import ( @@ -31,6 +32,8 @@ def handle_upload(request): # This means we shouldn't continue...raise an error. raise UploadException("Invalid content length: %r" % content_length) + mime_type = request.META.get('CONTENT_TYPE', 'application/octet-stream') + upload_handlers = request.upload_handlers for handler in upload_handlers: handler.handle_raw_input(request, @@ -89,16 +92,16 @@ def handle_upload(request): break else: if len(request.FILES) == 1: - upload, filename, is_raw = handle_request_files_upload(request) + upload, filename, is_raw, mime_type = handle_request_files_upload(request) else: raise UploadException("AJAX request not valid: Bad Upload") - return upload, filename, is_raw + return upload, filename, is_raw, mime_type def handle_request_files_upload(request): """ Handle request.FILES if len(request.FILES) == 1. - Returns tuple(upload, filename, is_raw) where upload is file itself. + Returns tuple(upload, filename, is_raw, mime_type) where upload is file itself. """ # FILES is a dictionary in Django but Ajax Upload gives the uploaded file # an ID based on a random number, so it cannot be guessed here in the code. @@ -109,7 +112,12 @@ def handle_request_files_upload(request): is_raw = False upload = list(request.FILES.values())[0] filename = upload.name - return upload, filename, is_raw + _, iext = os.path.splitext(filename) + mime_type = upload.content_type.lower() + if iext not in mimetypes.guess_all_extensions(mime_type): + msg = "MIME-Type '{mimetype}' does not correspond to file extension of {filename}." + raise UploadException(msg.format(mimetype=mime_type, filename=filename)) + return upload, filename, is_raw, mime_type def slugify(string): diff --git a/tests/test_admin.py b/tests/test_admin.py index f1269aec1..e4cd53b23 100644 --- a/tests/test_admin.py +++ b/tests/test_admin.py @@ -213,6 +213,10 @@ def setUp(self): self.video_name = 'test_file.mov' self.video_filename = os.path.join(settings.FILE_UPLOAD_TEMP_DIR, self.video_name) self.video.save(self.video_filename, 'JPEG') + self.binary_name = 'aaa.bin' + self.binary_filename = os.path.join(settings.FILE_UPLOAD_TEMP_DIR, self.binary_name) + with open(self.binary_filename, 'wb') as fh: + fh.write(bytearray(100 * b'a')) super().setUp() def tearDown(self): @@ -287,8 +291,25 @@ def test_filer_upload_file_no_folder(self, extra_headers={}): } response = self.client.post(url, post_data, **extra_headers) # noqa self.assertEqual(Image.objects.count(), 1) - self.assertEqual(Image.objects.all()[0].original_filename, - self.image_name) + stored_image = Image.objects.first() + self.assertEqual(stored_image.original_filename, self.image_name) + self.assertEqual(stored_image.mime_type, 'image/jpeg') + + def test_filer_upload_binary_data(self, extra_headers={}): + self.assertEqual(File.objects.count(), 0) + file_obj = django.core.files.File(open(self.binary_filename, 'rb')) + url = reverse('admin:filer-ajax_upload') + post_data = { + 'Filename': self.binary_name, + 'Filedata': file_obj, + 'jsessionid': self.client.session.session_key + } + response = self.client.post(url, post_data, **extra_headers) # noqa + self.assertEqual(Image.objects.count(), 0) + self.assertEqual(File.objects.count(), 1) + stored_file = File.objects.first() + self.assertEqual(stored_file.original_filename, self.binary_name) + self.assertEqual(stored_file.mime_type, 'application/octet-stream') def test_filer_ajax_upload_file(self): self.assertEqual(Image.objects.count(), 0) @@ -301,12 +322,33 @@ def test_filer_ajax_upload_file(self): response = self.client.post( # noqa url, data=file_obj.read(), - content_type='application/octet-stream', + content_type='image/jpeg', **{'HTTP_X_REQUESTED_WITH': 'XMLHttpRequest'} ) self.assertEqual(Image.objects.count(), 1) - self.assertEqual(Image.objects.all()[0].original_filename, - self.image_name) + stored_image = Image.objects.first() + self.assertEqual(stored_image.original_filename, self.image_name) + self.assertEqual(stored_image.mime_type, 'image/jpeg') + + def test_filer_ajax_upload_file_using_content_type(self): + self.assertEqual(Image.objects.count(), 0) + folder = Folder.objects.create(name='foo') + file_obj = django.core.files.File(open(self.binary_filename, 'rb')) + url = reverse( + 'admin:filer-ajax_upload', + kwargs={'folder_id': folder.pk} + ) + '?filename=renamed.pdf' + response = self.client.post( # noqa + url, + data=file_obj.read(), + content_type='application/pdf', + **{'HTTP_X_REQUESTED_WITH': 'XMLHttpRequest'} + ) + self.assertEqual(Image.objects.count(), 0) + self.assertEqual(File.objects.count(), 1) + stored_file = File.objects.first() + self.assertEqual(stored_file.original_filename, 'renamed.pdf') + self.assertEqual(stored_file.mime_type, 'application/pdf') def test_filer_ajax_upload_file_no_folder(self): self.assertEqual(Image.objects.count(), 0) @@ -317,12 +359,13 @@ def test_filer_ajax_upload_file_no_folder(self): response = self.client.post( # noqa url, data=file_obj.read(), - content_type='application/octet-stream', + content_type='image/jpeg', **{'HTTP_X_REQUESTED_WITH': 'XMLHttpRequest'} ) self.assertEqual(Image.objects.count(), 1) - self.assertEqual(Image.objects.all()[0].original_filename, - self.image_name) + stored_image = Image.objects.first() + self.assertEqual(stored_image.original_filename, self.image_name) + self.assertEqual(stored_image.mime_type, 'image/jpeg') def test_filer_upload_file_error(self, extra_headers={}): self.assertEqual(Image.objects.count(), 0) diff --git a/tests/test_server_backends.py b/tests/test_server_backends.py index dd9038bd4..54c0d9923 100644 --- a/tests/test_server_backends.py +++ b/tests/test_server_backends.py @@ -21,15 +21,16 @@ class Mock(): class BaseServerBackendTestCase(TestCase): def setUp(self): - original_filename = 'testimage.jpg' + original_filename, mime_type = 'testimage.jpg', 'image/jpeg' file_obj = SimpleUploadedFile( name=original_filename, content=create_image().tobytes(), - content_type='image/jpeg') + content_type=mime_type) self.filer_file = File.objects.create( is_public=False, file=file_obj, - original_filename=original_filename) + original_filename=original_filename, + mime_type=mime_type) def tearDown(self): self.filer_file.delete() @@ -40,27 +41,27 @@ def test_normal(self): server = DefaultServer() request = Mock() request.META = {} - response = server.serve(request, self.filer_file.file) + response = server.serve(request, self.filer_file) self.assertTrue(response.has_header('Last-Modified')) def test_save_as(self): server = DefaultServer() request = Mock() request.META = {} - response = server.serve(request, self.filer_file.file, save_as=True) + response = server.serve(request, self.filer_file, save_as=True) self.assertEqual(response['Content-Disposition'], 'attachment; filename=testimage.jpg') - response = server.serve(request, self.filer_file.file, save_as=False) + response = server.serve(request, self.filer_file, save_as=False) self.assertFalse(response.has_header('Content-Disposition')) - response = server.serve(request, self.filer_file.file, save_as='whatever.png') + response = server.serve(request, self.filer_file, save_as='whatever.png') self.assertEqual(response['Content-Disposition'], 'attachment; filename=whatever.png') def test_not_modified(self): server = DefaultServer() request = Mock() request.META = {'HTTP_IF_MODIFIED_SINCE': http_date(time.time())} - response = server.serve(request, self.filer_file.file) + response = server.serve(request, self.filer_file) self.assertTrue(isinstance(response, HttpResponseNotModified)) def test_missing_file(self): @@ -82,7 +83,7 @@ def setUp(self): def test_normal(self): request = Mock() request.META = {} - response = self.server.serve(request, self.filer_file.file) + response = self.server.serve(request, self.filer_file) headers = dict(response.items()) self.assertTrue(response.has_header('X-Accel-Redirect')) self.assertTrue(headers['X-Accel-Redirect'].startswith(self.server.nginx_location)) @@ -97,7 +98,7 @@ def test_missing_file(self): request = Mock() request.META = {} os.remove(self.filer_file.file.path) - response = self.server.serve(request, self.filer_file.file) + response = self.server.serve(request, self.filer_file) headers = dict(response.items()) self.assertTrue(response.has_header('X-Accel-Redirect')) self.assertTrue(headers['X-Accel-Redirect'].startswith(self.server.nginx_location)) @@ -112,7 +113,7 @@ def setUp(self): def test_normal(self): request = Mock() request.META = {} - response = self.server.serve(request, self.filer_file.file) + response = self.server.serve(request, self.filer_file) headers = dict(response.items()) self.assertTrue(response.has_header('X-Sendfile')) self.assertEqual(headers['X-Sendfile'], self.filer_file.file.path) @@ -127,7 +128,7 @@ def test_missing_file(self): request = Mock() request.META = {} os.remove(self.filer_file.file.path) - response = self.server.serve(request, self.filer_file.file) + response = self.server.serve(request, self.filer_file) headers = dict(response.items()) self.assertTrue(response.has_header('X-Sendfile')) self.assertEqual(headers['X-Sendfile'], self.filer_file.file.path)