From a230423d7f6206a85ab897dedab94d4fa58ba73a Mon Sep 17 00:00:00 2001 From: Brad Wilson Date: Tue, 25 Jun 2024 14:04:23 -0500 Subject: [PATCH 1/2] Issue 374: Allow an option to choose the hashing algorithm - hashAlgorithm key added to DEFAULT_OPTIONS - hashAlgorithm key can be overriden by value given in options argument - buildOptions function will ensure a default a hashAlgorithm exists, and throw an error if the provided option is not supported by the system. - throws early error during app setup, rather than waiting until an upload is attempted - memHandler and tempFileHandler use the hashAlgorithm option now - Updated tests for buildOptions function in utilities.spec.js ref: https://github.com/richardgirges/express-fileupload/issues/374 --- lib/index.js | 3 ++- lib/memHandler.js | 2 +- lib/tempFileHandler.js | 4 ++-- lib/utilities.js | 15 +++++++++++++++ test/utilities.spec.js | 23 +++++++++++++++-------- 5 files changed, 35 insertions(+), 12 deletions(-) diff --git a/lib/index.js b/lib/index.js index 5537c29..0908909 100644 --- a/lib/index.js +++ b/lib/index.js @@ -20,7 +20,8 @@ const DEFAULT_OPTIONS = { createParentPath: false, parseNested: false, useTempFiles: false, - tempFileDir: path.join(process.cwd(), 'tmp') + tempFileDir: path.join(process.cwd(), 'tmp'), + hashAlgorithm: 'md5' }; /** diff --git a/lib/memHandler.js b/lib/memHandler.js index 09accfe..a138484 100644 --- a/lib/memHandler.js +++ b/lib/memHandler.js @@ -10,7 +10,7 @@ const { debugLog } = require('./utilities'); */ module.exports = (options, fieldname, filename) => { const buffers = []; - const hash = crypto.createHash('md5'); + const hash = crypto.createHash(options.hashAlgorithm); let fileSize = 0; let completed = false; diff --git a/lib/tempFileHandler.js b/lib/tempFileHandler.js index d24416a..6751935 100644 --- a/lib/tempFileHandler.js +++ b/lib/tempFileHandler.js @@ -14,8 +14,8 @@ module.exports = (options, fieldname, filename) => { checkAndMakeDir({ createParentPath: true }, tempFilePath); debugLog(options, `Temporary file path is ${tempFilePath}`); - - const hash = crypto.createHash('md5'); + + const hash = crypto.createHash(options.hashAlgorithm); let fileSize = 0; let completed = false; diff --git a/lib/utilities.js b/lib/utilities.js index 7ecf94d..2cbf583 100644 --- a/lib/utilities.js +++ b/lib/utilities.js @@ -2,6 +2,7 @@ const fs = require('fs'); const path = require('path'); +const crypto = require('crypto'); const { Readable } = require('stream'); // Parameters for safe file name parsing. @@ -75,6 +76,20 @@ const buildOptions = function() { if (!options || typeof options !== 'object') return; Object.keys(options).forEach(i => result[i] = options[i]); }); + + // Ensure a hashAlgorithm option exists. + if (!result.hashAlgorithm) { + result.hashAlgorithm = 'md5'; + } + + // Ensure the configured hashAlgorithm is available on the system + if (crypto.getHashes().find(h => result.hashAlgorithm === h) === undefined) { + throw Error( + `Hashing algorithm '${result.hashAlgorithm}' is not supported by this system's OpenSSL ` + + `version` + ); + } + return result; }; diff --git a/test/utilities.spec.js b/test/utilities.spec.js index a9cc1b1..90380e0 100644 --- a/test/utilities.spec.js +++ b/test/utilities.spec.js @@ -199,25 +199,32 @@ describe('utilities: Test of the utilities functions', function() { describe('Test buildOptions function', () => { const source = { option1: '1', option2: '2' }; - const sourceAddon = { option3: '3'}; - const expected = { option1: '1', option2: '2' }; - const expectedAddon = { option1: '1', option2: '2', option3: '3'}; + const sourceAddon = { option3: '3', hashAlgorithm: 'sha256'}; + const expected = { option1: '1', option2: '2', hashAlgorithm: 'md5' }; + const expectedAddon = { option1: '1', option2: '2', option3: '3', hashAlgorithm: 'sha256'}; - it('buildOptions returns and equal object to the object which was paased', () => { - let result = buildOptions(source); - assert.deepStrictEqual(result, source); - }); + it( + 'buildOptions returns an equal object to the object which was passed + hashAlgorithm ' + + 'property', + () => { + let result = buildOptions(source); + assert.deepStrictEqual(result, expected); + } + ); it('buildOptions doesnt add non object or null arguments to the result', () => { let result = buildOptions(source, 2, '3', null); assert.deepStrictEqual(result, expected); }); - it('buildOptions adds value to the result from the several source argumets', () => { + it('buildOptions adds value to the result from the several source arguments', () => { let result = buildOptions(source, sourceAddon); assert.deepStrictEqual(result, expectedAddon); }); + it('buildOptions throws an error when given an unsupported hashAlgorithm', () => { + assert.throws(() => buildOptions({ hashAlgorithm: 'not-actual-algo' })); + }); }); //buildFields tests describe('Test buildFields function', () => { From 73496500ac6ef8dcf3772c0304a318ac11310727 Mon Sep 17 00:00:00 2001 From: Brad Wilson Date: Wed, 26 Jun 2024 09:27:55 -0500 Subject: [PATCH 2/2] PR 375: Suggested changes - `buildOptions` now throws an error if `hashAlgorithm` is not defined, instead of resetting to the default value of 'md5' - Moved + symbol for a string concatination to the beginning of the next line - Updated the README.md with information on the new hashAlgorithm option, and how the `md5` property is defined in the new version - Iterated the version in package.json from 1.5.0 -> 1.5.1 ref: https://github.com/richardgirges/express-fileupload/pull/375 --- README.md | 4 +++- lib/utilities.js | 10 +++------- package.json | 2 +- test/utilities.spec.js | 18 +++++++++--------- 4 files changed, 16 insertions(+), 18 deletions(-) diff --git a/README.md b/README.md index 31e1270..195ca54 100644 --- a/README.md +++ b/README.md @@ -50,7 +50,8 @@ The **req.files.foo** object will contain the following: * Before 1.0.0, `md5` is an MD5 checksum of the uploaded file. * From 1.0.0 until 1.1.1, `md5` is a function to compute an MD5 hash ([Read about it here.](https://github.com/richardgirges/express-fileupload/releases/tag/v1.0.0-alpha.1)). -* From 1.1.1 onward, `md5` is reverted back to MD5 checksum value and also added full MD5 support in case you are using temporary files. +* From 1.1.1 until 1.5.1, `md5` is reverted back to MD5 checksum value and also added full MD5 support in case you are using temporary files. +* From 1.5.1 onward, `md5` still holds the checksum value, but the checksum is generated with the provided `hashAlgorithm` option. The property name remains `md5` for backwards compatibility. ### Examples @@ -124,6 +125,7 @@ parseNested | | Turn on/off upload process logging. Can be useful for troubleshooting. logger |
  • console **(default)**
  • {log: function(msg: string)}
