Skip to content

Commit

Permalink
fix: Fix handling of array form parameters. (#43)
Browse files Browse the repository at this point in the history
  • Loading branch information
Mike Kistler authored Sep 16, 2019
1 parent ae3cc69 commit bad8960
Show file tree
Hide file tree
Showing 4 changed files with 59 additions and 47 deletions.
15 changes: 14 additions & 1 deletion lib/base_service.ts
Original file line number Diff line number Diff line change
Expand Up @@ -338,7 +338,20 @@ export class BaseService {
* are being used to authenticate the request. If so, the token is
* retrieved by the token manager.
*
* @param {Object} parameters - service request options passed in by user
* @param {Object} parameters - service request options passed in by user.
* @param {string} parameters.options.method - the http method.
* @param {string} parameters.options.url - the URL of the service.
* @param {string} parameters.options.path - the path to be appended to the service URL.
* @param {string} parameters.options.qs - the querystring to be included in the URL.
* @param {string} parameters.options.body - the data to be sent as the request body.
* @param {Object} parameters.options.form - an object containing the key/value pairs for a www-form-urlencoded request.
* @param {Object} parameters.options.formData - an object containing the contents for a multipart/form-data request.
* The following processing is performed on formData values:
* - string: no special processing -- the value is sent as is
* - object: the value is converted to a JSON string before insertion into the form body
* - NodeJS.ReadableStream|FileObject|Buffer|FileParamAttributes: sent as a file, with any associated metadata
* - array: each element of the array is sent as a separate form part using any special processing as described above
* @param {HeaderOptions} parameters.options.headers - additional headers to be passed on the request.
* @param {Function} callback - callback function to pass the response back to
* @returns {ReadableStream|undefined}
*/
Expand Down
14 changes: 12 additions & 2 deletions lib/helper.ts
Original file line number Diff line number Diff line change
Expand Up @@ -43,22 +43,31 @@ export interface FileStream extends NodeJS.ReadableStream {
}

// custom type guards
function isFileObject(obj: any): obj is FileObject {
export function isFileObject(obj: any): obj is FileObject {
return Boolean(obj && obj.value);
}

function isFileStream(obj: any): obj is FileStream {
return obj && isReadable(obj) && obj.path;
}

export function isFileParamAttributes(obj: any): obj is FileParamAttributes {
return obj && obj.data &&
(
isReadable(obj.data) ||
Buffer.isBuffer(obj.data) ||
isFileObject(obj.data)
);
}

export function isFileParam(obj: any): boolean {
return Boolean(
obj &&
(
isReadable(obj) ||
Buffer.isBuffer(obj) ||
isFileObject(obj) ||
(obj.data && isFileParam(obj.data))
isFileParamAttributes(obj)
)
);
}
Expand Down Expand Up @@ -164,6 +173,7 @@ export function getFormat(
* this function builds a `form-data` object for each file parameter
* @param {FileParamAttributes} fileParams - the file parameter attributes
* @param {NodeJS.ReadableStream|Buffer|FileObject} fileParams.data - the data content of the file
* @param (string) fileParams.filename - the filename of the file
* @param {string} fileParams.contentType - the content type of the file
* @returns {FileObject}
*/
Expand Down
66 changes: 24 additions & 42 deletions lib/requestwrapper.ts
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ import FormData = require('form-data');
import https = require('https');
import querystring = require('querystring');
import { PassThrough as readableStream } from 'stream';
import { buildRequestFileObject, getMissingParams, isEmptyObject, isFileParam } from './helper';
import { buildRequestFileObject, getMissingParams, isEmptyObject, isFileObject, isFileParam, isFileParamAttributes } from './helper';

const isBrowser = typeof window === 'object';
const globalTransactionId = 'x-global-transaction-id';
Expand Down Expand Up @@ -127,48 +127,30 @@ export class RequestWrapper {

// Form params
if (formData) {
// Remove keys with undefined/null values
// Remove empty objects
// Remove non-valid inputs for buildRequestFileObject,
// i.e things like {contentType: <contentType>}
Object.keys(formData).forEach(key => {
if (formData[key] == null ||
isEmptyObject(formData[key]) ||
(formData[key].hasOwnProperty('contentType') && !formData[key].hasOwnProperty('data'))) {
delete formData[key];
}
});
// Convert file form parameters to request-style objects
Object.keys(formData).forEach(key => {
if (formData[key].data != null) {
formData[key] = buildRequestFileObject(formData[key]);
}
});

// Stringify arrays
Object.keys(formData).forEach(key => {
if (Array.isArray(formData[key])) {
formData[key] = formData[key].join(',');
}
});

// Convert non-file form parameters to strings
Object.keys(formData).forEach(key => {
if (!isFileParam(formData[key]) &&
!Array.isArray(formData[key]) &&
typeof formData[key] === 'object') {
(formData[key] = JSON.stringify(formData[key]));
}
});

// build multipart form data
Object.keys(formData).forEach(key => {
// handle files differently to maintain options
if (formData[key].value) {
multipartForm.append(key, formData[key].value, formData[key].options);
} else {
multipartForm.append(key, formData[key]);
}
const values = Array.isArray(formData[key]) ? formData[key] : [formData[key]];
// Skip keys with undefined/null values or empty object value
values.filter(v => v != null && !isEmptyObject(v)).forEach(value => {

// Special case of empty file object
if (value.hasOwnProperty('contentType') && !value.hasOwnProperty('data')) {
return;
}

// Convert file form parameters to request-style objects
if (isFileParamAttributes(value)) {
value = buildRequestFileObject(value);
}

if (isFileObject(value)) {
multipartForm.append(key, value.value, value.options);
} else {
if (typeof value === 'object' && !isFileParam(value)) {
value = JSON.stringify(value);
}
multipartForm.append(key, value);
}
});
});
}

Expand Down
11 changes: 9 additions & 2 deletions test/unit/requestWrapper.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -200,11 +200,11 @@ describe('sendRequest', () => {
'add-header': 'add-header-value',
},
formData: {
file: fs.createReadStream('../blank.wav'),
file: fs.createReadStream(__dirname + '/../resources/blank.wav'),
null_item: null,
custom_file: {
filename: 'custom.wav',
data: fs.createReadStream('../blank.wav'),
data: fs.createReadStream(__dirname + '/../resources/blank.wav'),
},
array_item: ['a', 'b'],
object_item: { a: 'a', b: 'b' },
Expand Down Expand Up @@ -242,6 +242,13 @@ describe('sendRequest', () => {
expect(JSON.stringify(mockAxiosInstance.mock.calls[0][0])).toMatch(
'Content-Disposition: form-data; name=\\"array_item\\"'
);
// There should be two "array_item" parts
expect(
(
JSON.stringify(mockAxiosInstance.mock.calls[0][0].data).match(/name=\\"array_item\\"/g) ||
[]
).length
).toEqual(2);
expect(JSON.stringify(mockAxiosInstance.mock.calls[0][0])).toMatch(
'Content-Disposition: form-data; name=\\"custom_file\\"'
);
Expand Down

0 comments on commit bad8960

Please sign in to comment.