improve codebase and performance

.github/workflows/ci.yml

@@ -13,4 +13,6 @@ on:
 jobs:
   test:
     uses: fastify/workflows/.github/workflows/plugins-ci.yml@v3
-
+    with:
+      lint: true
+

.npmrc

@@ -0,0 +1 @@
+package-lock=false

benchmark/combined.js

@@ -0,0 +1,30 @@
+'use strict'
+const benchmark = require('benchmark')
+
+const forwarded = require('..')
+const req0 = fakerequest({})
+const req1 = fakerequest({ 'x-forwarded-for': '192.168.0.10' })
+const req2 = fakerequest({ 'x-forwarded-for': '192.168.0.10, 192.168.1.20' })
+const req5 = fakerequest({ 'x-forwarded-for': '192.168.0.10, 192.168.1.20, 192.168.1.21, 192.168.1.22, 192.168.1.23' })
+
+new benchmark.Suite()
+  .add('combined',
+    function combined () {
+      forwarded(req0)
+      forwarded(req1)
+      forwarded(req2)
+      forwarded(req5)
+    },
+    { minSamples: 100 }
+  )
+  .on('cycle', function onCycle (event) { console.log(String(event.target)) })
+  .run({ async: false })
+
+function fakerequest (headers) {
+  return {
+    headers,
+    socket: {
+      remoteAddress: '10.0.0.1'
+    }
+  }
+}

benchmark/index.js

@@ -1,56 +1,20 @@
-
-/**
- * Module dependencies.
- */
+'use strict'
 
 const benchmark = require('benchmark')
-const benchmarks = require('beautify-benchmark')
-
-/**
- * Globals for benchmark.js
- */
-
-global.forwarded = require('..')
-global.req0 = fakerequest({})
-global.req1 = fakerequest({ 'x-forwarded-for': '192.168.0.10' })
-global.req2 = fakerequest({ 'x-forwarded-for': '192.168.0.10, 192.168.1.20' })
-global.req5 = fakerequest({ 'x-forwarded-for': '192.168.0.10, 192.168.1.20, 192.168.1.21, 192.168.1.22, 192.168.1.23' })
-
-const suite = new benchmark.Suite()
-
-suite.add({
-  name: 'no header',
-  minSamples: 100,
-  fn: 'var addrs = forwarded(req0)'
-})
-
-suite.add({
-  name: '1 address',
-  minSamples: 100,
-  fn: 'var addrs = forwarded(req1)'
-})
-
-suite.add({
-  name: '2 addresses',
-  minSamples: 100,
-  fn: 'var addrs = forwarded(req2)'
-})
-
-suite.add({
-  name: '5 addresses',
-  minSamples: 100,
-  fn: 'var addrs = forwarded(req5)'
-})
-
-suite.on('cycle', function onCycle (event) {
-  benchmarks.add(event.target)
-})
-
-suite.on('complete', function onComplete () {
-  benchmarks.log()
-})
 
