Issue #1011 new

wsgiserver AF_UNIX support broken

Anonymous created an issue

One of our developers was trying to prototype UNIX socket support our application. They set the socket_file argument in the depot's configuration, and verified that the client could connect. To get to this point, they also had to CherryPy 3.1.2 with the fix for bug #942 (the patch for #847 has also been applied).

However, the server is returning this traceback, which looks like a bug in CherryPy:

[11/May/2010:16:40:57] Traceback (most recent call last): File "/home/xxx/hg_ws/pkg-uds/proto/root_i386/usr/lib/python2.6/vendor-packages/pkg/server/depotresponse.py", line 75, in setapp self.request = self.get_request() File "/home/xxx/hg_ws/pkg-uds/proto/root_i386/usr/lib/python2.6/vendor-packages/cherrypy/_cpwsgi.py", line 226, in get_request local = _http.Host('', int(env('SERVER_PORT', 80)), ValueError: invalid literal for int() with base 10: ''

If you look at /cherrypy/wsgiserver/init.py:1683, there's the following code:

        if isinstance(self.bind_addr, basestring):
            # AF_UNIX. This isn't really allowed by WSGI, which doesn't
            # address unix domain sockets. But it's better than nothing.
            environ["SERVER_PORT"] = ""
        else:
            environ["SERVER_PORT"] = str(self.bind_addr[1])
            # optional values
            # Until we do DNS lookups, omit REMOTE_HOST
            environ["REMOTE_ADDR"] = addr[0]
            environ["REMOTE_PORT"] = str(addr[1])

It appears that if an AF_UNIX socket is used, then environ["SERVER_PORT"] = "". Unfortunately, this triggers a bug in _cpwsgi.py, line 226, since it's invalid to call int(""). My guess is that the code in init should simply be changed not to add "SERVER_PORT" to the dictionary, and then the code on line 226 will just work, but I don't know if that's the right fix.

Guidance here appreciated.

Reported by swalker@opensolaris.org

Comments (5)

  1. Anonymous

    The patch makes things work, although it is slightly wrong. Then again, the WSGI spec explicitly forbids setting SERVER_PORT to an empty string, so it will be a problem anyway.

  2. dobo_pl

    I came across this issue when I was trying to run CherryPy behind nginx proxy server as unix socket. Moreover it came out that REMOTE_PORT variable is also affected. Please take a look at my patch, comment it if you want. I would be glad if it gets merged upstream. I'm not sure that it's best solution but it works.

    diff -rupN CherryPy-3.3.0/cherrypy/wsgiserver/wsgiserver2.py CherryPy-3.3.0p/cherrypy/wsgiserver/wsgiserver2.py
    --- CherryPy-3.3.0/cherrypy/wsgiserver/wsgiserver2.py   2014-04-16 17:53:44.000000000 +0200
    +++ CherryPy-3.3.0p/cherrypy/wsgiserver/wsgiserver2.py  2014-05-31 23:55:26.945235306 +0200
    @@ -2354,7 +2354,6 @@ class WSGIGateway_10(WSGIGateway):
                 'PATH_INFO': req.path,
                 'QUERY_STRING': req.qs,
                 'REMOTE_ADDR': req.conn.remote_addr or '',
    -            'REMOTE_PORT': str(req.conn.remote_port or ''),
                 'REQUEST_METHOD': req.method,
                 'REQUEST_URI': req.uri,
                 'SCRIPT_NAME': '',
    @@ -2371,12 +2370,10 @@ class WSGIGateway_10(WSGIGateway):
                 'wsgi.version': (1, 0),
             }
    
    -        if isinstance(req.server.bind_addr, basestring):
    -            # AF_UNIX. This isn't really allowed by WSGI, which doesn't
    -            # address unix domain sockets. But it's better than nothing.
    -            env["SERVER_PORT"] = ""
    -        else:
    +        if isinstance(req.server.bind_addr, tuple):
                 env["SERVER_PORT"] = str(req.server.bind_addr[1])
    +        if req.conn.remote_port is not None:
    +            env["REMOTE_PORT"] = str(req.conn.remote_port)
    
             # Request headers
             for k, v in req.inheaders.iteritems():
    diff -rupN CherryPy-3.3.0/cherrypy/wsgiserver/wsgiserver3.py CherryPy-3.3.0p/cherrypy/wsgiserver/wsgiserver3.py
    --- CherryPy-3.3.0/cherrypy/wsgiserver/wsgiserver3.py   2014-04-16 17:53:44.000000000 +0200
    +++ CherryPy-3.3.0p/cherrypy/wsgiserver/wsgiserver3.py  2014-05-31 23:55:29.131935804 +0200
    @@ -2053,7 +2053,6 @@ class WSGIGateway_10(WSGIGateway):
                 'PATH_INFO': req.path.decode('ISO-8859-1'),
                 'QUERY_STRING': req.qs.decode('ISO-8859-1'),
                 'REMOTE_ADDR': req.conn.remote_addr or '',
    -            'REMOTE_PORT': str(req.conn.remote_port or ''),
                 'REQUEST_METHOD': req.method.decode('ISO-8859-1'),
                 'REQUEST_URI': req.uri.decode('ISO-8859-1'),
                 'SCRIPT_NAME': '',
    @@ -2069,12 +2068,11 @@ class WSGIGateway_10(WSGIGateway):
                 'wsgi.url_scheme': req.scheme.decode('ISO-8859-1'),
                 'wsgi.version': (1, 0),
             }
    -        if isinstance(req.server.bind_addr, basestring):
    -            # AF_UNIX. This isn't really allowed by WSGI, which doesn't
    -            # address unix domain sockets. But it's better than nothing.
    -            env["SERVER_PORT"] = ""
    -        else:
    +
    +        if isinstance(req.server.bind_addr, tuple):
                 env["SERVER_PORT"] = str(req.server.bind_addr[1])
    +        if req.conn.remote_port is not None:
    +            env["REMOTE_PORT"] = str(req.conn.remote_port)
    
             # Request headers
             for k, v in req.inheaders.items():
    
  3. dobo_pl

    Pardon for two comments in the row but apparently it's impossible to include two source codes when editing comment. In PEP333 we can read "SERVER_NAME and SERVER_PORT can never be empty strings, and so are always required.". So maybe it's better solution to take care of casting empty string to int? We will break the standard anyway (actually in current implementation we break it). Diff of second fix proposal:

    diff -rupN CherryPy-3.3.0/cherrypy/_cpwsgi.py CherryPy-3.3.0p/cherrypy/_cpwsgi.py
    --- CherryPy-3.3.0/cherrypy/_cpwsgi.py  2014-04-12 09:33:58.000000000 +0200
    +++ CherryPy-3.3.0p/cherrypy/_cpwsgi.py 2014-06-01 00:51:57.000000000 +0200
    @@ -283,11 +283,12 @@ class AppResponse(object):
         def run(self):
             """Create a Request object using environ."""
             env = self.environ.get
    +        cast_port = lambda val, defval: int(val) if val else defval
    
    -        local = httputil.Host('', int(env('SERVER_PORT', 80)),
    +        local = httputil.Host('', cast_port(env('SERVER_PORT'), 80),
                                   env('SERVER_NAME', ''))
             remote = httputil.Host(env('REMOTE_ADDR', ''),
    -                               int(env('REMOTE_PORT', -1) or -1),
    +                               cast_port(env('REMOTE_PORT'), -1),
                                    env('REMOTE_HOST', ''))
             scheme = env('wsgi.url_scheme')
             sproto = env('ACTUAL_SERVER_PROTOCOL', "HTTP/1.1")
    

    What do you think which is better way to fix this issue?

  4. Pete Bohman

    The following workaround can be use in place of patching cherrypy:

    class MyAppResponse(cherrypy.wsgi.AppResponse):
        """Overrides AppResponse run function in order to work around
        https://bitbucket.org/cherrypy/cherrypy/issue/1011/wsgiserver-af_unix-support-broken"""
    
        def __init__(self, environ, start_response, cpapp):
            super(MyAppResponse, self).__init__(environ, start_response, cpapp)
    
        def run(self):
            self.environ['SERVER_PORT'] = 80
            self.environ['REMOTE_PORT'] = 80
            super(MyAppResponse, self).run()
    
    app.wsgiapp.response_class = MyAppResponse
    
  5. Log in to comment