Show error if selected CSV file contains null character (#982)

Author: matthew-white

src/components/entity/upload.vue

@@ -257,6 +257,7 @@ const parseEntities = async (file, headerResults, signal) => {
   warnings.value = results.warnings;
 };
 const selectFile = (file) => {
+  alert.blank();
   headerErrors.value = null;
   dataError.value = null;
 

src/locales/en.json5

@@ -436,6 +436,7 @@
     "csv": {
       // {message} is a description of the problem.
       "readError": "There was a problem reading your file: {message}",
+      "invalidCSV": "The file “{name}” is not a valid .csv file. It cannot be read.",
       // {row} is a row number. {message} is a description of the problem.
       "rowError": "There is a problem on row {row} of the file: {message}",
       // This is an error that is shown for a spreadsheet. The field may be any

src/util/csv.js

@@ -82,6 +82,12 @@ export const parseCSVHeader = async (i18n, file, signal = undefined) => {
     preview: 1
   });
   const columns = data.length !== 0 ? data[0] : [];
+  // Make a simple try at detecting a binary file by searching for a null
+  // character. We do that in order to avoid displaying unintelligible binary
+  // data to the user. Also, Backend would probably reject a null character
+  // that's sent to it.
+  if (columns.some(column => column.includes('\0')))
+    throw new Error(i18n.t('util.csv.invalidCSV', { name: file.name }));
   const unhandledErrors = errors.filter(error =>
     error.code !== 'UndetectableDelimiter');
   if (unhandledErrors.length === 0) {
@@ -200,6 +206,12 @@ export const parseCSV = async (i18n, file, columns, options = {}) => {
       throw new Error(i18n.tc('util.csv.dataWithoutHeader', columns.length, counts));
     }
 
+    if (values.some(value => value.includes('\0'))) {
+      const error = new Error(i18n.t('util.csv.invalidCSV', { name: file.name }));
+      error.row = NaN;
+      throw error;
+    }
+
     data.push(transformRow(values, columns));
     for (const warning of warnings) warning.pushRow(values, index, columns);
   };
@@ -235,6 +247,9 @@ export const parseCSV = async (i18n, file, columns, options = {}) => {
       worker: true
     });
   } catch (error) {
+    // Mention the row number in the error message unless the `row` property of
+    // the error has been set to NaN.
+    if (Number.isNaN(error.row)) throw error;
     throw new Error(i18n.t('util.csv.rowError', {
       message: error.message,
       row: i18n.n((error.row ?? rowIndex) + 1, 'default')

test/components/entity/upload.spec.js

@@ -177,6 +177,38 @@ describe('EntityUpload', () => {
     });
   });
 
+  describe('binary file', () => {
+    beforeEach(() => {
+      testData.extendedDatasets.createPast(1);
+    });
+
+    it('shows an alert for a null character in the header', async () => {
+      const modal = await showModal();
+      await selectFile(modal, createCSV('f\0o'));
+      modal.should.alert('danger', 'The file “my_data.csv” is not a valid .csv file. It cannot be read.');
+      modal.findComponent(EntityUploadHeaderErrors).exists().should.be.false();
+    });
+
+    it('hides the alert after a valid file is selected', async () => {
+      const modal = await showModal();
+      await selectFile(modal, createCSV('f\0o'));
+      await selectFile(modal);
+      modal.should.not.alert();
+      modal.findComponent(EntityUploadPopup).exists().should.be.true();
+    });
+
+    // This is not necessarily the ideal behavior. Showing an alert would be
+    // more consistent with what happens for a null character in the header.
+    // This test documents the current expected behavior.
+    it('renders EntityUploadDataError for a null character after header', async () => {
+      const modal = await showModal();
+      await selectFile(modal, createCSV('label\nf\0o'));
+      const { message } = modal.getComponent(EntityUploadDataError).props();
+      message.should.equal('The file “my_data.csv” is not a valid .csv file. It cannot be read.');
+      modal.should.not.alert();
+    });
+  });
+
   describe('warnings', () => {
     beforeEach(() => {
       testData.extendedDatasets.createPast(1, {

test/unit/csv.spec.js

@@ -48,7 +48,12 @@ describe('util/csv', () => {
       });
     });
 
-    describe('error', () => {
+    it('returns a rejected promise if there is a null character', () => {
+      const promise = parseCSVHeader(i18n, createCSV('f\0o,bar'));
+      return promise.should.be.rejectedWith('The file “my_data.csv” is not a valid .csv file. It cannot be read.');
+    });
+
+    describe('Papa Parse error', () => {
       it('returns an error for a missing quote', async () => {
         const { errors } = await parseCSVHeader(i18n, createCSV('"a\n1'));
         errors.length.should.equal(1);
@@ -119,6 +124,11 @@ describe('util/csv', () => {
       });
     });
 
+    it('returns a rejected promise if there is a null character', () => {
+      const promise = parseCSV(i18n, createCSV('a\nf\0o'), ['a']);
+      return promise.should.be.rejectedWith('The file “my_data.csv” is not a valid .csv file. It cannot be read.');
+    });
+
     describe('number of cells', () => {
       it('allows a row to be ragged', async () => {
         const csv = createCSV('a,b\n1,2\n3');

transifex/strings_en.json

@@ -918,6 +918,9 @@
         "string": "There was a problem reading your file: {message}",
         "developer_comment": "{message} is a description of the problem."
       },
+      "invalidCSV": {
+        "string": "The file “{name}” is not a valid .csv file. It cannot be read."
+      },
       "rowError": {
         "string": "There is a problem on row {row} of the file: {message}",
         "developer_comment": "{row} is a row number. {message} is a description of the problem."