crossbario / crossbar

Crossbar.io - WAMP application router
https://crossbar.io/
Other
2.05k stars 275 forks source link

REST Bridge is broken when require_ip option is used #798

Closed chaosk closed 7 years ago

chaosk commented 8 years ago

Setting require_ip causes all HTTP requests result in a 500 response.

Crossbar log:

Unhandled exception in Crossbar: ValueError('This is already a native string.',)

Error happens here: https://github.com/crossbario/crossbar/blob/4fe0cc89dce00c01bc53a14958a85ca4bb37fa76/crossbar/adapter/rest/common.py#L393

Transport path configuration:

...
"publish": {
    "type": "publisher",
    "realm": "default",
    "role": "application",
    "options": {
        "require_ip": ["127.0.0.1"]
    }
}
...
oberstet commented 8 years ago

@chaosk Could you dump the output of crossbar version here please? Python 2 vs 3 etc ..

hawkowl commented 8 years ago

Thanks for the report @chaosk, I'll look into it.

chaosk commented 8 years ago

crossbar version output:

 Crossbar.io        : 0.13.2
   Autobahn         : 0.14.0 (with JSON, MessagePack, CBOR)
   Twisted          : 16.1.1-EPollReactor
   LMDB             : 0.89/lmdb-0.9.18
   Python           : 3.4.3/CPython
 OS                 : Linux-4.2.0-25-generic-x86_64-with-Ubuntu-15.10-wily
 Machine            : x86_64
slav0nic commented 8 years ago
Crossbar.io        : 0.15.0
   Autobahn         : 0.16.0 (with JSON, MessagePack, CBOR, UBJSON)
   Twisted          : 16.4.1-EPollReactor
   LMDB             : 0.89/lmdb-0.9.18
   Python           : 3.5.2/CPython
 OS                 : Linux-4.6.0-1-amd64-x86_64-with-debian-stretch-sid
 Machine            : x86_64

have same problem

meejah commented 8 years ago

@hawkowl can we just return string here https://github.com/crossbario/crossbar/blob/4fe0cc89dce00c01bc53a14958a85ca4bb37fa76/crossbar/_compat.py#L44 or is there Good Reasons that's an error?

slav0nic commented 8 years ago

in py3 tests request.getClientIP() return bytes, in real app str

diff --git a/crossbar/adapter/rest/common.py b/crossbar/adapter/rest/common.py
index 0f3a79f..7621d14 100644
--- a/crossbar/adapter/rest/common.py
+++ b/crossbar/adapter/rest/common.py
@@ -422,7 +422,7 @@ class _CommonResource(Resource):
         # enforce client IP address
         #
         if self._require_ip:
-            ip = IPAddress(native_string(client_ip))
+            ip = IPAddress(client_ip)
             allowed = False
             for net in self._require_ip:
                 if ip in net:
diff --git a/crossbar/adapter/rest/test/_request.py b/crossbar/adapter/rest/test/_request.py
index e256c2d..8c4efc2 100644
--- a/crossbar/adapter/rest/test/_request.py
+++ b/crossbar/adapter/rest/test/_request.py
@@ -74,7 +74,7 @@ def request(path, method=b"GET", args=[], isSecure=False, headers={}, body=b'',

     # Set the host we are, and the client we're talking to
     req.setHost(host, port, isSecure)
-    req.client = IPv4Address("TCP", b"127.0.0.1", 8000)
+    req.client = IPv4Address("TCP", "127.0.0.1", 8000)

     _written_data = BytesIO()

-- 
meejah commented 7 years ago

Thanks for the patch @slav0nic!

(...and sorry the broken master build meant this took a while to merge even though it's a simple fix...)