diff --git a/cms/envs/common.py b/cms/envs/common.py index c9f45468c9..445df80d30 100644 --- a/cms/envs/common.py +++ b/cms/envs/common.py @@ -41,7 +41,7 @@ from lms.envs.common import ( # indirectly accessed through the email opt-in API, which is # technically accessible through the CMS via legacy URLs. PROFILE_IMAGE_BACKEND, PROFILE_IMAGE_DEFAULT_FILENAME, PROFILE_IMAGE_DEFAULT_FILE_EXTENSION, - PROFILE_IMAGE_SECRET_KEY, + PROFILE_IMAGE_SECRET_KEY, PROFILE_IMAGE_MIN_BYTES, PROFILE_IMAGE_MAX_BYTES, ) from path import path from warnings import simplefilter diff --git a/common/test/acceptance/tests/lms/test_learner_profile.py b/common/test/acceptance/tests/lms/test_learner_profile.py index 9844d43964..226ddfe87c 100644 --- a/common/test/acceptance/tests/lms/test_learner_profile.py +++ b/common/test/acceptance/tests/lms/test_learner_profile.py @@ -510,7 +510,7 @@ class OwnLearnerProfilePageTest(LearnerProfileTestMixin, WebAppTest): self.assert_default_image_has_public_access(profile_page) profile_page.upload_file(filename='larger_image.jpg') - self.assertEqual(profile_page.profile_image_message, "Your image must be smaller than 1 MB in size.") + self.assertEqual(profile_page.profile_image_message, "The file must be smaller than 1 MB in size.") profile_page.visit() self.assertTrue(profile_page.profile_has_default_image) @@ -534,7 +534,7 @@ class OwnLearnerProfilePageTest(LearnerProfileTestMixin, WebAppTest): self.assert_default_image_has_public_access(profile_page) profile_page.upload_file(filename='list-icon-visited.png') - self.assertEqual(profile_page.profile_image_message, "Your image must be at least 100 bytes in size.") + self.assertEqual(profile_page.profile_image_message, "The file must be at least 100 bytes in size.") profile_page.visit() self.assertTrue(profile_page.profile_has_default_image) @@ -558,7 +558,10 @@ class OwnLearnerProfilePageTest(LearnerProfileTestMixin, WebAppTest): self.assert_default_image_has_public_access(profile_page) profile_page.upload_file(filename='cohort_users_only_username.csv') - self.assertEqual(profile_page.profile_image_message, "Unsupported file type.") + self.assertEqual( + profile_page.profile_image_message, + "The file must be one of the following types: .gif, .png, .jpeg, .jpg." + ) profile_page.visit() self.assertTrue(profile_page.profile_has_default_image) diff --git a/lms/static/js/spec/student_profile/learner_profile_fields_spec.js b/lms/static/js/spec/student_profile/learner_profile_fields_spec.js index bbf289d263..0d307a2282 100644 --- a/lms/static/js/spec/student_profile/learner_profile_fields_spec.js +++ b/lms/static/js/spec/student_profile/learner_profile_fields_spec.js @@ -195,7 +195,7 @@ define(['backbone', 'jquery', 'underscore', 'js/common_helpers/ajax_helpers', 'j // Verify error message expect($('.message-banner').text().trim()) - .toBe('Your image must be smaller than 64 bytes in size.'); + .toBe('The file must be smaller than 64 bytes in size.'); }); it("can't upload image having size less than min size", function() { @@ -208,7 +208,7 @@ define(['backbone', 'jquery', 'underscore', 'js/common_helpers/ajax_helpers', 'j imageView.$('.upload-button-input').fileupload('add', {files: [createFakeImageFile(10)]}); // Verify error message - expect($('.message-banner').text().trim()).toBe('Your image must be at least 16 bytes in size.'); + expect($('.message-banner').text().trim()).toBe('The file must be at least 16 bytes in size.'); }); it("can't upload and remove image if parental consent required", function() { diff --git a/lms/static/js/views/fields.js b/lms/static/js/views/fields.js index 33456b9d63..1303b9bfc5 100644 --- a/lms/static/js/views/fields.js +++ b/lms/static/js/views/fields.js @@ -668,7 +668,7 @@ humanReadableSize = this.bytesToHumanReadable(this.options.imageMinBytes); this.showErrorMessage( interpolate_text( - gettext("Your image must be at least {size} in size."), {size: humanReadableSize} + gettext("The file must be at least {size} in size."), {size: humanReadableSize} ) ); return false; @@ -676,7 +676,7 @@ humanReadableSize = this.bytesToHumanReadable(this.options.imageMaxBytes); this.showErrorMessage( interpolate_text( - gettext("Your image must be smaller than {size} in size."), {size: humanReadableSize} + gettext("The file must be smaller than {size} in size."), {size: humanReadableSize} ) ); return false; diff --git a/openedx/core/djangoapps/profile_images/images.py b/openedx/core/djangoapps/profile_images/images.py index ab2e974cd0..4e78295a98 100644 --- a/openedx/core/djangoapps/profile_images/images.py +++ b/openedx/core/djangoapps/profile_images/images.py @@ -11,11 +11,55 @@ from PIL import Image from openedx.core.djangoapps.user_api.accounts.image_helpers import get_profile_image_storage -FILE_UPLOAD_TOO_LARGE = _noop('Maximum file size exceeded.') -FILE_UPLOAD_TOO_SMALL = _noop('Minimum file size not met.') -FILE_UPLOAD_BAD_TYPE = _noop('Unsupported file type.') -FILE_UPLOAD_BAD_EXT = _noop('File extension does not match data.') -FILE_UPLOAD_BAD_MIMETYPE = _noop('Content-Type header does not match data.') +IMAGE_TYPES = { + 'jpeg': { + 'extension': [".jpeg", ".jpg"], + 'mimetypes': ['image/jpeg', 'image/pjpeg'], + 'magic': ["ffd8"] + }, + 'png': { + 'extension': [".png"], + 'mimetypes': ['image/png'], + 'magic': ["89504e470d0a1a0a"] + }, + 'gif': { + 'extension': [".gif"], + 'mimetypes': ['image/gif'], + 'magic': ["474946383961", "474946383761"] + } +} + + +def user_friendly_size(size): + """ + Convert size in bytes to user friendly size. + + Arguments: + size (int): size in bytes + + Returns: + user friendly size + """ + units = [_('bytes'), _('KB'), _('MB')] + i = 0 + while size >= 1024: + size /= 1024 + i += 1 + return u'{} {}'.format(size, units[i]) + + +def get_valid_file_types(): + """ + Return comma separated string of valid file types. + """ + return ', '.join([', '.join(IMAGE_TYPES[ft]['extension']) for ft in IMAGE_TYPES.keys()]) + + +FILE_UPLOAD_TOO_LARGE = _noop(u'The file must be smaller than {image_max_size} in size.'.format(image_max_size=user_friendly_size(settings.PROFILE_IMAGE_MAX_BYTES))) # pylint: disable=line-too-long +FILE_UPLOAD_TOO_SMALL = _noop(u'The file must be at least {image_min_size} in size.'.format(image_min_size=user_friendly_size(settings.PROFILE_IMAGE_MIN_BYTES))) # pylint: disable=line-too-long +FILE_UPLOAD_BAD_TYPE = _noop(u'The file must be one of the following types: {valid_file_types}.'.format(valid_file_types=get_valid_file_types())) # pylint: disable=line-too-long +FILE_UPLOAD_BAD_EXT = _noop(u'The file name extension for this file does not match the file data. The file may be corrupted.') # pylint: disable=line-too-long +FILE_UPLOAD_BAD_MIMETYPE = _noop(u'The Content-Type header for this file does not match the file data. The file may be corrupted.') # pylint: disable=line-too-long class ImageValidationError(Exception): @@ -37,26 +81,10 @@ def validate_uploaded_image(uploaded_file): uploaded file as the source image for a user's profile image. Otherwise, returns nothing. """ + # validation code by @pmitros, # adapted from https://github.com/pmitros/ProfileXBlock # see also: http://en.wikipedia.org/wiki/Magic_number_%28programming%29 - image_types = { - 'jpeg': { - 'extension': [".jpeg", ".jpg"], - 'mimetypes': ['image/jpeg', 'image/pjpeg'], - 'magic': ["ffd8"] - }, - 'png': { - 'extension': [".png"], - 'mimetypes': ['image/png'], - 'magic': ["89504e470d0a1a0a"] - }, - 'gif': { - 'extension': [".gif"], - 'mimetypes': ['image/gif'], - 'magic': ["474946383961", "474946383761"] - } - } if uploaded_file.size > settings.PROFILE_IMAGE_MAX_BYTES: raise ImageValidationError(FILE_UPLOAD_TOO_LARGE) @@ -65,17 +93,17 @@ def validate_uploaded_image(uploaded_file): # check the file extension looks acceptable filename = unicode(uploaded_file.name).lower() - filetype = [ft for ft in image_types if any(filename.endswith(ext) for ext in image_types[ft]['extension'])] + filetype = [ft for ft in IMAGE_TYPES if any(filename.endswith(ext) for ext in IMAGE_TYPES[ft]['extension'])] if not filetype: raise ImageValidationError(FILE_UPLOAD_BAD_TYPE) filetype = filetype[0] # check mimetype matches expected file type - if uploaded_file.content_type not in image_types[filetype]['mimetypes']: + if uploaded_file.content_type not in IMAGE_TYPES[filetype]['mimetypes']: raise ImageValidationError(FILE_UPLOAD_BAD_MIMETYPE) # check magic number matches expected file type - headers = image_types[filetype]['magic'] + headers = IMAGE_TYPES[filetype]['magic'] if uploaded_file.read(len(headers[0]) / 2).encode('hex') not in headers: raise ImageValidationError(FILE_UPLOAD_BAD_EXT) # avoid unexpected errors from subsequent modules expecting the fp to be at 0 diff --git a/openedx/core/djangoapps/user_api/accounts/api.py b/openedx/core/djangoapps/user_api/accounts/api.py index 9c941cf5e3..c619e5c164 100644 --- a/openedx/core/djangoapps/user_api/accounts/api.py +++ b/openedx/core/djangoapps/user_api/accounts/api.py @@ -162,7 +162,7 @@ def update_account_settings(requesting_user, update, username=None): for read_only_field in read_only_fields: field_errors[read_only_field] = { "developer_message": u"This field is not editable via this API", - "user_message": _(u"Field '{field_name}' cannot be edited.").format(field_name=read_only_field) + "user_message": _(u"The '{field_name}' field cannot be edited.").format(field_name=read_only_field) } del update[read_only_field] @@ -263,7 +263,7 @@ def _add_serializer_errors(update, serializer, field_errors): "developer_message": u"Value '{field_value}' is not valid for field '{field_name}': {error}".format( field_value=field_value, field_name=key, error=error ), - "user_message": _(u"Value '{field_value}' is not valid for field '{field_name}'.").format( + "user_message": _(u"This value is invalid.").format( field_value=field_value, field_name=key ), } diff --git a/openedx/core/djangoapps/user_api/accounts/tests/test_views.py b/openedx/core/djangoapps/user_api/accounts/tests/test_views.py index 4caa602e5f..fd3c9aaeae 100644 --- a/openedx/core/djangoapps/user_api/accounts/tests/test_views.py +++ b/openedx/core/djangoapps/user_api/accounts/tests/test_views.py @@ -385,7 +385,7 @@ class TestAccountAPI(UserAPITestCase): if fails_validation_value: error_response = self.send_patch(client, {field: fails_validation_value}, expected_status=400) self.assertEqual( - u"Value '{0}' is not valid for field '{1}'.".format(fails_validation_value, field), + u'This value is invalid.', error_response.data["field_errors"][field]["user_message"] ) self.assertEqual( @@ -422,7 +422,7 @@ class TestAccountAPI(UserAPITestCase): "This field is not editable via this API", data["field_errors"][field_name]["developer_message"] ) self.assertEqual( - "Field '{0}' cannot be edited.".format(field_name), data["field_errors"][field_name]["user_message"] + "The '{0}' field cannot be edited.".format(field_name), data["field_errors"][field_name]["user_message"] ) for field_name in ["username", "date_joined", "is_active", "profile_image", "requires_parental_consent"]: