fix(import): validate that attachment contents is a Buffer (#159)

Fixes: #158

BREAKING CHANGE: Import's `addAttachment` must now provide the contents as a Buffer or TypedArray. Previously, base64 strings were accepted, though undocumented.

src/import/ImportResult.js

@@ -63,7 +63,7 @@ module.exports = class ImportResult {
         'String',
         'In attachment: reference'
       );
-      assertDefined(this.attachments[i].contents, 'In attachment: contents');
+      assertTypedArray(this.attachments[i].contents, 'In attachment: contents');
     }
 
     // Required if an there is a jpath
@@ -107,13 +107,19 @@ module.exports = class ImportResult {
 
 function assertType(data, expectedType, errorPrefix) {
   if (getType(data) !== expectedType) {
-    throw new Error(`${errorPrefix || ''} should be ${expectedType}`);
+    throw new Error(`${errorPrefix || ''} must be of type ${expectedType}`);
   }
 }
 
 function assertDefined(data, errorPrefix) {
   if (data === undefined) {
-    throw new Error(`${errorPrefix || ''} should be defined`);
+    throw new Error(`${errorPrefix || ''} must be defined`);
+  }
+}
+
+function assertTypedArray(data, errorPrefix) {
+  if (!ArrayBuffer.isView(data)) {
+    throw new Error(`${errorPrefix || ''} must be a Buffer or TypedArray`);
   }
 }
 

src/import/saveResult.js

@@ -76,6 +76,11 @@ module.exports = async function saveResult(importBase, result) {
 
   // Upload additional attachments with metadata
   for (const attachment of result.attachments) {
+    const contents = Buffer.from(
+      attachment.contents.buffer,
+      attachment.contents.byteOffset,
+      attachment.contents.byteLength
+    );
     // eslint-disable-next-line no-await-in-loop
     await couch.addFileToJpath(
       result.id,
@@ -86,7 +91,7 @@ module.exports = async function saveResult(importBase, result) {
         field: attachment.field,
         reference: attachment.reference,
         name: `${attachment.jpath.join('/')}/${fold(attachment.filename, '_')}`,
-        data: attachment.contents,
+        data: contents,
         content_type: attachment.content_type
       }
     );

test/homeDirectories/main/test-new-import/separate/import.js

@@ -1,31 +1,40 @@
 'use strict';
 
-module.exports = async function nmrImport(ctx, result) {
-    result.kind = 'sample';
-    result.id = 'separate';
-    result.owner = 'a@a.com';
-    result.reference = ctx.filename;
-    result.skipAttachment();
-    result.skipMetadata();
-    result.field = 'field';
-    if(ctx.fileExt === '.txt') {
-        result.content_type = 'plain/text';
-    }
-    result.content = {
-        sideEffect: true
-    };
-    result.addAttachment({
-        jpath: ['other','jpath'],
-        metadata: {hasMetadata: true},
-        reference: 'testRef',
-        contents: await ctx.getContents(),
-        field: 'testField',
-        filename: ctx.filename,
-        content_type: 'plain/text'
-    });
-    result.metadata = {
-        hasMetadata: true
-    };
-    result.addGroup('group1');
-    result.addGroups(['group2', 'group3']);
+module.exports = async function separateImport(ctx, result) {
+  result.kind = 'sample';
+  result.id = 'separate';
+  result.owner = 'a@a.com';
+  result.reference = ctx.filename;
+  result.skipAttachment();
+  result.skipMetadata();
+  result.field = 'field';
+  if (ctx.fileExt === '.txt') {
+    result.content_type = 'text/plain';
+  }
+  result.content = {
+    sideEffect: true
+  };
+  result.addAttachment({
+    jpath: ['other', 'jpath'],
+    metadata: { hasMetadata: true },
+    reference: 'testRef',
+    contents: await ctx.getContents(),
+    field: 'testField',
+    filename: ctx.filename,
+    content_type: 'text/plain'
+  });
+  result.addAttachment({
+    jpath: ['other2', 'jpath'],
+    reference: 'ref2',
+    // 'test2'
+    contents: Uint8Array.of(116, 101, 115, 116, 50),
+    field: 'testField',
+    filename: 'test2.txt',
+    content_type: 'text/plain'
+  });
+  result.metadata = {
+    hasMetadata: true
+  };
+  result.addGroup('group1');
+  result.addGroups(['group2', 'group3']);
 };

test/import/import.js

@@ -109,9 +109,18 @@ describe('import', () => {
     expect(metadata.reference).toBe('testRef');
     expect(metadata.testField.filename).toBe('other/jpath/test.txt');
 
-    // check attachmentss
+    // check attachments
     const secondaryAttachment = data._attachments['other/jpath/test.txt'];
     expect(secondaryAttachment).toBeDefined();
-    expect(secondaryAttachment.content_type).toBe('plain/text');
+    expect(secondaryAttachment.content_type).toBe('text/plain');
+
+    const otherAttachment = data._attachments['other2/jpath/test2.txt'];
+    expect(otherAttachment).toBeDefined();
+    expect(otherAttachment.content_type).toBe('text/plain');
+    const attachmentData = await importCouch.getAttachmentByName(
+      data._id,
+      'other2/jpath/test2.txt'
+    );
+    expect(attachmentData.toString('utf8')).toBe('test2');
   });
 });

test/import/importResult.js

@@ -86,81 +86,87 @@ describe('ImportResult', () => {
     // Mandotory fields
     checkWithoutPropShouldThrow(
       'id',
-      'id should be defined',
+      'id must be defined',
       constants.IMPORT_UPDATE_FULL
     );
     checkWithoutPropShouldThrow(
       'kind',
-      'kind should be String',
+      'kind must be of type String',
       constants.IMPORT_UPDATE_FULL
     );
     checkWithoutPropShouldThrow(
       'owner',
-      'owner should be String',
+      'owner must be of type String',
       constants.IMPORT_UPDATE_FULL
     );
     checkWithoutPropShouldThrow(
       'groups',
-      'groups should be Array',
+      'groups must be of type Array',
       constants.IMPORT_UPDATE_FULL
     );
 
     // Additional attachments
     checkWithoutAttachmentPropShouldThrow(
       'filename',
-      'In attachment: filename should be String',
+      'In attachment: filename must be of type String',
       constants.IMPORT_UPDATE_FULL
     );
     checkWithoutAttachmentPropShouldThrow(
       'jpath',
-      'In attachment: jpath should be Array',
+      'In attachment: jpath must be of type Array',
       constants.IMPORT_UPDATE_FULL
     );
     checkWithoutAttachmentPropShouldThrow(
       'field',
-      'In attachment: field should be String',
+      'In attachment: field must be of type String',
       constants.IMPORT_UPDATE_FULL
     );
     checkWithoutAttachmentPropShouldThrow(
       'reference',
-      'In attachment: reference should be String',
+      'In attachment: reference must be of type String',
       constants.IMPORT_UPDATE_FULL
     );
     checkWithoutAttachmentPropShouldThrow(
-      'contents',
-      'In attachment: contents should be defined',
+      'content_type',
+      'In attachment: content_type must be of type String',
       constants.IMPORT_UPDATE_FULL
     );
     checkWithoutAttachmentPropShouldThrow(
-      'content_type',
-      'In attachment: content_type should be String',
+      'metadata',
+      'In attachment: metadata must be of type Object',
       constants.IMPORT_UPDATE_FULL
     );
     checkWithoutAttachmentPropShouldThrow(
-      'metadata',
-      'In attachment: metadata should be Object',
+      'contents',
+      'In attachment: contents must be a Buffer or TypedArray',
+      constants.IMPORT_UPDATE_FULL
+    );
+    checkWithWrongTypeAttachmentPropShouldThrow(
+      'contents',
+      'this is a string',
+      'In attachment: contents must be a Buffer or TypedArray',
       constants.IMPORT_UPDATE_FULL
     );
 
     // Full import
     checkWithoutPropShouldThrow(
       'content_type',
-      'content_type should be String',
+      'content_type must be of type String',
       constants.IMPORT_UPDATE_FULL
     );
     checkWithoutPropShouldThrow(
       'metadata',
-      'metadata should be Object',
+      'metadata must be of type Object',
       constants.IMPORT_UPDATE_FULL
     );
     checkWithoutPropShouldThrow(
       'field',
-      'field should be String',
+      'field must be of type String',
       constants.IMPORT_UPDATE_FULL
     );
     checkWithoutPropShouldThrow(
       'reference',
-      'reference should be String',
+      'reference must be of type String',
       constants.IMPORT_UPDATE_FULL
     );
 
@@ -170,45 +176,45 @@ describe('ImportResult', () => {
   test('throws when fields are missing - without attachment', () => {
     checkWithoutPropShouldThrow(
       'id',
-      'id should be defined',
+      'id must be defined',
       constants.IMPORT_UPDATE_WITHOUT_ATTACHMENT
     );
     checkWithoutPropShouldThrow(
       'kind',
-      'kind should be String',
+      'kind must be of type String',
       constants.IMPORT_UPDATE_WITHOUT_ATTACHMENT
     );
     checkWithoutPropShouldThrow(
       'owner',
-      'owner should be String',
+      'owner must be of type String',
       constants.IMPORT_UPDATE_WITHOUT_ATTACHMENT
     );
     checkWithoutPropShouldThrow(
       'groups',
-      'groups should be Array',
+      'groups must be of type Array',
       constants.IMPORT_UPDATE_WITHOUT_ATTACHMENT
     );
   });
 
-  test('throws when fields aer missing - content only', () => {
+  test('throws when fields are missing - content only', () => {
     checkWithoutPropShouldThrow(
       'id',
-      'id should be defined',
+      'id must be defined',
       constants.IMPORT_UPDATE_$CONTENT_ONLY
     );
     checkWithoutPropShouldThrow(
       'kind',
-      'kind should be String',
+      'kind must be of type String',
       constants.IMPORT_UPDATE_$CONTENT_ONLY
     );
     checkWithoutPropShouldThrow(
       'owner',
-      'owner should be String',
+      'owner must be of type String',
       constants.IMPORT_UPDATE_$CONTENT_ONLY
     );
     checkWithoutPropShouldThrow(
       'groups',
-      'groups should be Array',
+      'groups must be of type Array',
       constants.IMPORT_UPDATE_$CONTENT_ONLY
     );
   });
@@ -238,3 +244,16 @@ function checkWithoutAttachmentPropShouldThrow(prop, message, importType) {
     importResult.check();
   }).toThrowError(message);
 }
+
+function checkWithWrongTypeAttachmentPropShouldThrow(
+  prop,
+  value,
+  message,
+  importType
+) {
+  const importResult = getValidResult(importType);
+  importResult.attachments[0][prop] = value;
+  expect(function () {
+    importResult.check();
+  }).toThrowError(message);
+}