Web: get real ip (on proxy use)

[treemo]

No description provided.

[landscape-bot]

Code quality remained the same when pulling e6ab052 on treemo:master into 16d52a1 on circuits:master.

[prologic]

I think this is fine :) 👍

[prologic]

@spaceone What are your objections behind this? Sometimes being compliant with RFC(s) is not that useful when the real world is very much different :)

@treemo What problem is this solving? The OSError(s) you were getting?

[spaceone]

@prologic

1. non-compiant parsing: There is no parsing of header field syntax e.g. if multiple proxies are used.
2. security. this is an API change which causes user which rely that the field is a real IP address obtained by a connected socket will be vulnerable.

Why don't you add a

class BehindProxy(BaseComponent):

    @handler('request', priority=1)
    def _on_request(self, req. res):
        req.remote = req.headers.elements('X-Forwarded-For', req.remote.ip)[0]

instead of that ungeneric global change.... :(
But as I said: I don't really mind, as I am developing circuits.http which will never contain such a thing.

[treemo]

@prologic this commit is my hotfix used for OSError (I use a reverse proxy for resolve my problem)

@spaceone yes this idea (BehindProxy) is à good solution :)

[prologic]

@spnaceone I like your idea here; (btw: let's keep things positive?). I think we should be moving forward more into this loosely coupled design with circuits.web itself and improve more of the design to be more flexible like this. It is circuits afterall and meant to be completely event-driven and component architectures!

@treemo Would something like BehindProxy() solve your particular issue here (even if we added it as a tool or such?)

[spaceone]

sorry, for ranting when seeing commits i don't like

[prologic]

It's quite okay :) I think it's important to keep an open mind though and keep communications flowing positively :) I do agree with you and I think your idea of splitting this out into a separate component is a great idea!

TBQH when I originally wrote/designed circuits.web I cobbled together pieces of cherrypy, basehttpserver and a few other things. It could be a lot better and more in-line with the "circuits" way so to speak :)

However; that being said -- I strongly believe we can achieve a better design without necessarily breaking large parts of the higher level API :)

[prologic]

@treemo I think we could all be happy if you moved this code change into something like circuits.web.tools.ProxyFilter or such; something appropriately named :) That way it's a) loosely coupled and b) optional to the end-user/developer Agreed @spaceone?

[treemo]

@spaceone no problem (the comments are designed for make avance the idea) :)
@prologic yes, I'm go test this :)

[prologic]

Awesome work guys :) 👍

[landscape-bot]

Repository health decreased by 0.05% when pulling 6988bbe on treemo:master into 16d52a1 on circuits:master.

• 2 new problems were found (including 0 errors and 1 code smell).
• No problems were fixed.

[landscape-bot]

Repository health decreased by 0.04% when pulling ec839e7 on treemo:master into 16d52a1 on circuits:master.

• 1 new problem was found (including 0 errors and 1 code smell).
• No problems were fixed.

[landscape-bot]

Repository health decreased by 0.06% when pulling 3e2d689 on treemo:master into 16d52a1 on circuits:master.

• 1 new problem was found (including 1 error and 0 code smells).
• No problems were fixed.

[landscape-bot]

Repository health decreased by 0.03% when pulling 87ceadd on treemo:master into 16d52a1 on circuits:master.

• 1 new problem was found (including 0 errors and 1 code smell).
• No problems were fixed.

[landscape-bot]

Code quality remained the same when pulling 0ecfd1f on treemo:master into 16d52a1 on circuits:master.

[prologic]

Move these imports down; we like to separate imports:

std lib imports


3rd party imports


local imports

[landscape-bot]

Code quality remained the same when pulling 9aed186 on treemo:master into 40e4e7e on circuits:master.

[prologic]

@treemo Don't you want to short circuit this and reutrn on the first header found?

[prologic]

I'm proposing this slight change:

diff --git a/circuits/web/tools.py b/circuits/web/tools.py
index b99203e..1d5a357 100644
--- a/circuits/web/tools.py
+++ b/circuits/web/tools.py
@@ -455,21 +455,14 @@ def gzip(response, level=4, mime_types=("text/html", "text/plain",)):


 class ReverseProxy(BaseComponent):
-    headers = ('X-Real-IP', 'X-Forwarded-For')

-    def __init__(self, headers=None):
-        """ Component for have the original client IP when a reverse proxy is used
-
-        :param headers: List of HTTP headers to read the original client IP
-        """
-        super(ReverseProxy, self).__init__()
+    headers = ("X-Real-IP", "X-Forwarded-For")

+    def init(self, headers=None):
         if headers:
             self.headers = headers

     @handler('request', priority=1)
     def _on_request(self, req, *_):
-        for index in self.headers:
-            real_ip = req.headers.get(index)
-            req.remote = Host(real_ip, '', real_ip)
-            return
+        ip = filter(None, map(req.headers.get, self.headers))
+        req.remote = ip and Host(ip, "", ip) or req.remote

Thoughts?

[landscape-bot]

Code quality remained the same when pulling a93d47a on treemo:master into 40e4e7e on circuits:master.

[prologic]

If this works for you let's merge it :) 👍

[treemo]

it's ok on my server :)

[prologic]

Merged :) but I don't know wtf I did!