fix(x-forwarded-for): Follow AWS Load Balancer spec for X-Forwarded-For Header

[austince]

Closes #25.
AWS ELB Docs

Still works with the unknown case, just searches backwards through the IP list instead of forwards.

NOTE: This might be considered a breaking change.

[austince]

While I still think this should be merged in, I've run into some issues with the AWS ELB in api configurations that have an AWS API gateway in front. In that case, the X-Forwarded-For header looks like:
real-client-ip, aws-proxy-ip, aws-api-gateway-private-ip

Perhaps we should allow a config option that can choose from the list of stripped ips from the header?

// ...
getClientIp(req, {
   xForwardedForHandler: (ips) => ips[0]
}
// ...

Would also be happy to put in a PR for that feature, but we should plan it out if you're onboard with it.

[austince]

Hey @pbojinov, do you have any thoughts on this?

[pbojinov]

@austince Sorry for the delay. Thank you for this, I'll review and get back you.

[austince]

Hi @pbojinov, any update on this?

[evdama]

any reason it's not merged?

[marcuspoehls]

@austince @pbojinov Hey Austin, Hey Petar, any chance to get this merged?

I'm maintaining hapi-rate-limitor and users run into the same issue on Heroku where the client's actual IP address is the last item in the chain, not the first one.

I see conflicts on this PR that must be resolved before merging. @austince are you still interested in this feature? Would you mind resolving the conflicts?

[austince]

Hey @marcuspoehls, I’d be happy to resolve the conflicts if we get a confirmation that they will be merged.

[ruscon]

dead ?

[austince]

dead ?

@pbojinov 🤷

[marcuspoehls]

Just a note: I created a separate request-ip as a drop-in replacement for this package. I used this package here for my hapi.js plugins and now took the time to implement my own request-ip package. It may be worth a look.

Looks like Petar (the maintainer of this package here) can’t find the time for this request-ip package. Nonetheless, it’s a helpful package!

[pbojinov]

Sorry for the delay. I stopped getting these notifications via email due to some spam filters. Those should be cleared up now.

I’m happy to get some of these new changes merged in this week. Will review and get back shortly.

[austince]

Not sure what happened here... but am going to close this since it doesn't seem like much else will.

[pbojinov]

Fixed merge conflicts and merged in #51