Definitive way to get the IP address of a request

[silvinci]

Express is awesome. Yet there's one thing that annoys me: retrieving the IP address of a request.

Yeah I know. You can grab it with req.connection.remoteAddress.

But what if the request was sent via a proxy? Express has a solution for this, too:

app.set("trust proxy", true);

// ...

console.log(req.ips); // ["proxy-n", "proxy-2", "proxy-1", "client"]

But here's already the first flaw: req.ips only works if the client uses a proxy. If the client doesn't use a proxy you'd have to check that first and then fall back to req.connection.remoteAddress.

"This is shit. Change that." — Co-worker.

But wait, it gets even worse. Let's say I use http-proxy. So http-proxy send all the client's requests to my nice little express app and is so kind to add an x-forwarded-for header with the client's IP.

console.log(req.ips); would return ["clientIP"], which is fine.

After half a year of testing and development on my http-proxyied testserver I want to move on to my production server. Without http-proxy.

Well, shit. req.ips is now empty and I'd have to use req.connection.remoteAddress everywhere where I used req.ips, which would be a lot of changing an reviewing.

I suggest, that req.ip (without plural-s) should always return a string either containing the req.connection.remoteAddress or if available and allowed by app.set("trust proxy", true); the last IP address (aka the client's IP).

req.ips  wouldn't have to be altered.

/**

 * If "trust proxy" is `true`, parse

 * the "X-Forwarded-For" ip address list

 * and return the last ip address (the client's ip).
 * 

 * If not, return req.connection.remoteAddress.

 * 

 * @return {String}

 * @api public

 */



req.__defineGetter__('ip', function(){

  var trustProxy = this.app.get('trust proxy');

  return trustProxy ? (this.ips.reverse()[0] || this.connection.remoteAddress) : this.connection.remoteAddress;

});

By the way: You have a security failure in req.ips. According to your specification it should contain a trustProxy check, like so:

req.__defineGetter__('ips', function(){

  var trustProxy = this.app.get('trust proxy');
  var val = this.get('X-Forwarded-For');
  return trustProxy && val
    ? val.split(/ *, */).reverse()
    : [];
});

Greetings,

Jan

[tjholowaychuk]

yeah I agree, sorry about that, req.ips is new, and wow yeah you're
right I completely omitted the setting check :s sketchy, thanks! I
added req.ip and removed reversing of the parsed header field for now
to simplify things a little

On May 29, 6:24 pm, silvinci <buschto...@googlemail.com> wrote:
> Express is awesome. Yet there's one thing that annoys me: retrieving the IP
> address of a request.
>
> Yeah I know. You can grab it with req.connection.remoteAddress.
> But what if the request was sent via a proxy? Express has a solution for
> this, too:
> app.set("trust proxy", true);
> // ...
> console.log(req.ips); // ["proxy-n", "proxy-2", "proxy-1", "client"]
>
> But here's already the first flaw: req.ips only works if the client uses a
> proxy. If the client doesn't use a proxy you'd have to check that first and
> then fall back to req.connection.remoteAddress.
> "This is shit. Change that." — Co-worker.
>

> But wait, it gets even worse. Let's say I use http-proxy<https://github.com/silvinci/node-http-proxy>.

>  return trustProxy *?* *(*this.ips.reverse()[0] *||*this.connection.remoteAddress
> *)* *:* this.connection.remoteAddress;

[silvinci]

Just for the records. I made a pull request here with some more specific information.

TJ fixed the bugs in a few minutes. Future versions of express or a fresh copy of the
master branch will have a slightly altered logic of retrieving IPs.

req.ip now returns a string containing the clients IP. If app.set("trust proxy", true); was set and a x-forwarded-for header is provided, express will return the forwarded IP. If x-forwarded-for is empty or not given, express will return req.connection.remoteAddress. If trust proxy wasn't set it will always return req.connection.remoteAddress.

req.ips formerly returned the x-forwarded-for list in a reversed form like ["proxy-n", "proxy-2", "proxy-1", "client"]. Now it will return the original un-reversed list: ["client", "proxy-1", "proxy-2", " proxy-n "]

If trust proxy isn't set req.ips will always return an empty array, as defined in the specs.