Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Merged
merged 1 commit into from
Sep 17, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion README.md
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down
27 changes: 24 additions & 3 deletions index.js
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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()

Expand Down
35 changes: 34 additions & 1 deletion test/index.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -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()
Expand Down Expand Up @@ -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
}
})
})
})
Loading