← Back to team overview

widelands-dev team mailing list archive

Re: [Merge] lp:~widelands-dev/widelands-website/fix_REMOTE_ADDR into lp:widelands-website

 

Review: Approve

lgtm. one possible improvement in a comment further down.

> It looks like the one parameter is used if the previous doesn't exist.

That is exactly right. get() is a method on the built in type dict in Python. documentation (for python 2.7) is here: https://docs.python.org/2/library/stdtypes.html#dict.get

> Because we use the tracking only for showing online users (and not all the other things it can do), it needs also a big cleanup or replaced with another, or own, implementation.

I am not sure what you want to say with this. Why is the current implementation not sufficient?

Diff comments:

> 
> === added file 'wl_utils.py'
> --- wl_utils.py	1970-01-01 00:00:00 +0000
> +++ wl_utils.py	2016-10-13 16:08:08 +0000
> @@ -0,0 +1,13 @@
> +from django.conf import settings
> +
> +
> +def get_real_ip(request):
> +    """Returns the real user IP, even if behind a proxy.
> +
> +    Set BEHIND_PROXY to True in your settings if Django is running
> +    behind a proxy.
> +
> +    """
> +    if getattr(settings, 'BEHIND_PROXY', False):

do we require this setting? We could check if 'HTTP_X_FORWARDED_FOR' is set and if it is, just use it. If not, we assume no proxy:

for v in ('HTTP_X_FORWARDED_FOR', 'REMOTE_ADDR'):
   if v in request.META:
      return request.META[v]
return None

> +        return request.META['HTTP_X_FORWARDED_FOR']
> +    return request.META['REMOTE_ADDR']


-- 
https://code.launchpad.net/~widelands-dev/widelands-website/fix_REMOTE_ADDR/+merge/308337
Your team Widelands Developers is subscribed to branch lp:widelands-website.


References