pyepye/django-amplitude

IP parsing with X_FORWARDED_FOR

Closed this issue · 1 comments

unqx commented

Hi, @pyepye!

I experienced an error using your library: there was no ip present in the geoip database.
Digging deeper, I found that the problem is in that line:

ip = x_forwarded_for.split(',')[-1].strip()

I have the following servers configuration with X-Forwarded-For headers configured:

Balancer NGINX -> Backend NGINX -> Django

The header that reaches Django looks like this: 93.81.xxx.xxx, 172.31.xxx.xxx. The line above will select the last one which is local ip address. That causes an exception at GeoIP

AddressNotFoundError
The address 172.31.xxx.xxx is not in the database

As far as I know, it is a common behaviour for proxies to create a chain of IPs that starts with client IP and is followed by proxies.

So, i'd suggest the following change for this line:

ip = x_forwarded_for.split(',')[0].strip()

Would like to know your thoughts, thanks.

Hey @unqx

Sorry for the late reply, I've not had much spare time recently.

Yep that does look like a bug, the first IP is the most likely to be the client's address.

I've just published version 0.7.1 to Pypi which now uses your suggested line / the first IP. (I also created a new tag / release in Github)

Hopefully that was the only issue with the GeoIP stuff, it doesn't have any tests wrapping it so I've been a little worried it wouldn't work as expected.

Thanks
Matt