Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Merged
merged 3 commits into from
Oct 5, 2018
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 9 additions & 3 deletions src/import/ImportResult.js
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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`);
}
}

Expand Down
7 changes: 6 additions & 1 deletion src/import/saveResult.js
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -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
}
);
Expand Down
65 changes: 37 additions & 28 deletions test/homeDirectories/main/test-new-import/separate/import.js
Original file line number Diff line number Diff line change
@@ -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']);
};
13 changes: 11 additions & 2 deletions test/import/import.js
Original file line number Diff line number Diff line change
Expand Up @@ -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');
});
});
73 changes: 46 additions & 27 deletions test/import/importResult.js
Original file line number Diff line number Diff line change
Expand Up @@ -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
);

Expand All @@ -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
);
});
Expand Down Expand Up @@ -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);
}