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
nodejs · Mar 1, 2024 · ef5fec8 · ef5fec8
1 parent e343948
commit ef5fec8
Show file tree

Hide file tree

Showing 3 changed files with 104 additions and 29 deletions.
diff --git a/benchmarks/core/is-valid-header-char.mjs 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
@@ -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,15 +111,15 @@ 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
     } else if (body instanceof ArrayBuffer) {
       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
@@ -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,