Fix file upload issue

File extensions with capital letters where being filtered out by allowed_file. Also now provide a proper error message when upload fail

compair/api/file.py

@@ -27,7 +27,7 @@
 
 def allowed_file(filename, allowed):
     return '.' in filename and \
-        filename.rsplit('.', 1)[1] in allowed
+        filename.lower().rsplit('.', 1)[1] in allowed
 
 
 def random_generator(size=8, chars=string.ascii_uppercase + string.digits):
@@ -38,42 +38,44 @@ def random_generator(size=8, chars=string.ascii_uppercase + string.digits):
 class FileAPI(Resource):
     @login_required
     def post(self):
-        uploaded_file = request.files['file']
+        uploaded_file = request.files.get('file')
+
+        if not uploaded_file:
+            return {"error": "No file attachment found"}, 400
+        elif not allowed_file(uploaded_file.filename, current_app.config['ATTACHMENT_ALLOWED_EXTENSIONS']):
+            return {"error": "Invalid file extension"}, 400
 
         on_save_file.send(
             self,
             event_name=on_save_file.name,
             user=current_user,
             data={'file': uploaded_file.filename})
 
-        if uploaded_file and allowed_file(uploaded_file.filename, current_app.config['ATTACHMENT_ALLOWED_EXTENSIONS']):
-            try:
-                db_file = File(
-                    user_id=current_user.id,
-                    name='',
-                    alias=uploaded_file.filename
-                )
-                db.session.add(db_file)
-                db.session.commit()
-
-                # use uuid generated by file model for name
-                name = db_file.uuid + '.' + uploaded_file.filename.rsplit('.', 1)[1]
-
-                # create new file with name
-                full_path = os.path.join(current_app.config['ATTACHMENT_UPLOAD_FOLDER'], name)
-                uploaded_file.save(full_path)
-                current_app.logger.debug("Saved attachment {}/{}".format(current_app.config['ATTACHMENT_UPLOAD_FOLDER'], name))
-
-                # update file record with name
-                db_file.name = name
-                db.session.commit()
-            except Exception as e:
-                db.session.rollback()
-                raise e
-
-            return {'file': marshal(db_file, dataformat.get_file())}
-
-        return False
+        try:
+            db_file = File(
+                user_id=current_user.id,
+                name='',
+                alias=uploaded_file.filename
+            )
+            db.session.add(db_file)
+            db.session.commit()
+
+            # use uuid generated by file model for name
+            name = db_file.uuid + '.' + uploaded_file.filename.lower().rsplit('.', 1)[1]
+
+            # create new file with name
+            full_path = os.path.join(current_app.config['ATTACHMENT_UPLOAD_FOLDER'], name)
+            uploaded_file.save(full_path)
+            current_app.logger.debug("Saved attachment {}/{}".format(current_app.config['ATTACHMENT_UPLOAD_FOLDER'], name))
+
+            # update file record with name
+            db_file.name = name
+            db.session.commit()
+        except Exception as e:
+            db.session.rollback()
+            raise e
+
+        return {'file': marshal(db_file, dataformat.get_file())}
 
 api.add_resource(FileAPI, '')
 

compair/static/modules/attachment/attachment-module.js

@@ -176,18 +176,21 @@ module.service('attachService',
 
     var onComplete = function() {
         return function(fileItem, response) {
-            if (response) {
-                file = response['file'];
+            if (response && response.file) {
+                file = response.file;
             }
         };
     };
 
     var onError = function() {
         return function(fileItem, response, status) {
+            fileItem.cancel();
+            fileItem.remove();
+            reset();
             if (response == '413') {
                 Toaster.error("File Size Error", "The file is larger than 25MB. Please upload a smaller file.");
             } else {
-                Toaster.reqerror("Attachment Fail", status);
+                Toaster.reqerror("Attachment Upload Fail", status);
             }
         };
     };
@@ -265,19 +268,22 @@ module.service('answerAttachService',
     var uploadedCallback = function(fileItem) {};
     var onComplete = function() {
         return function(fileItem, response) {
-            if (response) {
-                file = response['file'];
+            if (response && response.file) {
+                file = response.file;
                 uploadedCallback(file);
             }
         };
     };
 
     var onError = function() {
         return function(fileItem, response, status) {
+            fileItem.cancel();
+            fileItem.remove();
+            reset();
             if (response == '413') {
                 Toaster.error("File Size Error", "The file is larger than 25MB. Please upload a smaller file.");
             } else {
-                Toaster.reqerror("Attachment Fail", status);
+                Toaster.reqerror("Attachment Upload Fail", status);
             }
         };
     };

compair/tests/api/test_file.py

@@ -104,10 +104,15 @@ def test_create_attachment(self):
         url = '/api/attachment'
         test_formats = [
             ('pdf', 'application/pdf'),
+            ('PDF', 'application/pdf'),
             ('mp3', 'audio/mpeg'),
+            ('MP3', 'audio/mpeg'),
             ('mp4', 'video/mp4'),
+            ('MP4', 'video/mp4'),
             ('jpg', 'image/jpeg'),
-            ('jpeg', 'image/jpeg')
+            ('JPG', 'image/jpeg'),
+            ('jpeg', 'image/jpeg'),
+            ('JPEG', 'image/jpeg')
         ]
 
         # test login required
@@ -117,6 +122,20 @@ def test_create_attachment(self):
         uploaded_file.close()
 
         with self.login(self.fixtures.instructor.username):
+            # test no file uploaded
+            filename = 'alias.pdf'
+            rv = self.client.post(url, data=dict())
+            self.assert400(rv)
+            print(rv.json)
+            self.assertEqual("No file attachment found", rv.json['error'])
+
+            # test no file uploaded
+            filename = 'alias.xyz'
+            uploaded_file = io.BytesIO(b"this is a test")
+            rv = self.client.post(url, data=dict(file=(uploaded_file, filename)))
+            self.assert400(rv)
+            self.assertEqual("Invalid file extension", rv.json['error'])
+
             for extension, mimetype in test_formats:
                 filename = 'alias.'+extension
 
@@ -127,9 +146,9 @@ def test_create_attachment(self):
 
                 actual_file = rv.json['file']
                 self.files_to_cleanup.append(actual_file['name'])
-                self.assertEqual(actual_file['id']+"."+extension, actual_file['name'])
+                self.assertEqual(actual_file['id']+"."+extension.lower(), actual_file['name'])
                 self.assertEqual(filename, actual_file['alias'])
-                self.assertEqual(extension, actual_file['extension'])
+                self.assertEqual(extension.lower(), actual_file['extension'])
                 self.assertEqual(mimetype, actual_file['mimetype'])
 
     def test_delete_attachment(self):