Skip to content

Commit

Permalink
fix: update file-type to resolve vulnerability (#205)
Browse files Browse the repository at this point in the history
The `file-type` package has a vulnerability that persists until v16.5.4. This
commit updates the package to v16.5.4 to avoid the vulnerability. However, the
package update required changes in how we use the package in our code which
resulted in incompatible updates to a couple of functions that are part of our
public API. Though it is unlikely these functions are being widely used, this
change will need to go into a new major version.

BREAKING CHANGE: two synchronous public functions are now asynchronous

The function `getContentType` formerly returned a string but now returns a
Promise that resolves to a string. The function `buildRequestFileObject`
formerly returned a `FileObject` but now returns a Promise that resolves to
a `FileObject`.

Fixes #204

Signed-off-by: Dustin Popp <dpopp07@gmail.com>
  • Loading branch information
dpopp07 authored Jul 28, 2022
1 parent c0e9cd8 commit 843e66d
Show file tree
Hide file tree
Showing 7 changed files with 233 additions and 78 deletions.
6 changes: 3 additions & 3 deletions etc/ibm-cloud-sdk-core.api.md
Original file line number Diff line number Diff line change
Expand Up @@ -94,7 +94,7 @@ export class BearerTokenAuthenticator extends Authenticator {
}

// @public
export function buildRequestFileObject(fileParam: FileWithMetadata): FileObject;
export function buildRequestFileObject(fileParam: FileWithMetadata): Promise<FileObject>;

// @public
export function checkCredentials(obj: any, credsToCheck: string[]): Error | null;
Expand Down Expand Up @@ -155,7 +155,7 @@ export class ContainerTokenManager extends IamRequestBasedTokenManager {

// @public (undocumented)
export const contentType: {
fromFilename: (file: String | File | Buffer | NodeJS.ReadableStream | FileObject) => string;
fromFilename: (file: String | File | FileObject | NodeJS.ReadableStream | Buffer) => string;
fromHeader: (buffer: Buffer) => string;
};

Expand Down Expand Up @@ -211,7 +211,7 @@ export interface FileWithMetadata {
export function getAuthenticatorFromEnvironment(serviceName: string): Authenticator;

// @public
export function getContentType(inputData: NodeJS.ReadableStream | Buffer): string;
export function getContentType(inputData: NodeJS.ReadableStream | Buffer): Promise<string>;

// @public
export function getCurrentTime(): number;
Expand Down
12 changes: 7 additions & 5 deletions lib/helper.ts
Original file line number Diff line number Diff line change
Expand Up @@ -14,12 +14,13 @@
* limitations under the License.
*/

import fileType from 'file-type';
import { isReadable } from 'isstream';
import { lookup } from 'mime-types';
import { basename } from 'path';
import logger from './logger';

const FileType = require('file-type');

export interface FileObject {
value: NodeJS.ReadableStream | Buffer | string;
options?: FileOptions;
Expand Down Expand Up @@ -67,15 +68,15 @@ export function isEmptyObject(obj: any): boolean {
* @param {NodeJS.ReadableStream|Buffer} inputData - The data to retrieve content type for.
* @returns {string} the content type of the input.
*/
export function getContentType(inputData: NodeJS.ReadableStream | Buffer): string {
export async function getContentType(inputData: NodeJS.ReadableStream | Buffer): Promise<string> {
let contentType = null;
if (isFileStream(inputData)) {
// if the inputData is a NodeJS.ReadableStream
const mimeType = lookup(inputData.path as any); // TODO: cleue quick hack, per type definition path could also be a Buffer
contentType = { mime: mimeType || null };
} else if (Buffer.isBuffer(inputData)) {
// if the inputData is a Buffer
contentType = fileType(inputData);
contentType = await FileType.fromBuffer(inputData);
}

return contentType ? contentType.mime : null;
Expand Down Expand Up @@ -239,7 +240,7 @@ export function getFormat(params: { [key: string]: any }, formats: string[]): st
* @param {string} fileParam.contentType The content type of the file.
* @returns {FileObject}
*/
export function buildRequestFileObject(fileParam: FileWithMetadata): FileObject {
export async function buildRequestFileObject(fileParam: FileWithMetadata): Promise<FileObject> {
let fileObj: FileObject;
if (isFileObject(fileParam.data)) {
// For backward compatibility, we allow the data to be a FileObject.
Expand Down Expand Up @@ -277,7 +278,8 @@ export function buildRequestFileObject(fileParam: FileWithMetadata): FileObject

// build contentType
if (!fileObj.options.contentType && isFileData(fileObj.value)) {
fileObj.options.contentType = getContentType(fileObj.value) || 'application/octet-stream';
fileObj.options.contentType =
(await getContentType(fileObj.value)) || 'application/octet-stream';
}

return fileObj;
Expand Down
4 changes: 2 additions & 2 deletions lib/request-wrapper.ts
Original file line number Diff line number Diff line change
Expand Up @@ -178,7 +178,7 @@ export class RequestWrapper {
// Skip keys with undefined/null values or empty object value
values
.filter((v) => v != null && !isEmptyObject(v))
.forEach((value) => {
.forEach(async (value) => {
// Special case of empty file object
if (
Object.prototype.hasOwnProperty.call(value, 'contentType') &&
Expand All @@ -188,7 +188,7 @@ export class RequestWrapper {
}

if (isFileWithMetadata(value)) {
const fileObj = buildRequestFileObject(value);
const fileObj = await buildRequestFileObject(value);
multipartForm.append(key, fileObj.value, fileObj.options);
} else {
if (typeof value === 'object' && !isFileData(value)) {
Expand Down
Loading

0 comments on commit 843e66d

Please sign in to comment.