From 4a9ed7ef7e447accd6b831a11bc02f59f5d20838 Mon Sep 17 00:00:00 2001 From: Phillip9587 Date: Tue, 22 Oct 2024 22:39:10 +0200 Subject: [PATCH 1/4] extract shared utility functions --- HISTORY.md | 1 + lib/types/json.js | 30 +----------------------- lib/types/raw.js | 14 +---------- lib/types/text.js | 30 +----------------------- lib/types/urlencoded.js | 30 +----------------------- lib/utils.js | 51 +++++++++++++++++++++++++++++++++++++++++ 6 files changed, 56 insertions(+), 100 deletions(-) create mode 100644 lib/utils.js diff --git a/HISTORY.md b/HISTORY.md index f889cf1c..524d0f96 100644 --- a/HISTORY.md +++ b/HISTORY.md @@ -11,6 +11,7 @@ 2.0.2 / 2024-10-31 ========================= +* extract shared utility functions * remove `unpipe` package and use native `unpipe()` method 2.0.1 / 2024-09-10 diff --git a/lib/types/json.js b/lib/types/json.js index 30bf8cab..5b4a5ff3 100644 --- a/lib/types/json.js +++ b/lib/types/json.js @@ -13,12 +13,12 @@ */ var bytes = require('bytes') -var contentType = require('content-type') var createError = require('http-errors') var debug = require('debug')('body-parser:json') var isFinished = require('on-finished').isFinished var read = require('../read') var typeis = require('type-is') +var { getCharset, typeChecker } = require('../utils') /** * Module exports. @@ -196,21 +196,6 @@ function firstchar (str) { : undefined } -/** - * Get the charset of a request. - * - * @param {object} req - * @api private - */ - -function getCharset (req) { - try { - return (contentType.parse(req).parameters.charset || '').toLowerCase() - } catch (e) { - return undefined - } -} - /** * Normalize a SyntaxError for JSON.parse. * @@ -235,16 +220,3 @@ function normalizeJsonSyntaxError (error, obj) { return error } - -/** - * Get the simple type checker. - * - * @param {string} type - * @return {function} - */ - -function typeChecker (type) { - return function checkType (req) { - return Boolean(typeis(req, type)) - } -} diff --git a/lib/types/raw.js b/lib/types/raw.js index bfe274cf..f3931116 100644 --- a/lib/types/raw.js +++ b/lib/types/raw.js @@ -15,6 +15,7 @@ var debug = require('debug')('body-parser:raw') var isFinished = require('on-finished').isFinished var read = require('../read') var typeis = require('type-is') +var { typeChecker } = require('../utils') /** * Module exports. @@ -89,16 +90,3 @@ function raw (options) { }) } } - -/** - * Get the simple type checker. - * - * @param {string} type - * @return {function} - */ - -function typeChecker (type) { - return function checkType (req) { - return Boolean(typeis(req, type)) - } -} diff --git a/lib/types/text.js b/lib/types/text.js index b153931b..8cbd3338 100644 --- a/lib/types/text.js +++ b/lib/types/text.js @@ -11,11 +11,11 @@ */ var bytes = require('bytes') -var contentType = require('content-type') var debug = require('debug')('body-parser:text') var isFinished = require('on-finished').isFinished var read = require('../read') var typeis = require('type-is') +var { getCharset, typeChecker } = require('../utils') /** * Module exports. @@ -94,31 +94,3 @@ function text (options) { }) } } - -/** - * Get the charset of a request. - * - * @param {object} req - * @api private - */ - -function getCharset (req) { - try { - return (contentType.parse(req).parameters.charset || '').toLowerCase() - } catch (e) { - return undefined - } -} - -/** - * Get the simple type checker. - * - * @param {string} type - * @return {function} - */ - -function typeChecker (type) { - return function checkType (req) { - return Boolean(typeis(req, type)) - } -} diff --git a/lib/types/urlencoded.js b/lib/types/urlencoded.js index 687745f8..a974f34c 100644 --- a/lib/types/urlencoded.js +++ b/lib/types/urlencoded.js @@ -13,13 +13,13 @@ */ var bytes = require('bytes') -var contentType = require('content-type') var createError = require('http-errors') var debug = require('debug')('body-parser:urlencoded') var isFinished = require('on-finished').isFinished var read = require('../read') var typeis = require('type-is') var qs = require('qs') +var { getCharset, typeChecker } = require('../utils') /** * Module exports. @@ -184,21 +184,6 @@ function createQueryParser (options, extended) { } } -/** - * Get the charset of a request. - * - * @param {object} req - * @api private - */ - -function getCharset (req) { - try { - return (contentType.parse(req).parameters.charset || '').toLowerCase() - } catch (e) { - return undefined - } -} - /** * Count the number of parameters, stopping once limit reached * @@ -222,16 +207,3 @@ function parameterCount (body, limit) { return count } - -/** - * Get the simple type checker. - * - * @param {string} type - * @return {function} - */ - -function typeChecker (type) { - return function checkType (req) { - return Boolean(typeis(req, type)) - } -} diff --git a/lib/utils.js b/lib/utils.js new file mode 100644 index 00000000..3e97f78f --- /dev/null +++ b/lib/utils.js @@ -0,0 +1,51 @@ +/*! + * body-parser + * Copyright(c) 2014-2015 Douglas Christopher Wilson + * MIT Licensed + */ + +'use strict' + +/** + * Module dependencies. + */ + +var contentType = require('content-type') +var typeis = require('type-is') + +/** + * Module exports. + */ + +module.exports = { + getCharset, + typeChecker +} + +/** + * Get the charset of a request. + * + * @param {object} req + * @api private + */ + +function getCharset (req) { + try { + return (contentType.parse(req).parameters.charset || '').toLowerCase() + } catch (e) { + return undefined + } +} + +/** + * Get the simple type checker. + * + * @param {string} type + * @return {function} + */ + +function typeChecker (type) { + return function checkType (req) { + return Boolean(typeis(req, type)) + } +} From d93b76d1fc1b67ffbd60bf4f585ea18cfb62d646 Mon Sep 17 00:00:00 2001 From: Phillip9587 Date: Tue, 22 Oct 2024 23:09:58 +0200 Subject: [PATCH 2/4] refactor: normalize common options for all parsers --- HISTORY.md | 6 +- lib/types/json.js | 19 +------ lib/types/raw.js | 20 +------ lib/types/text.js | 19 +------ lib/types/urlencoded.js | 19 +------ lib/utils.js | 44 +++++++++++++- test/utils.js | 123 ++++++++++++++++++++++++++++++++++++++++ 7 files changed, 177 insertions(+), 73 deletions(-) create mode 100644 test/utils.js diff --git a/HISTORY.md b/HISTORY.md index 524d0f96..14f4f192 100644 --- a/HISTORY.md +++ b/HISTORY.md @@ -1,3 +1,8 @@ +unreleased +========================= + + * refactor: normalize common options for all parsers + 2.1.0 / 2025-02-10 ========================= @@ -11,7 +16,6 @@ 2.0.2 / 2024-10-31 ========================= -* extract shared utility functions * remove `unpipe` package and use native `unpipe()` method 2.0.1 / 2024-09-10 diff --git a/lib/types/json.js b/lib/types/json.js index 5b4a5ff3..4647c0a5 100644 --- a/lib/types/json.js +++ b/lib/types/json.js @@ -12,13 +12,12 @@ * @private */ -var bytes = require('bytes') var createError = require('http-errors') var debug = require('debug')('body-parser:json') var isFinished = require('on-finished').isFinished var read = require('../read') var typeis = require('type-is') -var { getCharset, typeChecker } = require('../utils') +var { getCharset, normalizeOptions } = require('../utils') /** * Module exports. @@ -53,24 +52,10 @@ var JSON_SYNTAX_REGEXP = /#+/g function json (options) { var opts = options || {} + var { inflate, limit, verify, shouldParse } = normalizeOptions(opts, 'application/json') - var limit = typeof opts.limit !== 'number' - ? bytes.parse(opts.limit || '100kb') - : opts.limit - var inflate = opts.inflate !== false var reviver = opts.reviver var strict = opts.strict !== false - var type = opts.type || 'application/json' - var verify = opts.verify || false - - if (verify !== false && typeof verify !== 'function') { - throw new TypeError('option verify must be function') - } - - // create the appropriate type checking function - var shouldParse = typeof type !== 'function' - ? typeChecker(type) - : type function parse (body) { if (body.length === 0) { diff --git a/lib/types/raw.js b/lib/types/raw.js index f3931116..611ac0c2 100644 --- a/lib/types/raw.js +++ b/lib/types/raw.js @@ -10,12 +10,11 @@ * Module dependencies. */ -var bytes = require('bytes') var debug = require('debug')('body-parser:raw') var isFinished = require('on-finished').isFinished var read = require('../read') var typeis = require('type-is') -var { typeChecker } = require('../utils') +var { normalizeOptions } = require('../utils') /** * Module exports. @@ -33,22 +32,7 @@ module.exports = raw function raw (options) { var opts = options || {} - - var inflate = opts.inflate !== false - var limit = typeof opts.limit !== 'number' - ? bytes.parse(opts.limit || '100kb') - : opts.limit - var type = opts.type || 'application/octet-stream' - var verify = opts.verify || false - - if (verify !== false && typeof verify !== 'function') { - throw new TypeError('option verify must be function') - } - - // create the appropriate type checking function - var shouldParse = typeof type !== 'function' - ? typeChecker(type) - : type + var { inflate, limit, verify, shouldParse } = normalizeOptions(opts, 'application/octet-stream') function parse (buf) { return buf diff --git a/lib/types/text.js b/lib/types/text.js index 8cbd3338..3b51060d 100644 --- a/lib/types/text.js +++ b/lib/types/text.js @@ -10,12 +10,11 @@ * Module dependencies. */ -var bytes = require('bytes') var debug = require('debug')('body-parser:text') var isFinished = require('on-finished').isFinished var read = require('../read') var typeis = require('type-is') -var { getCharset, typeChecker } = require('../utils') +var { getCharset, normalizeOptions } = require('../utils') /** * Module exports. @@ -33,23 +32,9 @@ module.exports = text function text (options) { var opts = options || {} + var { inflate, limit, verify, shouldParse } = normalizeOptions(opts, 'text/plain') var defaultCharset = opts.defaultCharset || 'utf-8' - var inflate = opts.inflate !== false - var limit = typeof opts.limit !== 'number' - ? bytes.parse(opts.limit || '100kb') - : opts.limit - var type = opts.type || 'text/plain' - var verify = opts.verify || false - - if (verify !== false && typeof verify !== 'function') { - throw new TypeError('option verify must be function') - } - - // create the appropriate type checking function - var shouldParse = typeof type !== 'function' - ? typeChecker(type) - : type function parse (buf) { return buf diff --git a/lib/types/urlencoded.js b/lib/types/urlencoded.js index a974f34c..725fec2e 100644 --- a/lib/types/urlencoded.js +++ b/lib/types/urlencoded.js @@ -12,14 +12,13 @@ * @private */ -var bytes = require('bytes') var createError = require('http-errors') var debug = require('debug')('body-parser:urlencoded') var isFinished = require('on-finished').isFinished var read = require('../read') var typeis = require('type-is') var qs = require('qs') -var { getCharset, typeChecker } = require('../utils') +var { getCharset, normalizeOptions } = require('../utils') /** * Module exports. @@ -37,21 +36,12 @@ module.exports = urlencoded function urlencoded (options) { var opts = options || {} + var { inflate, limit, verify, shouldParse } = normalizeOptions(opts, 'application/x-www-form-urlencoded') var extended = Boolean(opts.extended) - var inflate = opts.inflate !== false - var limit = typeof opts.limit !== 'number' - ? bytes.parse(opts.limit || '100kb') - : opts.limit - var type = opts.type || 'application/x-www-form-urlencoded' - var verify = opts.verify || false var charsetSentinel = opts.charsetSentinel var interpretNumericEntities = opts.interpretNumericEntities - if (verify !== false && typeof verify !== 'function') { - throw new TypeError('option verify must be function') - } - var defaultCharset = opts.defaultCharset || 'utf-8' if (defaultCharset !== 'utf-8' && defaultCharset !== 'iso-8859-1') { throw new TypeError('option defaultCharset must be either utf-8 or iso-8859-1') @@ -60,11 +50,6 @@ function urlencoded (options) { // create the appropriate query parser var queryparse = createQueryParser(opts, extended) - // create the appropriate type checking function - var shouldParse = typeof type !== 'function' - ? typeChecker(type) - : type - function parse (body, encoding) { return body.length ? queryparse(body, encoding) diff --git a/lib/utils.js b/lib/utils.js index 3e97f78f..c7468168 100644 --- a/lib/utils.js +++ b/lib/utils.js @@ -1,6 +1,6 @@ /*! * body-parser - * Copyright(c) 2014-2015 Douglas Christopher Wilson + * Copyright(c) Express Contributors * MIT Licensed */ @@ -10,6 +10,7 @@ * Module dependencies. */ +var bytes = require('bytes') var contentType = require('content-type') var typeis = require('type-is') @@ -19,7 +20,7 @@ var typeis = require('type-is') module.exports = { getCharset, - typeChecker + normalizeOptions } /** @@ -32,7 +33,7 @@ module.exports = { function getCharset (req) { try { return (contentType.parse(req).parameters.charset || '').toLowerCase() - } catch (e) { + } catch { return undefined } } @@ -49,3 +50,40 @@ function typeChecker (type) { return Boolean(typeis(req, type)) } } + +/** + * Normalizes the common options for all parsers. + * + * @param {object} options options to normalize + * @param {string} defaultType default content type + * @returns {object} + */ +function normalizeOptions (options, defaultType) { + if (!defaultType || typeof defaultType !== 'string') { + // Parsers must define a default content type + throw new TypeError('defaultType must be provided') + } + + var inflate = options?.inflate !== false + var limit = typeof options?.limit !== 'number' + ? bytes.parse(options?.limit || '100kb') + : options?.limit + var type = options?.type || defaultType + var verify = options?.verify || false + + if (verify !== false && typeof verify !== 'function') { + throw new TypeError('option verify must be function') + } + + // create the appropriate type checking function + var shouldParse = typeof type !== 'function' + ? typeChecker(type) + : type + + return { + inflate, + limit, + verify, + shouldParse + } +} diff --git a/test/utils.js b/test/utils.js new file mode 100644 index 00000000..373b49c3 --- /dev/null +++ b/test/utils.js @@ -0,0 +1,123 @@ +'use strict' + +const assert = require('node:assert') +const { normalizeOptions } = require('../lib/utils.js') + +describe('normalizeOptions(options, defaultType)', () => { + it('should return default options when no options are provided', () => { + for (const options of [undefined, null, {}]) { + const result = normalizeOptions(options, 'application/json') + assert.strictEqual(result.inflate, true) + assert.strictEqual(result.limit, 100 * 1024) // 100kb in bytes + assert.strictEqual(result.verify, false) + assert.strictEqual(typeof result.shouldParse, 'function') + } + }) + + it('should override default options with provided options', () => { + const options = { + inflate: false, + limit: '200kb', + type: 'application/xml', + verify: () => {} + } + const result = normalizeOptions(options, 'application/json') + assert.strictEqual(result.inflate, false) + assert.strictEqual(result.limit, 200 * 1024) // 200kb in bytes + assert.strictEqual(result.verify, options.verify) + assert.strictEqual(typeof result.shouldParse, 'function') + }) + + it('should remove additional options', () => { + const options = { + inflate: false, + limit: '200kb', + type: 'application/xml', + verify: () => {}, + additional: 'option', + something: 'weird' + } + const result = normalizeOptions(options, 'application/json') + assert.strictEqual(result.inflate, false) + assert.strictEqual(result.limit, 200 * 1024) // 200kb in bytes + assert.strictEqual(result.verify, options.verify) + assert.strictEqual(typeof result.shouldParse, 'function') + assert.strictEqual(result.additional, undefined) + assert.strictEqual(result.something, undefined) + }) + + describe('options', () => { + describe('verify', () => { + it('should throw an error if verify is not a function', () => { + assert.throws(() => { + normalizeOptions({ verify: 'not a function' }, 'application/json') + }, /option verify must be function/) + }) + + it('should accept a verify function', () => { + const verify = () => {} + const result = normalizeOptions({ verify }, 'application/json') + assert.strictEqual(result.verify, verify) + }) + }) + + describe('limit', () => { + it('should return the default limit if limit is not provided', () => { + const result = normalizeOptions({}, 'application/json') + assert.strictEqual(result.limit, 100 * 1024) // 100kb in bytes + }) + + it('should accept a number limit', () => { + const result = normalizeOptions({ limit: 1234 }, 'application/json') + assert.strictEqual(result.limit, 1234) + }) + + it('should parse a string limit', () => { + const result = normalizeOptions({ limit: '200kb' }, 'application/json') + assert.strictEqual(result.limit, 200 * 1024) // 200kb in bytes + }) + + it('should return null for an invalid limit', () => { + const result = normalizeOptions({ limit: 'invalid' }, 'application/json') + assert.strictEqual(result.limit, null) + }) + }) + + describe('type', () => { + it('should return the default type if type is not provided', () => { + const result = normalizeOptions({}, 'application/json') + assert.strictEqual(result.shouldParse({ headers: { 'content-type': 'application/json', 'content-length': '1024' } }), true) + assert.strictEqual(result.shouldParse({ headers: { 'content-type': 'application/xml', 'content-length': '1024' } }), false) + }) + + it('should accept a string type', () => { + const result = normalizeOptions({ type: 'application/xml' }, 'application/json') + assert.strictEqual(result.shouldParse({ headers: { 'content-type': 'application/xml', 'content-length': '1024' } }), true) + assert.strictEqual(result.shouldParse({ headers: { 'content-type': 'application/json', 'content-length': '1024' } }), false) + }) + + it('should accept a type checking function', () => { + const result = normalizeOptions({ type: () => true }, 'application/json') + assert.strictEqual(result.shouldParse({ headers: { 'content-type': 'application/xml' } }), true) + assert.strictEqual(result.shouldParse({ headers: { 'content-type': 'application/json' } }), true) + }) + }) + }) + + describe('defaultType', () => { + it('should throw an error if defaultType is not provided', () => { + assert.throws(() => { + normalizeOptions({}) + }, /defaultType must be provided/) + assert.throws(() => { + normalizeOptions({}, undefined) + }, /defaultType must be provided/) + }) + + it('should throw an error if defaultType is not a string', () => { + assert.throws(() => { + normalizeOptions({}, 123) + }, /defaultType must be provided/) + }) + }) +}) From dda71697b791d6a88a496607ea461186186dbb54 Mon Sep 17 00:00:00 2001 From: Phillip9587 Date: Wed, 19 Feb 2025 22:16:58 +0100 Subject: [PATCH 3/4] refactor: apply suggestions from @ctpip and @jonchurch --- lib/utils.js | 12 +++--------- test/utils.js | 32 ++++++++++++++++++++++++++++---- 2 files changed, 31 insertions(+), 13 deletions(-) diff --git a/lib/utils.js b/lib/utils.js index c7468168..2a9ec8c0 100644 --- a/lib/utils.js +++ b/lib/utils.js @@ -1,9 +1,3 @@ -/*! - * body-parser - * Copyright(c) Express Contributors - * MIT Licensed - */ - 'use strict' /** @@ -41,7 +35,7 @@ function getCharset (req) { /** * Get the simple type checker. * - * @param {string} type + * @param {string | string[]} type * @return {function} */ @@ -55,11 +49,11 @@ function typeChecker (type) { * Normalizes the common options for all parsers. * * @param {object} options options to normalize - * @param {string} defaultType default content type + * @param {string | string[] | function} defaultType default content type * @returns {object} */ function normalizeOptions (options, defaultType) { - if (!defaultType || typeof defaultType !== 'string') { + if (!defaultType) { // Parsers must define a default content type throw new TypeError('defaultType must be provided') } diff --git a/test/utils.js b/test/utils.js index 373b49c3..ee6df5d8 100644 --- a/test/utils.js +++ b/test/utils.js @@ -96,6 +96,13 @@ describe('normalizeOptions(options, defaultType)', () => { assert.strictEqual(result.shouldParse({ headers: { 'content-type': 'application/json', 'content-length': '1024' } }), false) }) + it('should accept an array of strings type', () => { + const result = normalizeOptions({ type: ['application/xml', 'application/*+json'] }, 'application/json') + assert.strictEqual(result.shouldParse({ headers: { 'content-type': 'application/xml', 'content-length': '1024' } }), true) + assert.strictEqual(result.shouldParse({ headers: { 'content-type': 'application/ld+json', 'content-length': '1024' } }), true) + assert.strictEqual(result.shouldParse({ headers: { 'content-type': 'application/json', 'content-length': '1024' } }), false) + }) + it('should accept a type checking function', () => { const result = normalizeOptions({ type: () => true }, 'application/json') assert.strictEqual(result.shouldParse({ headers: { 'content-type': 'application/xml' } }), true) @@ -114,10 +121,27 @@ describe('normalizeOptions(options, defaultType)', () => { }, /defaultType must be provided/) }) - it('should throw an error if defaultType is not a string', () => { - assert.throws(() => { - normalizeOptions({}, 123) - }, /defaultType must be provided/) + it('should convert string defaultType to a request content-type checking function', () => { + const result = normalizeOptions({}, 'application/json') + assert.strictEqual(typeof result.shouldParse, 'function') + assert.strictEqual(result.shouldParse({ headers: { 'content-type': 'application/json', 'content-length': '1024' } }), true) + assert.strictEqual(result.shouldParse({ headers: { 'content-type': 'application/xml', 'content-length': '100' } }), false) + }) + + it('should convert array of strings defaultType to a request content-type checking function', () => { + const result = normalizeOptions({}, ['application/json', 'application/*+json']) + assert.strictEqual(typeof result.shouldParse, 'function') + assert.strictEqual(result.shouldParse({ headers: { 'content-type': 'application/json', 'content-length': '1024' } }), true) + assert.strictEqual(result.shouldParse({ headers: { 'content-type': 'application/ld+json', 'content-length': '1024' } }), true) + assert.strictEqual(result.shouldParse({ headers: { 'content-type': 'application/xml', 'content-length': '100' } }), false) + }) + + it('should use function defaultType directly as the request content-type checker', () => { + const typeFunction = (req) => req.headers['content-type'].endsWith('+json') + const result = normalizeOptions({}, typeFunction) + assert.strictEqual(result.shouldParse, typeFunction) + assert.strictEqual(result.shouldParse({ headers: { 'content-type': 'application/ld+json' } }), true) + assert.strictEqual(result.shouldParse({ headers: { 'content-type': 'application/xml' } }), false) }) }) }) From ff43fdf1692139e73e5271199132eb1853c47c04 Mon Sep 17 00:00:00 2001 From: Phillip9587 Date: Wed, 19 Feb 2025 22:21:34 +0100 Subject: [PATCH 4/4] docs: fix defaultType jsdoc --- lib/utils.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/utils.js b/lib/utils.js index 2a9ec8c0..eee5d952 100644 --- a/lib/utils.js +++ b/lib/utils.js @@ -49,7 +49,7 @@ function typeChecker (type) { * Normalizes the common options for all parsers. * * @param {object} options options to normalize - * @param {string | string[] | function} defaultType default content type + * @param {string | string[] | function} defaultType default content type(s) or a function to determine it * @returns {object} */ function normalizeOptions (options, defaultType) {