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()

@Giampaolo it looks as if you meant to attach a patch but didn't! :)  If you do attach one could you also supply a modified unit test file with it, thanks.

Shame on me, it seems I totally forgot to attach the patch.
Unfortunately the patch went lost but I'm going to try to rewrite it.
As for tests, ECONN and EAGAIN error conditions are hardly reproducible unless you're using nmap.
I'm assigning this to me for now as a reminder and update the issue as long as I have a working patch.

giampaolo: did you ever rewrite the patch?

For reference to other users:
http://code.google.com/p/pyftpdlib/source/browse/trunk/pyftpdlib/ftpserver.py

Note the complexity of the two handle_accept implementations in that file; both of them begin:
        try:
            sock, addr = self.accept()
        except TypeError:
            # sometimes accept() might return None (see issue 91)
            return
        except socket.error, err:
            # ECONNABORTED might be thrown on *BSD (see issue 105)
            if err[0] != errno.ECONNABORTED:
                logerror(traceback.format_exc())
            return
        else:
            # sometimes addr == None instead of (ip, port) (see issue 104)
            if addr == None:
                return

Here's a rewriting attempt (not tested).
Now that I look at it I must say it's quite ugly, so I don't think we should follow this road.
An alternative I see is to return None in case of errors occurring on accept() and make this very clear in doc. 
Other than accept(), doc should explicitly show how to use handle_accept() in general, which would end up looking like this:

class SomeServer(asyncore.dispatcher):
    
    ...

    def handle_accept():
       conn = self.accept()
       if conn is None:
           return
       else:
           sock, addr = conn
       ...

    ...


A completely different approach could be to provide a new "TCPServer" class which deprecates direct accept() method usage. Something like this:


class TCPServer(asyncore.dispatcher):

    def __init__(self, ip, port, handler, interface='', map=None):
        asyncore.dispatcher.__init__(self, map=map)
        self.create_socket(family=socket.AF_INET, type=socket.SOCK_STREAM)
        self.bind((ip, port))
        self.listen(5)
        self.handler = handler

    def handle_accept(self):
        try:
            sock, addr = self.accept()
        except TypeError:
            return
        except socket.error, err:
            if err[0] != errno.ECONNABORTED:
                raise
            return
        else:
            if addr == None:
                return
        handler = self.handler(conn, self._map)

    def writable(self):
        return 0



...but for some reason I don't like it either. Point is asyncore API design is fundamentally wrong and there's nothing we can do about it unless we break backward compatibility or a brand new "asyncore2" module is written.

Patch in attachment makes accept() return None in case no connection takes place and modifies the doc to make this very clear, also providing an example on how an asyncore-based server should be properly set-up .

> EAGAIN can be raised too. I never experienced this error condition 
> myself in pyftpdlib

From the accept() man page:

EAGAIN or EWOULDBLOCK
              The  socket  is  marked  nonblocking and no connections are present to be accepted.
              POSIX.1-2001 allows either error to be returned for this case, and does not require
              these  constants to have the same value, so a portable application should check for
              both possibilities.

> The resulting address can be None, which means that the connection 
> didn't take place.

The only way this can happen is if the accept() system call returned 0 in addrlen, which sounds rather strange. I'm not convinced hiding operating system bugs is a good idea.

> I'm not convinced hiding operating system bugs is a good idea.

Do you propose to let the error raise then?
The point of frameworks such as asyncore and twisted is to hide all system-specific errors as much as possible and provide a portable interface across all platforms.
AFAICT, the whole point of this issue is that there should be only one way for an asyncore-based server to accept an incoming connection, possibly avoiding the user to deal with low-level details such as catching EWOULDBLOCK/ECONNABORTED/... in his application, and looking for accept() returning None is one possibility.

As I said, in a better designed framework the user shouldn't be supposed to call accept() at all, but that's how asyncore is designed.

> The point of frameworks such as asyncore and twisted is to hide all
> system-specific errors as much as possible and provide a portable
> interface across all platforms.

As long as these errors are reasonably explainable.
If a strange error only occurs when running nmap on an OS X server, then
it might be useful for the user to know that the OS X server isn't able
to service all connections properly due to a bug in the operating
system.

> AFAICT, the whole point of this issue is that there should be only one
> way for an asyncore-based server to accept an incoming connection,
> possibly avoiding the user to deal with low-level details such as
> catching EWOULDBLOCK/ECONNABORTED/...

I am talking specifically about the address being None (i.e., a (sock,
None) tuple is successfully returned).
EWOULDBLOCK and ECONNABORTED are documented error conditions for
accept(), but returning 0 in addrlen is not.

> As I said, in a better designed framework the user shouldn't be
> supposed to call accept() at all, but that's how asyncore is designed.

Perhaps it's time to add an alternative handle_accepted(sock, addr)
which relieves the user from calling accept() :))
Then, perhaps self.accept() shouldn't filter any errors at all, but the
default handle_accept() would silence them before calling
handle_accepted().

> As long as these errors are reasonably explainable.
> If a strange error only occurs when running nmap on an OS X server, 
> then it might be useful for the user to know that the OS X server 
> isn't able to service all connections properly due to a bug in the 
> operating system.

It might be useful to find whether this is tracked somewhere.
For the record, here (comment #10) is a code which should reproduce this problem:
https://bugs.launchpad.net/zodb/+bug/135108
As for what asyncore should in this case I think it should theoretically be safe for accept() to return (sock, None). What usually happen after that is that the socket object is passed to another dispatcher subclass (the handler) and the connection gets automatically closed once recv() or send() get called on the next async-loop.

> Perhaps it's time to add an alternative handle_accepted(sock, addr)
> which relieves the user from calling accept() :))

This sounds like a very good idea for 3.2.
As for 3.1 and 2.7 (2.6?) I think it can be useful to suppress EWOULDBLOCK/ECONNABORTED and return None instead.
Considering accept() can *already* return None it wouldn't break backward compatibility.

Patch in attachment adds a handled_accepted() method to dispatcher class as recommended by Antoine.

I'm not an asyncore expert, but I can't see anything wrong with the patch.

Python 3.2 changes committed in r85220.
Still have to commit EWOULDBLOCK/ECONNABORTED changes for 3.1 and 2.7.

CVE-2010-3492 references this issue.
http://web.nvd.nist.gov/view/vuln/detail?vulnId=CVE-2010-3492

Fixed in r86084 (2.7) and r86085 (3.1).

I do not see this as a security bug so no patch for 2.6 please.  (Comment requested from IRC).