| Customizable logger to write debug messages to. Console is default. uploadTimeout |
  • 60000 **(default)**
  • Integer
| This defines how long to wait for data before aborting. Set to 0 if you want to turn off timeout checks. +hashAlgorithm |
  • md5 **(default)**
  • String
| Allows the usage of alternative hashing algorithms for file integrity checks. This option must be an algorithm that is supported on the running system's installed OpenSSL version. On recent releases of OpenSSL, openssl list -digest-algorithms will display the available digest algorithms. # Help Wanted Looking for additional maintainers. Please contact `richardgirges [ at ] gmail.com` if you're interested. Pull Requests are welcome! diff --git a/lib/utilities.js b/lib/utilities.js index 2cbf583..1545399 100644 --- a/lib/utilities.js +++ b/lib/utilities.js @@ -69,6 +69,7 @@ const promiseCallback = (resolve, reject) => { /** * Builds instance options from arguments objects(can't be arrow function). * @returns {Object} - result options. + * @throws {Error} - when a valid hashAlgorithm option is not provided. */ const buildOptions = function() { const result = {}; @@ -77,16 +78,11 @@ const buildOptions = function() { Object.keys(options).forEach(i => result[i] = options[i]); }); - // Ensure a hashAlgorithm option exists. - if (!result.hashAlgorithm) { - result.hashAlgorithm = 'md5'; - } - // Ensure the configured hashAlgorithm is available on the system if (crypto.getHashes().find(h => result.hashAlgorithm === h) === undefined) { throw Error( - `Hashing algorithm '${result.hashAlgorithm}' is not supported by this system's OpenSSL ` + - `version` + `Hashing algorithm '${result.hashAlgorithm}' is not supported by this system's OpenSSL ` + + `version` ); } diff --git a/package.json b/package.json index a5f2785..6360fbe 100644 --- a/package.json +++ b/package.json @@ -1,6 +1,6 @@ { "name": "express-fileupload", - "version": "1.5.0", + "version": "1.5.1", "author": "Richard Girges ", "description": "Simple express file upload middleware that wraps around Busboy", "main": "./lib/index", diff --git a/test/utilities.spec.js b/test/utilities.spec.js index 90380e0..1386df5 100644 --- a/test/utilities.spec.js +++ b/test/utilities.spec.js @@ -198,19 +198,15 @@ describe('utilities: Test of the utilities functions', function() { //buildOptions tests describe('Test buildOptions function', () => { - const source = { option1: '1', option2: '2' }; + const source = { option1: '1', option2: '2', hashAlgorithm: 'md5' }; const sourceAddon = { option3: '3', hashAlgorithm: 'sha256'}; const expected = { option1: '1', option2: '2', hashAlgorithm: 'md5' }; const expectedAddon = { option1: '1', option2: '2', option3: '3', hashAlgorithm: 'sha256'}; - it( - 'buildOptions returns an equal object to the object which was passed + hashAlgorithm ' - + 'property', - () => { - let result = buildOptions(source); - assert.deepStrictEqual(result, expected); - } - ); + it('buildOptions returns an equal object to the object which was passed', () => { + let result = buildOptions(source); + assert.deepStrictEqual(result, source); + }); it('buildOptions doesnt add non object or null arguments to the result', () => { let result = buildOptions(source, 2, '3', null); @@ -222,6 +218,10 @@ describe('utilities: Test of the utilities functions', function() { assert.deepStrictEqual(result, expectedAddon); }); + it('buildOptions throws an error when not provided a supported hashAlgorithm', () => { + assert.throws(() => buildOptions({})); + }); + it('buildOptions throws an error when given an unsupported hashAlgorithm', () => { assert.throws(() => buildOptions({ hashAlgorithm: 'not-actual-algo' })); });