From ef5fec820ed3f08b49ff4e1303d7900f1a9a1d08 Mon Sep 17 00:00:00 2001 From: tsctx <91457664+tsctx@users.noreply.github.com> Date: Fri, 1 Mar 2024 22:36:26 +0900 Subject: [PATCH] perf: optimize check invalid field-vchar (#2889) * perf: optimize check invalid field-vchar * add benchmark, use named import, fix regression * fix benchmark * apply suggestions from code review --- benchmarks/core/is-valid-header-char.mjs | 57 ++++++++++++++++++++++++ lib/core/request.js | 56 +++++++++++------------ lib/core/util.js | 20 ++++++++- 3 files changed, 104 insertions(+), 29 deletions(-) create mode 100644 benchmarks/core/is-valid-header-char.mjs diff --git a/benchmarks/core/is-valid-header-char.mjs b/benchmarks/core/is-valid-header-char.mjs new file mode 100644 index 00000000000..4734d119011 --- /dev/null +++ b/benchmarks/core/is-valid-header-char.mjs @@ -0,0 +1,57 @@ +import { bench, group, run } from 'mitata' +import { isValidHeaderChar } from '../../lib/core/util.js' + +const html = 'text/html' +const json = 'application/json; charset=UTF-8' + +const headerCharRegex = /[^\t\x20-\x7e\x80-\xff]/ + +/** + * @param {string} characters + */ +function charCodeAtApproach (characters) { + // Validate if characters is a valid field-vchar. + // field-value = *( field-content / obs-fold ) + // field-content = field-vchar [ 1*( SP / HTAB ) field-vchar ] + // field-vchar = VCHAR / obs-text + for (let i = 0; i < characters.length; ++i) { + const code = characters.charCodeAt(i) + // not \x20-\x7e, \t and \x80-\xff + if ((code < 0x20 && code !== 0x09) || code === 0x7f || code > 0xff) { + return false + } + } + return true +} + +group(`isValidHeaderChar# ${html}`, () => { + bench('regexp.test', () => { + return !headerCharRegex.test(html) + }) + bench('regexp.exec', () => { + return headerCharRegex.exec(html) === null + }) + bench('charCodeAt', () => { + return charCodeAtApproach(html) + }) + bench('isValidHeaderChar', () => { + return isValidHeaderChar(html) + }) +}) + +group(`isValidHeaderChar# ${json}`, () => { + bench('regexp.test', () => { + return !headerCharRegex.test(json) + }) + bench('regexp.exec', () => { + return headerCharRegex.exec(json) === null + }) + bench('charCodeAt', () => { + return charCodeAtApproach(json) + }) + bench('isValidHeaderChar', () => { + return isValidHeaderChar(json) + }) +}) + +await run() diff --git a/lib/core/request.js b/lib/core/request.js index dc136408b55..45f349c85c6 100644 --- a/lib/core/request.js +++ b/lib/core/request.js @@ -5,21 +5,22 @@ const { NotSupportedError } = require('./errors') const assert = require('node:assert') -const util = require('./util') +const { + isValidHTTPToken, + isValidHeaderChar, + isStream, + destroy, + isBuffer, + isFormDataLike, + isIterable, + isBlobLike, + buildURL, + validateHandler, + getServerName +} = require('./util') const { channels } = require('./diagnostics.js') const { headerNameLowerCasedRecord } = require('./constants') -// headerCharRegex have been lifted from -// https://github.com/nodejs/node/blob/main/lib/_http_common.js - -/** - * Matches if val contains an invalid field-vchar - * field-value = *( field-content / obs-fold ) - * field-content = field-vchar [ 1*( SP / HTAB ) field-vchar ] - * field-vchar = VCHAR / obs-text - */ -const headerCharRegex = /[^\t\x20-\x7e\x80-\xff]/ - // Verifies that a given path is valid does not contain control chars \x00 to \x20 const invalidPathRegex = /[^\u0021-\u00ff]/ @@ -55,7 +56,7 @@ class Request { if (typeof method !== 'string') { throw new InvalidArgumentError('method must be a string') - } else if (!util.isValidHTTPToken(method)) { + } else if (!isValidHTTPToken(method)) { throw new InvalidArgumentError('invalid request method') } @@ -91,13 +92,13 @@ class Request { if (body == null) { this.body = null - } else if (util.isStream(body)) { + } else if (isStream(body)) { this.body = body const rState = this.body._readableState if (!rState || !rState.autoDestroy) { this.endHandler = function autoDestroy () { - util.destroy(this) + destroy(this) } this.body.on('end', this.endHandler) } @@ -110,7 +111,7 @@ class Request { } } this.body.on('error', this.errorHandler) - } else if (util.isBuffer(body)) { + } else if (isBuffer(body)) { this.body = body.byteLength ? body : null } else if (ArrayBuffer.isView(body)) { this.body = body.buffer.byteLength ? Buffer.from(body.buffer, body.byteOffset, body.byteLength) : null @@ -118,7 +119,7 @@ class Request { this.body = body.byteLength ? Buffer.from(body) : null } else if (typeof body === 'string') { this.body = body.length ? Buffer.from(body) : null - } else if (util.isFormDataLike(body) || util.isIterable(body) || util.isBlobLike(body)) { + } else if (isFormDataLike(body) || isIterable(body) || isBlobLike(body)) { this.body = body } else { throw new InvalidArgumentError('body must be a string, a Buffer, a Readable stream, an iterable, or an async iterable') @@ -130,7 +131,7 @@ class Request { this.upgrade = upgrade || null - this.path = query ? util.buildURL(path, query) : path + this.path = query ? buildURL(path, query) : path this.origin = origin @@ -166,22 +167,21 @@ class Request { if (!Array.isArray(header) || header.length !== 2) { throw new InvalidArgumentError('headers must be in key-value pair format') } - const [key, value] = header - processHeader(this, key, value) + processHeader(this, header[0], header[1]) } } else { const keys = Object.keys(headers) - for (const key of keys) { - processHeader(this, key, headers[key]) + for (let i = 0; i < keys.length; ++i) { + processHeader(this, keys[i], headers[keys[i]]) } } } else if (headers != null) { throw new InvalidArgumentError('headers must be an object or an array') } - util.validateHandler(handler, method, upgrade) + validateHandler(handler, method, upgrade) - this.servername = util.getServerName(this.host) + this.servername = getServerName(this.host) this[kHandler] = handler @@ -315,7 +315,7 @@ class Request { } } -function processHeader (request, key, val, skipAppend = false) { +function processHeader (request, key, val) { if (val && (typeof val === 'object' && !Array.isArray(val))) { throw new InvalidArgumentError(`invalid ${key} header`) } else if (val === undefined) { @@ -326,7 +326,7 @@ function processHeader (request, key, val, skipAppend = false) { if (headerName === undefined) { headerName = key.toLowerCase() - if (headerNameLowerCasedRecord[headerName] === undefined && !util.isValidHTTPToken(headerName)) { + if (headerNameLowerCasedRecord[headerName] === undefined && !isValidHTTPToken(headerName)) { throw new InvalidArgumentError('invalid header key') } } @@ -335,7 +335,7 @@ function processHeader (request, key, val, skipAppend = false) { const arr = [] for (let i = 0; i < val.length; i++) { if (typeof val[i] === 'string') { - if (headerCharRegex.exec(val[i]) !== null) { + if (!isValidHeaderChar(val[i])) { throw new InvalidArgumentError(`invalid ${key} header`) } arr.push(val[i]) @@ -349,7 +349,7 @@ function processHeader (request, key, val, skipAppend = false) { } val = arr } else if (typeof val === 'string') { - if (headerCharRegex.exec(val) !== null) { + if (!isValidHeaderChar(val)) { throw new InvalidArgumentError(`invalid ${key} header`) } } else if (val === null) { diff --git a/lib/core/util.js b/lib/core/util.js index 9789a240575..cbb6d7495e5 100644 --- a/lib/core/util.js +++ b/lib/core/util.js @@ -495,6 +495,24 @@ function isValidHTTPToken (characters) { return true } +// headerCharRegex have been lifted from +// https://github.com/nodejs/node/blob/main/lib/_http_common.js + +/** + * Matches if val contains an invalid field-vchar + * field-value = *( field-content / obs-fold ) + * field-content = field-vchar [ 1*( SP / HTAB ) field-vchar ] + * field-vchar = VCHAR / obs-text + */ +const headerCharRegex = /[^\t\x20-\x7e\x80-\xff]/ + +/** + * @param {string} characters + */ +function isValidHeaderChar (characters) { + return !headerCharRegex.test(characters) +} + // Parsed accordingly to RFC 9110 // https://www.rfc-editor.org/rfc/rfc9110#field.content-range function parseRangeHeader (range) { @@ -516,7 +534,6 @@ kEnumerableProperty.enumerable = true module.exports = { kEnumerableProperty, nop, - isDisturbed, isErrored, isReadable, @@ -546,6 +563,7 @@ module.exports = { buildURL, addAbortListener, isValidHTTPToken, + isValidHeaderChar, isTokenCharCode, parseRangeHeader, nodeMajor,