Re: [CherryPy] #919: CherryPy crashes if socket.accept() does not return a remote address

[CherryPy]

#919: CherryPy crashes if socket.accept() does not return a remote address
------------------------+---------------------------------------------------
Reporter: gwatts | Owner: fumanchu
Type: defect | Status: new
Priority: high | Milestone:
Component: wsgiserver | Resolution:
Keywords: |
------------------------+---------------------------------------------------
Old description:

> I've come across some situations, on OS X, where socket.accept() does not
> return the remote peer's ip address. The actual accept() system call
> actually returns a zero length address. This seems to be a legitimate
> condition (though i've no idea what really leads to it).
>
> Apache just logs :: for the address in this situation, but CherryPy
> crashes in wsgiserver/__init__.py in tick()
>
> Suggest adding a couple of lines after socket.accept() in tick() such as
> if addr is None:
> addr = ('::', '0')

New description:

I've come across some situations, on OS X, where socket.accept() does not
return the remote peer's ip address. The actual accept() system call
actually returns a zero length address. This seems to be a legitimate
condition (though i've no idea what really leads to it).

Apache just logs :: for the address in this situation, but CherryPy
crashes in wsgiserver/__init__.py in tick()

Suggest adding a couple of lines after socket.accept() in tick() such as

{{{
if addr is None:
addr = ('::', '0')
}}}

--
Ticket URL: <http://www.cherrypy.org/ticket/919>
CherryPy <http://www.cherrypy.org>
CherryPy - a pythonic, object-oriented HTTP framework

[CherryPy]

#919: CherryPy crashes if socket.accept() does not return a remote address
------------------------+---------------------------------------------------
Reporter: gwatts | Owner: fumanchu
Type: defect | Status: new
Priority: high | Milestone:

Component: wsgiserver | Keywords:
------------------------+---------------------------------------------------

[CherryPy]

#919: CherryPy crashes if socket.accept() does not return a remote address
------------------------+---------------------------------------------------
Reporter: gwatts | Owner: fumanchu
Type: defect | Status: new

Priority: high | Milestone: 3.1

Component: wsgiserver | Resolution:
Keywords: |
------------------------+---------------------------------------------------

Changes (by visteya):

* milestone: => 3.1

Comment:

patch:
{{{
--- __init__.py (revision 2279)
+++ __init__.py (working copy)
@@ -1680,6 +1680,15 @@
"""Accept a new connection and put it on the Queue."""
try:
s, addr = self.socket.accept()
+ if addr is None: # sometimes this can happen
+ # figure out if AF_INET or AF_INET6
+ if len(s.getsockname()) == 2:
+ # AF_INET
+ addr = ('0.0.0.0', 0)
+ else:
+ # AF_INET6
+ assert len(s.getsockname()) == 4
+ addr = ('::', 0)
prevent_socket_inheritance(s)
if not self.ready:
return

[CherryPy]

#919: CherryPy crashes if socket.accept() does not return a remote address
------------------------+---------------------------------------------------
Reporter: gwatts | Owner: fumanchu
Type: defect | Status: new

Priority: low | Milestone: 3.2

Component: wsgiserver | Resolution:
Keywords: |
------------------------+---------------------------------------------------

Changes (by lawouach):

* milestone: 3.1 => 3.2
* priority: high => low

Comment:

This seems indeed legit and comes up when no information could be
retrieved about the remote host but it's hard to find any more meaningful
info than that.

The patch looks alright to me, if only for the assert call which I don't
think CherryPy uses in general. That being said, might be interesting to
at least log a message about it as well.

I'm lowering the priority because I doubt it's a common issue though I
agree the severity is high considering CP's crash.

[CherryPy]

#919: CherryPy crashes if socket.accept() does not return a remote address
------------------------+---------------------------------------------------
Reporter: gwatts | Owner: fumanchu

Type: defect | Status: closed
Priority: low | Milestone: 3.2
Component: wsgiserver | Resolution: fixed

Keywords: |
------------------------+---------------------------------------------------
Changes (by visteya):

* resolution: => fixed
* status: new => closed

Comment:

patch applied to trunk, without the assert(). r2294

I didn't much like the assert() either. There about 6 other asserts in
CherryPy (excluding tests): two in `wsgiserver.__init__.py`, and several
in `lib/httpauth.py`