[New-bugs-announce] [issue6706] asyncore's accept() is broken

[Giampaolo Rodola']

New submission from Giampaolo Rodola' <billiejoex at users.sourceforge.net>:

An old bad design choice in asyncore is how it forces the user to 
override handle_accept() and then call self.accept() to obtain a socket 
pair.

    def handle_accept(self):
        conn, addr = self.accept()

The documentation itself shows the code above as an example of how an 
asyncore-based server should handle an incoming connection.
What the doc doesn't say is that the user calling self.accept() is 
exposed to different risks:

- self.accept() can return None instead of a socket pair in which case 
TypeError is raised (see pyftpdlib bug: 
http://code.google.com/p/pyftpdlib/issues/detail?id=91)

- ECONNABORTED can be raised. This is reproducible on Linux by hammering 
the server with nmap (see pyftpdlib bug: 
http://code.google.com/p/pyftpdlib/issues/detail?id=105)

- EAGAIN can be raised too. I never experienced this error condition 
myself in pyftpdlib but by looking at twisted/internet/tcp.py it seems 
reasonable to catch EAGAIN too.

- The resulting address can be None, which means that the connection 
didn't take place.
This is reproducible by hammering the server with nmap (see pyftpdlib 
bug http://code.google.com/p/pyftpdlib/issues/detail?id=104).


The right thing to do when one of such events occur is to "return" as no 
connection has really been established between client and server.

Now, altough handling these kind of conditions is quite easy, the API 
design remains fundamentally wrong, as it forces the user to call 
accept().
As asyncore.py is structured right now the only chance the user has to 
properly accepting a connection is manually handling all these cases in 
his subclass.


My patch in attachment tries to solve this problem by defining a new 
"handle_accept_event()" method which takes care of all the error 
conditions described above resulting in handle_accept() be called *only 
when a connection really took place*.
When the connection is established handle_accept_event() stores the 
socket pair as an attribute and the next call to accept() returns that 
pair.
This preserves the current implementation without breaking any code as 
the user will have to override handle_accept() and then call accept() in 
the same manner [1], with the difference that self.accept() will always 
return a valid socket pair.

[1] 
    def handle_accept(self):
        conn, addr = self.accept()

----------
components: Library (Lib)
messages: 91579
nosy: giampaolo.rodola, josiahcarlson
severity: normal
status: open
title: asyncore's accept() is broken
versions: Python 2.7

_______________________________________
Python tracker <report at bugs.python.org>
<http://bugs.python.org/issue6706>
_______________________________________