fix: strip port from X-Forwarded-For in request.ip (#827)

__tests__/request/ip-port.test.js

@@ -0,0 +1,45 @@
+'use strict'
+
+const { describe, it } = require('node:test')
+const assert = require('node:assert/strict')
+const Stream = require('stream')
+const Koa = require('../..')
+const Request = require('../../test-helpers/context').request
+
+describe('req.ip with port in X-Forwarded-For', () => {
+  describe('when XFF contains IPv4 with port', () => {
+    it('should strip the port', () => {
+      const req = request()
+      req.app.proxy = true
+      req.header['x-forwarded-for'] = '1.2.3.4:8080, 5.6.7.8'
+      assert.strictEqual(req.ip, '1.2.3.4')
+    })
+  })
+
+  describe('when XFF contains IPv6 with port', () => {
+    it('should strip the port and brackets', () => {
+      const req = request()
+      req.app.proxy = true
+      req.header['x-forwarded-for'] = '[::1]:8080, 127.0.0.1'
+      assert.strictEqual(req.ip, '::1')
+    })
+  })
+
+  describe('when XFF contains plain IPv4 (no port)', () => {
+    it('should return the address as-is', () => {
+      const req = request()
+      req.app.proxy = true
+      req.header['x-forwarded-for'] = '1.2.3.4, 5.6.7.8'
+      assert.strictEqual(req.ip, '1.2.3.4')
+    })
+  })
+
+  describe('when XFF contains plain IPv6 (no port)', () => {
+    it('should return the address as-is', () => {
+      const req = request()
+      req.app.proxy = true
+      req.header['x-forwarded-for'] = '::1, 127.0.0.1'
+      assert.strictEqual(req.ip, '::1')
+    })
+  })
+})

lib/request.js

@@ -15,6 +15,7 @@ const sp = require('./search-params.js')
 const typeis = require('type-is')
 const fresh = require('fresh')
 const only = require('./only.js')
+const forwarded = require('forwarded')
 const util = require('util')
 
 const IP = Symbol('context#ip')
@@ -23,6 +24,28 @@ const IP = Symbol('context#ip')
  * Prototype.
  */
 
+
+/**
+ * Strip port from an IP address string.
+ * Handles IPv4 (e.g. "1.2.3.4:8080") and bracketed IPv6 (e.g. "[::1]:8080").
+ * @param {string} addr
+ * @returns {string}
+ * @private
+ */
+function stripPort(addr) {
+  if (!addr) return ''
+  if (addr.charCodeAt(0) === 91 /* [ */) {
+    const end = addr.indexOf(']')
+    return end > 1 ? addr.slice(1, end) : addr
+  }
+  // IPv4 with port has exactly one colon; IPv6 has multiple
+  const lastColon = addr.lastIndexOf(':')
+  if (lastColon > 0 && addr.indexOf(':') === lastColon) {
+    return addr.slice(0, lastColon)
+  }
+  return addr
+}
+
 module.exports = {
 
   /**
@@ -462,7 +485,13 @@ module.exports = {
 
   get ip () {
     if (!this[IP]) {
-      this[IP] = this.ips[0] || this.socket.remoteAddress || ''
+      // Use forwarded() to properly parse the X-Forwarded-For chain.
+      // forwarded() returns [socketAddr, ...xffEntriesReversed], so the
+      // first XFF entry (original client) is at the end of the array.
+      // See: https://github.com/koajs/koa/issues/827
+      const addrs = forwarded(this.req)
+      const addr = stripPort(this.ips[0]) || stripPort(addrs[addrs.length - 1]) || addrs[0] || ''
+      this[IP] = addr
     }
     return this[IP]
   },
@@ -744,4 +773,4 @@ if (util.inspect.custom) {
  */
 function splitCommaSeparatedValues (value, limit) {
   return value.split(',', limit).map(v => v.trim())
-}
+}

package.json

@@ -44,6 +44,7 @@
     "destroy": "^1.2.0",
     "encodeurl": "^2.0.0",
     "escape-html": "^1.0.3",
+    "forwarded": "^0.2.0",
     "fresh": "~0.5.2",
     "http-assert": "^1.5.0",
     "http-errors": "^2.0.0",
@@ -70,4 +71,4 @@
     "lib"
   ],
   "homepage": "https://koajs.com"
-}
+}