fix: redefine ip decorator when trustProxy option is set to true (#51)

Author: Dafyh

README.md

@@ -27,7 +27,7 @@ The plugin will make a best-effort to infer the request's IP based on a subset o
 
 The plugin will use a FIFO strategy to infer the right IP. This can be customised by passing a custom  order  property that includes your custom headers.
 
->Note: It is important to remark that this does not alters the `Request#ips` behaviour for inferring IPs when setting the `FastifyOptions#trustProxy` to `true`. It rather allows you to infer the IP of a given request by headers out of the common spec or standard.
+>Note: It is important to remark that this does not alters the `Request#ips` behavior for inferring IPs when setting the `FastifyOptions#trustProxy` to `true`. It rather allows you to infer the IP of a given request by headers out of the common spec or standard. However, the `ip` decorator may be updated by fastify-ip, depending on the order of priority in which the above-mentioned headers are processed.
 
 ## Setup
 

index.js

@@ -61,8 +61,8 @@ function fastifyIp (
   instance.decorateRequest('inferIPVersion', inferIPVersion)
   instance.decorateRequest('_fastifyip', '')
 
-  // Core method
-  instance.decorateRequest('ip', {
+  const ipDecorator = {
+    // Core method
     getter: function () {
       let ip = this._fastifyip
       if (ip !== '') return ip
@@ -90,7 +90,28 @@ function fastifyIp (
 
       return ip
     }
-  })
+  }
+
+  const isTrustProxyOn = [
+    'ip',
+    'ips',
+    'hostname',
+    'protocol'
+  ].every((key) => instance.hasRequestDecorator(key))
+
+  if (isTrustProxyOn) {
+    function redefineIpDecorator (request, unusedReply, done) {
+      Object.defineProperty(request, 'ip', {
+        get: ipDecorator.getter.bind(request)
+      })
+
+      done()
+    }
+
+    instance.addHook('preHandler', redefineIpDecorator)
+  } else {
+    instance.decorateRequest('ip', ipDecorator)
+  }
 
   done()
 

test/index.test.js

@@ -112,7 +112,7 @@ tap.test('Plugin#Decoration', scope => {
 })
 
 tap.test('Plugin#Request IP', scope => {
-  scope.plan(8)
+  scope.plan(9)
 
   scope.test('Should infer the header based on default priority', async t => {
     const app = fastify()
@@ -475,4 +475,37 @@ tap.test('Plugin#Request IP', scope => {
       })
     }
   )
+
+  scope.test('Should replace the IP even if trustProxy option is set to true', async t => {
+    const app = fastify({ trustProxy: true })
+    const localIP = '127.0.0.1'
+    const expectedIP = faker.internet.ip()
+
+    async function withoutPlugin (instance) {
+      instance.get('/withoutPlugin', async (req) => {
+        t.equal(req.ip, localIP)
+      })
+    }
+
+    async function withPlugin (instance) {
+      instance.register(plugin)
+      instance.get('/withPlugin', async (req) => {
+        t.equal(req.ip, expectedIP)
+        t.equal(req._fastifyip, expectedIP)
+      })
+    }
+
+    app.register(withoutPlugin)
+    app.register(withPlugin)
+
+    t.plan(3)
+
+    await app.inject('/withoutPlugin')
+    await app.inject({
+      path: '/withPlugin',
+      headers: {
+        'x-client-ip': expectedIP
+      }
+    })
+  })
 })