-suite.run({ async: false })
+const forwarded = require('..')
+const req0 = fakerequest({})
+const req1 = fakerequest({ 'x-forwarded-for': '192.168.0.10' })
+const req2 = fakerequest({ 'x-forwarded-for': '192.168.0.10, 192.168.1.20' })
+const req5 = fakerequest({ 'x-forwarded-for': '192.168.0.10, 192.168.1.20, 192.168.1.21, 192.168.1.22, 192.168.1.23' })
+
+new benchmark.Suite()
+  .add('no header', function () { forwarded(req0) }, { minSamples: 100 })
+  .add('1 address', function () { forwarded(req1) }, { minSamples: 100 })
+  .add('2 addresses', function () { forwarded(req2) }, { minSamples: 100 })
+  .add('5 addresses', function () { forwarded(req5) }, { minSamples: 100 })
+  .on('cycle', function onCycle (event) { console.log(String(event.target)) })
+  .run({ async: false })
 
 function fakerequest (headers) {
   return {

index.js

@@ -7,70 +7,53 @@
 'use strict'
 
 /**
- * Module exports.
- * @public
+ * Get all addresses in the request used in the `X-Forwarded-For` header.
  */
-
-module.exports = forwarded
-
-/**
- * Get all addresses in the request, using the `X-Forwarded-For` header.
- *
- * @param {object} req
- * @return {array}
- * @public
- */
-
 function forwarded (req) {
   if (!req) {
     throw new TypeError('argument req is required')
   }
 
-  // simple header parsing
-  const proxyAddrs = parse(req.headers['x-forwarded-for'] || '')
+  const header = req.headers['x-forwarded-for']
   const socketAddr = req.socket.remoteAddress
-  const addrs = [socketAddr].concat(proxyAddrs)
 
-  // return all addresses
-  return addrs
+  if (!header || typeof header !== 'string') {
+    return [socketAddr]
+  } else if (header.indexOf(',') === -1) {
+    const remote = header.trim()
+    return (remote.length)
+      ? [socketAddr, remote]
+      : [socketAddr]
+  } else {
+    return parse(header, socketAddr)
+  }
 }
 
-/**
- * Parse the X-Forwarded-For header.
- *
- * @param {string} header
- * @private
- */
+function parse (header, socketAddr) {
+  const result = [socketAddr]
 
-function parse (header) {
   let end = header.length
-  const list = []
-  let start = header.length
-
-  // gather addresses, backwards
-  for (let i = header.length - 1; i >= 0; i--) {
-    switch (header.charCodeAt(i)) {
-      case 0x20: /*   */
-        if (start === end) {
-          start = end = i
-        }
-        break
-      case 0x2c: /* , */
-        if (start !== end) {
-          list.push(header.substring(start, end))
-        }
-        start = end = i
-        break
-      default:
-        start = i
-        break
+  let start = end
+  let char
+  let i
+
+  for (i = end - 1; i >= 0; --i) {
+    char = header[i]
+    if (char === ' ') {
+      (start === end) && (start = end = i)
+    } else if (char === ',') {
+      (start !== end) && result.push(header.slice(start, end))
+      start = end = i
+    } else {
+      start = i
     }
   }
 
-  // final address
-  if (start !== end) {
-    list.push(header.substring(start, end))
-  }
+  (start !== end) && result.push(header.substring(start, end))
 
-  return list
+  return result
 }
+
+module.exports = forwarded
+module.exports.default = forwarded
+module.exports.forwarded = forwarded

package.json

@@ -4,7 +4,8 @@
   "version": "2.0.0",
   "contributors": [
     "Matteo Collina <[email protected]>",
-    "Douglas Christopher Wilson <[email protected]>"
+    "Douglas Christopher Wilson <[email protected]>",
+    "Aras Abbasi <[email protected]"
   ],
   "license": "MIT",
   "keywords": [
@@ -21,19 +22,29 @@
   },
   "homepage": "https://github.com/fastify/forwarded#readme",
   "devDependencies": {
-    "beautify-benchmark": "0.2.4",
+    "@types/node": "^18.0.3",
     "benchmark": "2.1.4",
     "standard": "^17.0.0",
-    "tap": "^16.0.0"
+    "tap": "^16.0.0",
+    "tsd": "^0.22.0"
   },
+  "engines": {
+    "node": ">=14"
+  },
+  "types": "types/index.d.ts",
   "files": [
     "LICENSE",
-    "HISTORY.md",
     "README.md",
-    "index.js"
+    "index.js",
+    "types/index.d.ts"
   ],
   "scripts": {
     "bench": "node benchmark/index.js",
-    "test": "standard && tap"
+    "bench:combined": "node benchmark/combined.js",
+    "lint": "standard",
+    "lint:fix": "standard --fix",
+    "test": "npm run test:unit && npm run test:typescript",
+    "test:unit": "tap",
+    "test:typescript": "tsd"
   }
 }

test/test.js

@@ -22,6 +22,22 @@ test('should include entries from X-Forwarded-For', function (t) {
   t.same(forwarded(req), ['127.0.0.1', '10.0.0.1', '10.0.0.2'])
 })
 
+test('should include entries from X-Forwarded-For', function (t) {
+  t.plan(1)
+  const req = createReq('127.0.0.1', {
+    'x-forwarded-for': '   '
+  })
+  t.same(forwarded(req), ['127.0.0.1'])
+})
+
+test('should include entries from X-Forwarded-For', function (t) {
+  t.plan(1)
+  const req = createReq('127.0.0.1', {
+    'x-forwarded-for': '10.0.0.1'
+  })
+  t.same(forwarded(req), ['127.0.0.1', '10.0.0.1'])
+})
+
 test('should skip blank entries', function (t) {
   t.plan(1)
   const req = createReq('127.0.0.1', {
@@ -38,6 +54,38 @@ test('should trim leading OWS', function (t) {
   t.same(forwarded(req), ['127.0.0.1', '10.0.0.1', '10.0.0.2'])
 })
 
+test('should handle correctly when beginning with a comma', function (t) {
+  t.plan(1)
+  const req = createReq('127.0.0.1', {
+    'x-forwarded-for': ', 10.0.0.2 ,  , 10.0.0.1'
+  })
+  t.same(forwarded(req), ['127.0.0.1', '10.0.0.1', '10.0.0.2'])
+})
+
+test('should handle correctly when ending with a comma', function (t) {
+  t.plan(1)
+  const req = createReq('127.0.0.1', {
+    'x-forwarded-for': '10.0.0.2 ,  , 10.0.0.1,'
+  })
+  t.same(forwarded(req), ['127.0.0.1', '10.0.0.1', '10.0.0.2'])
+})
+
+test('should trim trailing OWS before a comma', function (t) {
+  t.plan(1)
+  const req = createReq('127.0.0.1', {
+    'x-forwarded-for': ' , 10.0.0.2 ,  , 10.0.0.1'
+  })
+  t.same(forwarded(req), ['127.0.0.1', '10.0.0.1', '10.0.0.2'])
+})
+
+test('should trim trailing OWS after a comma', function (t) {
+  t.plan(1)
+  const req = createReq('127.0.0.1', {
+    'x-forwarded-for': ' 10.0.0.2 ,  , 10.0.0.1 , '
+  })
+  t.same(forwarded(req), ['127.0.0.1', '10.0.0.1', '10.0.0.2'])
+})
+
 function createReq (socketAddr, headers) {
   return {
     socket: {

types/index.d.ts

@@ -0,0 +1,11 @@
+import { IncomingMessage } from 'http';
+
+/**
+ * Get all addresses in the request used in the `X-Forwarded-For` header.
+ */
+declare function forwarded(req: IncomingMessage): string[];
+
+export default forwarded
+export {
+  forwarded
+}

types/index.test-d.ts

@@ -0,0 +1,12 @@
+import { IncomingMessage } from "http";
+import { expectError, expectType } from "tsd";
+import { forwarded } from "..";
+import forwardedESM from "..";
+import * as forwardedVarImport from "..";
+
+expectType<string[]>(forwardedESM({} as IncomingMessage))
+expectType<string[]>(forwarded({} as IncomingMessage))
+expectType<string[]>(forwardedVarImport.default({} as IncomingMessage))
+expectType<string[]>(forwardedVarImport.forwarded({} as IncomingMessage))
+expectError(forwarded())
+expectError(forwarded('10.0.0.1'))