Opened 21 months ago

Last modified 13 months ago

#8241 defect new

WSGI throws exception with IPv6 client due to string conversion on None result from twisted.web.http.getClientIP

Reported by: Trent Lloyd Owned by:
Priority: normal Milestone:
Component: web Keywords:
Cc: Branch:
Author:

Description

Connecting to a WSGI instance with an IPv6 client throws an exception in the request processing.

File "/usr/lib/python3/dist-packages/twisted/web/wsgi.py", line 293, in init

'REMOTE_ADDR': _wsgiString(request.getClientIP()),

File "/usr/lib/python3/dist-packages/twisted/web/wsgi.py", line 85, in _wsgiString

return string.decode("iso-8859-1")

builtins.AttributeError: 'NoneType' object has no attribute 'decode'

The problem is that twisted.web.http.getClientIP() specifically checks that the address object is an instance of IPv4Address before returning, otherwise it returns None which then blows up in the string type conversion by _wsgiString()

This has previously been documented in #7704, however that bug was closed as wontfix and split into bugs about designing a better interface then the current interface. While I agree that is a good goal, it would be create to fix the basic support now. In either case the current call within the WSGI module would be deprecated so making a change here seems fine in that regard.

Attaching patch suggestions for review input in the next update.

Change History (3)

comment:1 Changed 21 months ago by Trent Lloyd

I am not sure on the "best" way to make this change, so seeking review input.

The first change, is we could simply modify twisted.web.http.getClientIP() to check for either an IPv4Address or IPv6Address. The code will work in either case:

diff --git a/twisted/web/http.py b/twisted/web/http.py
index 66ba3da..07f6672 100644
--- a/twisted/web/http.py
+++ b/twisted/web/http.py
@@ -1244,7 +1244,7 @@ class Request:
         @returns: the client IP address
         @rtype: C{str}
         """
-        if isinstance(self.client, address.IPv4Address):
+        if isinstance(self.client, address.IPv4Address) or isinstance(self.client, address.IPv6Address):
             return self.client.host
         else:
             return None

The other change we could make is to simply bypass this function, and lookup the attribute directly in the wsgi module. This would require further work to work with any other socket type, but they wouldn't work now anyway as above.

diff --git a/twisted/web/wsgi.py b/twisted/web/wsgi.py
index 3f30658..b72d211 100644
--- a/twisted/web/wsgi.py
+++ b/twisted/web/wsgi.py
@@ -290,7 +290,7 @@ class _WSGIResponse:
         # *both* Python 2 and Python 3, so says PEP-3333.
         self.environ = {
             'REQUEST_METHOD': _wsgiString(request.method),
-            'REMOTE_ADDR': _wsgiString(request.getClientIP()),
+            'REMOTE_ADDR': _wsgiString(request.client.host),
             'SCRIPT_NAME': _wsgiString(scriptName),
             'PATH_INFO': _wsgiString(pathInfo),
             'QUERY_STRING': _wsgiString(queryString),

The rest of the code seems to work fine with an IPv6 request and works in my application with this change, my main concern is breaking other use cases of the code but I suspect that they would already be broken since it would always return None and thus throw an error. Would appreciate your input.

comment:2 Changed 21 months ago by Trent Lloyd

Summary: WSGI throws exception with IPv6 client due to twisted.web.getClientIPWSGI throws exception with IPv6 client due to string conversion on None result from twisted.web.http.getClientIP

comment:3 Changed 13 months ago by Glyph

See also #7704

Note: See TracTickets for help on using tickets.