From a0288303f6ede7d2fc25b3d03096d16bb4c00e40 Mon Sep 17 00:00:00 2001
From: Tim Walling <mail@timwalling.com>
Date: Thu, 11 Aug 2016 03:46:06 -0400
Subject: [PATCH] Check file type and file extension when importing csv (#7185)

issue #7144
- added a check for file type and file extension
- added an error message to the localization file
- added integration test
---
 core/server/api/subscribers.js                |  5 ++++-
 core/server/translations/en.json              |  3 ++-
 .../integration/api/api_subscription_spec.js  | 21 +++++++++++++++++--
 3 files changed, 25 insertions(+), 4 deletions(-)

diff --git a/core/server/api/subscribers.js b/core/server/api/subscribers.js
index d7f0ac8882a7..f3d9a9a95e5d 100644
--- a/core/server/api/subscribers.js
+++ b/core/server/api/subscribers.js
@@ -275,7 +275,10 @@ subscribers = {
                 return Promise.reject(new errors.ValidationError(i18n.t('errors.api.db.selectFileToImport')));
             }
 
-            // TODO: check for valid entries
+            // Check for valid file type
+            if (!utils.checkFileIsValid(options, ['text/csv','application/csv'], ['.csv'])) {
+                return Promise.reject(new errors.ValidationError(i18n.t('errors.api.subscribers.selectValidFile')));
+            }
 
             return options;
         }
diff --git a/core/server/translations/en.json b/core/server/translations/en.json
index 6c4a7b34334c..9ed93b74b567 100644
--- a/core/server/translations/en.json
+++ b/core/server/translations/en.json
@@ -347,7 +347,8 @@
             },
             "subscribers": {
                 "subscriberNotFound": "Subscriber not found.",
-                "subscriberAlreadyExists": "Email address is already subscribed."
+                "subscriberAlreadyExists": "Email address is already subscribed.",
+                "selectValidFile": "Please select a valid CSV file to import."
             },
             "tags": {
                 "tagNotFound": "Tag not found."
diff --git a/core/test/integration/api/api_subscription_spec.js b/core/test/integration/api/api_subscription_spec.js
index 7cbd6b94b0e3..b53813e2b8a9 100644
--- a/core/test/integration/api/api_subscription_spec.js
+++ b/core/test/integration/api/api_subscription_spec.js
@@ -238,13 +238,15 @@ describe('Subscribers API', function () {
     });
 
     describe('Read CSV', function () {
-        var scope = {};
+        var scope = {},
+            stub;
 
         beforeEach(function () {
             sinon.stub(fs, 'unlink', function (path, cb) {
                 cb();
             });
             sinon.stub(apiUtils, 'checkFileExists').returns(true);
+            stub = sinon.stub(apiUtils, 'checkFileIsValid').returns(true);
             sinon.stub(serverUtils, 'readCSV', function () {
                 if (scope.csvError) {
                     return Promise.reject(new Error('csv'));
@@ -257,7 +259,9 @@ describe('Subscribers API', function () {
         afterEach(function () {
             fs.unlink.restore();
             apiUtils.checkFileExists.restore();
+            apiUtils.checkFileIsValid.restore();
             serverUtils.readCSV.restore();
+            scope.csvError = false;
         });
 
         it('check that fn works in general', function (done) {
@@ -286,7 +290,7 @@ describe('Subscribers API', function () {
                 .catch(done);
         });
 
-        it('read csv throws an error', function (done) {
+        it('read csv throws a not found error', function (done) {
             scope.csvError = true;
 
             SubscribersAPI.importCSV(_.merge(testUtils.context.internal, {path: '/somewhere'}))
@@ -298,5 +302,18 @@ describe('Subscribers API', function () {
                     done();
                 });
         });
+
+        it('throws an invalid file error', function (done) {
+            stub.returns(false);
+
+            SubscribersAPI.importCSV(_.merge(testUtils.context.internal, {path: '/somewhere'}))
+                .then(function () {
+                    done(new Error('we expected an error here!'));
+                })
+                .catch(function (err) {
+                    err.message.should.eql('Please select a valid CSV file to import.');
+                    done();
+                });
+        });
     });
 });