rsmills36 / pyftpdlib

Automatically exported from code.google.com/p/pyftpdlib
Other
0 stars 0 forks source link

Socket type check insufficient #265

Closed GoogleCodeExporter closed 9 years ago

GoogleCodeExporter commented 9 years ago
What steps will reproduce the problem?

I am using circus to manage my FTP server processes. Circus has the ability to 
pre-bind the socket and pass it to the FTP server via the command line.

http://circus.readthedocs.org/en/latest/sockets/

Using this capability, I am doing the following:

--
def make_socket(value):
    try:
        host, port = value.split(':')
        port = int(port)
    except (ValueError, IndexError):
        try:
            fd = int(value)
        except ValueError:
            raise Exception('Invalid bind directive "%s"' % value)
        else:
            sock = socket.fromfd(fd, socket.AF_INET, socket.SOCK_STREAM)
    else:
        sock = socket.socket(socket.AF_INET, socket.SOCK_STREAM)
        sock.bind((host, port))
    return sock

# Note, args are command line parameters.
server = FTPServer(make_socket(args[1], handler)
--

However, the check in servers.py fails to detect the socket, as it's type is 
_socket.socket, not socket.socket.

http://code.google.com/p/pyftpdlib/source/browse/trunk/pyftpdlib/servers.py#133

What is the expected output? What do you see instead?

This causes self.bind_af_unspecified(address_or_socket) to be called on the 
_socket.socket instance, which fails.

If I replace:

if isinstance(address_or_socket, socket.socket):

with:

if hasattr(address_or_socket, 'listen'):

Then all is well.

Original issue reported on code.google.com by btimby@gmail.com on 2 Jul 2013 at 2:16

Attachments:

GoogleCodeExporter commented 9 years ago
Mmm... I'm not sure I like the hasattr() check. I would first investigate into 
why circus uses  _socket.socket rather than socket.socket, which is kind of 
unusual, and possibly fix this in circus.

I have seen Tarek (author of circus) at Euro Python. I guess I can ask him 
directly. =)
More later.

Original comment by g.rodola on 3 Jul 2013 at 12:27

GoogleCodeExporter commented 9 years ago
It's not circus that uses _socket.socket, it is the socket module.
_socket.socket is what is returned by socket.fromfd().

hasattr is pretty standard duck typing, we want to listen() using the
object passed in, so make sure it is capable of listen()ing...

Original comment by btimby@gmail.com on 3 Jul 2013 at 7:35

GoogleCodeExporter commented 9 years ago
[deleted comment]
GoogleCodeExporter commented 9 years ago
Some I frequently do is like this...

if callable(getattr(address_or_socket, 'listen', None)):
    ...

To make sure listen is a method and not another type of attribute.

Original comment by btimby@gmail.com on 4 Jul 2013 at 1:13

GoogleCodeExporter commented 9 years ago
Any more feedback?

Original comment by btimby@gmail.com on 11 Jul 2013 at 5:19

GoogleCodeExporter commented 9 years ago
It looks like socket.fromfd() started returning a socket.socket instance 
starting from Python 3.3 while on Python 2.7 we still get socket._socket.
Please go ahead and apply your initial patch.

Original comment by g.rodola on 12 Jul 2013 at 10:22

GoogleCodeExporter commented 9 years ago
I committed a fix:

http://code.google.com/p/pyftpdlib/source/detail?r=1230

Original comment by btimby@gmail.com on 16 Jul 2013 at 4:08

GoogleCodeExporter commented 9 years ago

Original comment by g.rodola on 6 Nov 2013 at 8:31

GoogleCodeExporter commented 9 years ago

Original comment by g.rodola on 8 Nov 2013 at 7:52