Merge pull request #23 from OtterJS/fix-proxied-stuff

Fix proxy stuff

Author: Lordfirespeed

.changeset/weak-toes-refuse.md

@@ -0,0 +1,7 @@
+---
+"@otterhttp/proxy-address": patch
+"@otterhttp/forwarded": patch
+"@otterhttp/request": patch
+---
+
+Fix: `req.ip`, `req.ips`, `req.proto` inaccuracies when (trusted) proxies are involved

package.json

@@ -19,6 +19,7 @@
     "eta": "2.2.0",
     "header-range-parser": "^1.1.3",
     "husky": "9.0.11",
+    "ipaddr.js": "^2.2.0",
     "jsonwebtoken": "9.0.2",
     "regexparam": "3.0.0",
     "supertest-fetch": "1.5.0",

packages/forwarded/package.json

@@ -21,5 +21,7 @@
     "build": "tsup",
     "prepack": "pnpm build"
   },
-  "dependencies": {}
+  "dependencies": {
+    "ipaddr.js": "^2.2.0"
+  }
 }

packages/forwarded/src/index.ts

@@ -1,21 +1,41 @@
+import assert from "node:assert/strict"
 import type { IncomingMessage } from "node:http"
+import ipaddr, { type IPv6, type IPv4 } from "ipaddr.js"
+
+/**
+ * Type-guard to determine whether a parsed IP address is an IPv4 address.
+ */
+const isIPv4 = (value: IPv4 | IPv6): value is IPv4 => value.kind() === "ipv4"
+/**
+ * Type-guard to determine whether a parsed IP address is an IPv6 address.
+ */
+const isIPv6 = (value: IPv4 | IPv6): value is IPv6 => value.kind() === "ipv6"
+
+function parseIp(value: string): IPv4 | IPv6 {
+  const ip = ipaddr.parse(value)
+  if (isIPv6(ip) && ip.isIPv4MappedAddress()) return ip.toIPv4Address()
+  return ip
+}
 
 /**
  * Get all addresses in the request, using the `X-Forwarded-For` header.
  */
