From d586d259b8a5bbc954b4ed17ef7d097a2202de47 Mon Sep 17 00:00:00 2001
From: Nicolas MARTEAU <nico_mart@hotmail.fr>
Date: Mon, 16 Sep 2024 23:03:47 +0200
Subject: [PATCH] fix: redefine ip decorator when trustProxy option (fastify
 instance) is set to true

---
 README.md          |  2 +-
 index.js           | 27 ++++++++++++++++++++++++---
 test/index.test.js | 35 ++++++++++++++++++++++++++++++++++-
 3 files changed, 59 insertions(+), 5 deletions(-)

diff --git a/README.md b/README.md
index 61fe109..bb93331 100644
--- a/README.md
+++ b/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
 
diff --git a/index.js b/index.js
index cb87857..26b2042 100644
--- a/index.js
+++ b/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()
 
diff --git a/test/index.test.js b/test/index.test.js
index 9bb6eba..d865807 100644
--- a/test/index.test.js
+++ b/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
+      }
+    })
+  })
 })