-export function forwarded(req: Pick<IncomingMessage, "headers" | "socket">): Array<string | undefined> {
-  // simple header parsing
-  const proxyAddrs = parse((req.headers["x-forwarded-for"] as string) || "")
-  const socketAddr = req.socket.remoteAddress
+export function* forwarded(req: Pick<IncomingMessage, "headers" | "socket">): Generator<IPv4 | IPv6 | undefined, void> {
+  const socketAddress = req.socket.remoteAddress != null ? parseIp(req.socket.remoteAddress) : undefined
+  yield socketAddress
 
-  // return all addresses
-  return [socketAddr].concat(proxyAddrs)
+  const xForwardedHeader = req.headers["x-forwarded-for"]
+  if (!xForwardedHeader) return
+  // https://github.com/nodejs/node/blob/58a7b0011a1858f4fde2fe553240153b39c13cd0/lib/_http_incoming.js#L381
+  assert(!Array.isArray(xForwardedHeader))
+
+  yield* parse(xForwardedHeader)
 }
 
 /**
- * Parse the X-Forwarded-For header.
+ * Parse the X-Forwarded-For header, returning a {@link string} for each entry.
  */
-export function parse(header: string): string[] {
+export function parseRaw(header: string): string[] {
   let end = header.length
   const list: string[] = []
   let start = header.length
@@ -45,3 +65,16 @@ export function parse(header: string): string[] {
 
   return list
 }
+
+/**
+ * Parse the X-Forwarded-For header, returning an IP address (see `ipaddr.js` {@link IPv4}, {@link IPv6})
+ * for each entry.
+ */
+export function* parse(header: string): Generator<IPv4 | IPv6, void> {
+  const raw = parseRaw(header)
+  for (const entry of raw) {
+    yield parseIp(entry)
+  }
+}
+
+export type { IPv4, IPv6 }

packages/proxy-address/src/index.ts

@@ -5,7 +5,7 @@ import ipaddr, { type IPv6, type IPv4 } from "ipaddr.js"
 type Req = Pick<IncomingMessage, "headers" | "socket">
 
 export type TrustParameter = string | number | string[]
-export type TrustFunction = (addr: string | undefined, i: number) => boolean
+export type TrustFunction = (addr: IPv4 | IPv6 | undefined, i: number) => boolean
 export type Trust = TrustFunction | TrustParameter
 
 type Subnet = {
@@ -14,8 +14,9 @@ type Subnet = {
 }
 
 const DIGIT_REGEXP = /^[0-9]+$/
-const isip = ipaddr.isValid
-const parseip = ipaddr.parse
+const isIp = ipaddr.isValid
+const parseIp = ipaddr.parse
+
 /**
  * Pre-defined IP ranges.
  */
@@ -55,21 +56,23 @@ const trustNone: TrustFunction = () => false
  * @param req
  * @param trust
  */
-function allAddresses(req: Req, trust?: Trust): Array<string | undefined> {
+function allAddresses(req: Req, trust?: Trust): Array<IPv4 | IPv6 | undefined> {
   // get addresses
+  const addressGenerator = forwarded(req)
 
-  const addrs = forwarded(req)
-
-  if (trust == null) return addrs
-
+  if (trust == null) return Array.from(addressGenerator)
   if (typeof trust !== "function") trust = compile(trust)
 
-  for (let i = 0; i < addrs.length - 1; i++) {
-    if (trust(addrs[i], i)) continue
-    addrs.length = i + 1
+  let i = 0
+  const addresses: Array<IPv4 | IPv6 | undefined> = []
+  for (const address of addressGenerator) {
+    addresses.push(address)
+    if (!trust(address, i)) break
+    i += 1
   }
-  return addrs
+  return addresses
 }
+
 /**
  * Compile argument into trust function.
  *
@@ -93,6 +96,7 @@ function compile(val: string | number | string[]): TrustFunction {
   }
   return compileTrust(compileRangeSubnets(trust))
 }
+
 /**
  * Compile 'hops' number into trust function.
  *
@@ -108,6 +112,7 @@ function compileHopsTrust(hops: number): TrustFunction {
 function compileRangeSubnets(arr: string[]) {
   return arr.map((ip) => parseIPNotation(ip))
 }
+
 /**
  * Compile range subnet array into trust function.
  *
@@ -118,6 +123,7 @@ function compileTrust(rangeSubnets: Subnet[]): TrustFunction {
   const len = rangeSubnets.length
   return len === 0 ? trustNone : len === 1 ? trustSingle(rangeSubnets[0]) : trustMulti(rangeSubnets)
 }
+
 /**
  * Parse IP notation string into range subnet.
  *
@@ -128,9 +134,9 @@ export function parseIPNotation(note: string): Subnet {
   const pos = note.lastIndexOf("/")
   const str = pos !== -1 ? note.substring(0, pos) : note
 
-  if (!isip(str)) throw new TypeError(`invalid IP address: ${str}`)
+  if (!isIp(str)) throw new TypeError(`invalid IP address: ${str}`)
 
-  let ip = parseip(str)
+  let ip = parseIp(str)
   const max = ip.kind() === "ipv6" ? 128 : 32
 
   if (pos === -1) {
@@ -142,43 +148,43 @@ export function parseIPNotation(note: string): Subnet {
   let range: number | null = null
 
   if (DIGIT_REGEXP.test(rangeString)) range = Number.parseInt(rangeString, 10)
-  else if (ip.kind() === "ipv4" && isip(rangeString)) range = parseNetmask(rangeString)
+  else if (ip.kind() === "ipv4" && isIp(rangeString)) range = parseNetmask(rangeString)
 
   if (range == null || range <= 0 || range > max) throw new TypeError(`invalid range on address: ${note}`)
   return { ip, range }
 }
+
 /**
  * Parse netmask string into CIDR range.
  *
  * @param netmask
  * @private
  */
 function parseNetmask(netmask: string) {
-  const ip = parseip(netmask)
+  const ip = parseIp(netmask)
   return ip.kind() === "ipv4" ? ip.prefixLengthFromSubnetMask() : null
 }
+
 /**
  * Determine address of proxied request.
  *
  * @param req
  * @param trust
  * @public
  */
-function proxyAddress(req: Req, trust: Trust): string | undefined {
+function proxyAddress(req: Req, trust: Trust): IPv4 | IPv6 | undefined {
   if (trust == null) throw new TypeError("trust argument cannot be null-ish")
-  const addrs = allAddresses(req, trust)
+  const addresses = allAddresses(req, trust)
 
-  return addrs[addrs.length - 1]
+  return addresses[addresses.length - 1]
 }
 
 /**
  * Compile trust function for multiple subnets.
  */
 function trustMulti(subnets: Subnet[]): TrustFunction {
-  return function trust(addr: string | undefined) {
-    if (addr == null) return false
-    if (!isip(addr)) return false
-    const ip = parseip(addr)
+  return function trust(ip: IPv4 | IPv6 | undefined) {
+    if (ip == null) return false
     let ipconv: IPv4 | IPv6 | null = null
     const kind = ip.kind()
     for (let i = 0; i < subnets.length; i++) {
@@ -197,6 +203,7 @@ function trustMulti(subnets: Subnet[]): TrustFunction {
     return false
   }
 }
+
 /**
  * Compile trust function for single subnet.
  *
@@ -205,10 +212,8 @@ function trustMulti(subnets: Subnet[]): TrustFunction {
 function trustSingle(subnet: Subnet): TrustFunction {
   const subnetKind = subnet.ip.kind()
   const subnetIsIPv4 = subnetKind === "ipv4"
-  return function trust(addr: string | undefined) {
-    if (addr == null) return false
-    if (!isip(addr)) return false
-    let ip = parseip(addr)
+  return function trust(ip: IPv4 | IPv6 | undefined) {
+    if (ip == null) return false
     const kind = ip.kind()
     if (kind !== subnetKind) {
       if (subnetIsIPv4 && !(ip as IPv6).isIPv4MappedAddress()) return false
@@ -220,3 +225,4 @@ function trustSingle(subnet: Subnet): TrustFunction {
 }
 
 export { allAddresses, compile, proxyAddress }
+export type { IPv4, IPv6 }

packages/request/src/addresses.ts

@@ -1,8 +1,6 @@
-import { type Trust, allAddresses, proxyAddress } from "@otterhttp/proxy-address"
+import { type IPv4, type IPv6, type Trust, allAddresses } from "@otterhttp/proxy-address"
 
 import type { HasHeaders, HasSocket } from "./types"
 
-export const getIP = (req: HasHeaders & HasSocket, trust: Trust): string | undefined =>
-  proxyAddress(req, trust)?.replace(/^.*:/, "") // stripping the redundant prefix added by OS to IPv4 address
-
-export const getIPs = (req: HasHeaders & HasSocket, trust: Trust): Array<string | undefined> => allAddresses(req, trust)
+export const getIPs = (req: HasHeaders & HasSocket, trust: Trust): Array<IPv4 | IPv6 | undefined> =>
+  allAddresses(req, trust)

packages/request/src/host.ts

@@ -2,7 +2,7 @@ import type { Trust } from "@otterhttp/proxy-address"
 
 import { isIP } from "node:net"
 import { getRequestHeader } from "./get-header"
-import type { HasHeaders, HasSocket } from "./types"
+import type { HasHeaders, HasIpAddresses, HasSocket } from "./types"
 import { trustRemoteAddress } from "./util/trust-remote-address"
 
 export type Host = {
@@ -38,7 +38,7 @@ const getDefaultHeaderHostString = (req: HasHeaders): string | undefined => {
   return normalizeHostString(host)
 }
 
-const getHostString = (req: HasHeaders & HasSocket, trust: Trust): string | undefined => {
+const getHostString = (req: HasHeaders & HasIpAddresses, trust: Trust): string | undefined => {
   if (trustRemoteAddress(req, trust)) {
     const forwardedHost = getForwardedHeaderHostString(req)
     if (forwardedHost) return forwardedHost
@@ -56,7 +56,7 @@ const getHostString = (req: HasHeaders & HasSocket, trust: Trust): string | unde
   return authorityHost ?? defaultHost ?? undefined
 }
 
-export const getHost = (req: HasHeaders & HasSocket, trust: Trust): Host => {
+export const getHost = (req: HasHeaders & HasIpAddresses, trust: Trust): Host => {
   const host = getHostString(req, trust)
   if (!host) throw new Error("Request does not include valid host information")
 

packages/request/src/protocol.ts

@@ -1,7 +1,7 @@
 import { TLSSocket } from "node:tls"
 import type { Trust } from "@otterhttp/proxy-address"
 
-import type { HasHeaders, HasSocket } from "./types"
+import type { HasHeaders, HasIpAddresses, HasSocket } from "./types"
 import { trustRemoteAddress } from "./util/trust-remote-address"
 
 export type Protocol = "http" | "https" | string
@@ -10,12 +10,12 @@ const hasSecureConnection = (req: HasHeaders & HasSocket): boolean => {
   return req.socket instanceof TLSSocket && req.socket.encrypted
 }
 
-export const getProtocol = (req: HasHeaders & HasSocket, trust: Trust): Protocol => {
+export const getProtocol = (req: HasHeaders & HasSocket & HasIpAddresses, trust: Trust): Protocol => {
   const proto = `http${hasSecureConnection(req) ? "s" : ""}`
 
   if (!trustRemoteAddress(req, trust)) return proto
 
-  const header = (req.headers["X-Forwarded-Proto"] as string) || proto
+  const header = (req.headers["x-forwarded-proto"] as string) || proto
 
   const index = header.indexOf(",")
 

packages/request/src/prototype.ts

@@ -2,12 +2,12 @@ import { IncomingMessage } from "node:http"
 import type { ParsedUrlQuery } from "node:querystring"
 import { Accepts } from "@otterhttp/accepts"
 import { ContentType } from "@otterhttp/content-type"
-import type { Trust } from "@otterhttp/proxy-address"
+import type { IPv4, IPv6, Trust } from "@otterhttp/proxy-address"
 import type { Middleware } from "@otterhttp/router"
 import { type URLParams, getQueryParams } from "@otterhttp/url"
 import type { Result as RangeParseResult, Options as RangeParsingOptions, Ranges } from "header-range-parser"
 
-import { getIP, getIPs } from "./addresses"
+import { getIPs } from "./addresses"
 import { type Cookie, parseCookieHeader } from "./cookies"
 import { getRequestHeader } from "./get-header"
 import { getHost, getSubdomains } from "./host"
@@ -36,8 +36,8 @@ export class Request<Body = unknown> extends IncomingMessage {
   private declare _hostname: string
   private declare _port: number | undefined
   private declare _subdomains: string[]
-  private declare _ip: string | undefined
-  private declare _ips: (string | undefined)[]
+  private declare _ip: IPv4 | IPv6 | undefined
+  private declare _ips: (IPv4 | IPv6 | undefined)[]
 
   // own members
   private _cookies?: Record<string, Cookie>
@@ -47,13 +47,13 @@ export class Request<Body = unknown> extends IncomingMessage {
   populate({ trust, subdomainOffset }: { trust: Trust; subdomainOffset: number | undefined }) {
     this._acceptsMeta = new Accepts(this)
     this._query = getQueryParams(this.url)
+    this._ips = getIPs(this, trust)
+    this._ip = this._ips[this._ips.length - 1]
     this._protocol = getProtocol(this, trust)
     const host = getHost(this, trust)
     this._hostname = host.hostname
     this._port = host.port
     this._subdomains = getSubdomains(host, subdomainOffset)
-    this._ip = getIP(this, trust)
-    this._ips = getIPs(this, trust)
   }
 
   getHeader<HeaderName extends Lowercase<string>>(header: HeaderName): Headers[HeaderName] {
@@ -109,10 +109,10 @@ export class Request<Body = unknown> extends IncomingMessage {
   get subdomains(): readonly string[] {
     return this._subdomains
   }
-  get ip(): string | undefined {
+  get ip(): IPv4 | IPv6 | undefined {
     return this._ip
   }
-  get ips(): readonly (string | undefined)[] {
+  get ips(): readonly (IPv4 | IPv6 | undefined)[] {
     return this._ips
   }
   get xhr(): boolean {

packages/request/src/types.ts

@@ -1,6 +1,9 @@
 import type { IncomingHttpHeaders } from "node:http"
 import type { Socket } from "node:net"
 
+import type { IPv4, IPv6 } from "@otterhttp/proxy-address"
+
+export type HasIpAddresses = { ips: readonly (IPv4 | IPv6 | undefined)[] }
 export type HasHeaders = { headers: IncomingHttpHeaders }
 export type HasSocket = { socket: Socket }