Attached is a diff created using "svn diff" rather than an regular context diff.

By looking at your changes it seems that this kind of work is not supposed to 
take place in base FTPServer class, at least as it is currently implemented.
Passing a socket object instead of an address tuple is misleading in terms of 
API, so is socket_for_fd() method which looks like something which doesn't 
belong in FTPServer class.

An idea could be doing this in a separate InetdFTPServer class inheriting from 
FTPServer, providing a proper constructor (say __init__(self, socket, handler) 
and a replacement for serve_forever() method. Also, I'm not sure how 
InetdFTPServer should work with max_cons and max_cons_per_ip attributes.

Considering that by looking at your changes this doesn't look too complex, 
another possibility is to hack FTPServer class in your script and provide this 
functionnality as a recipe in the documentation.

I'm gonna try to be more precise.
Your changes basically require two things:
- FTPServer class should be able to accept a socket object
- server_forever should return after a certain time

The first requirement can be satisfied with an ugly hack as such:

<!-- snippet -->
def socket_for_fd(fd):
    # Unfortunately there is no way to tell if the file descriptor
    # represents an IPv6 socket without making the socket object
    ss = socket.fromfd(fd,socket.AF_INET, socket.SOCK_STREAM)
    if len(ss.getsockname()) == 4:
        ss = socket.fromfd(fd,socket.AF_INET6, socket.SOCK_STREAM)
    return ss

dummy_address = ("localhost", 0)
server = FTPServer(dummy_address, handler)
# toss the socket; we don't need it
server.close()  

sys.stdout = sys.stderr
# re-set the server socket
server.socket = socket_for_fd(sys.stdin.fileno())
<!-- /snippet -->

To achieve the second requirement (stop serving after a while) you can use 
current serve_forever() method as-is by specifying timeout and count arguments.
For example, this returns after 1 second:

server.serve_forever(timeout=0.1, count=10)

Despite we achieved the second goal I do not understand how this could work 
since when serve_forever() method returns all connected clients get 
disconnected (am I missing something?).

Upon reflection I think you are right that this does not need to be in 
FTPServer and it might be better to offer this up as a recipe instead. That 
said, there are a couple of changes that would make this sort of recipe much 
easier to implement and be much less of an ugly hack.

The solution I think is to allow an FTPServer object to be created with None 
supplied as the address. In this case the object would be created with no 
server socket at all, it would just have the infrastructure for acting as a 
server. We would then an another method that allows a server socket to be 
specified later.  Thus the code for running in inetd mode would just look like:

<!-- snippet -->
server = FTPServer(dummy_address, handler)
server.socket = socket_for_fd(sys.stdin.fileno())
<!-- /snippet -->

A useful side-effect of going this path is that we can then offer a second 
recipe for operating in the inetd 'nowait' mode, simply by never adding a 
server socket but instead adding just an FTPHandler object built on a socket 
constructed from stdin.

With regard to the server_forever() method, the goal is for an inetd 'wait' 
service to exit once all the client connections have quit, even though the 
server socket is still alive. This can indeed be synthesized using the 'count' 
parameter but it seems of sufficiently general use that I thought it was worth 
putting in the main code.

> With regard to the server_forever() method, the goal is for an inetd 'wait' service 
> to exit once all the client connections have quit, even though the server socket is 
> still alive.

Ok, clear. In this case the "hack" should look like this, more or less (not tested):

class InetdFTPServer(FTPServer):

    def __init__(socket):
        ...

    def serve_forever(self, timeout=10):
        """Start and keep serving until all clients gets disconnected.
        When no clients are connected the server will still be listening
        for the period specified in timeout, which is expressed in 
        seconds
        """
        # being the timeout expressed in seconds, multiply * 10
        timeout = timeout * 10
        while 1:
            # loop for 10 seconds
            FTPServer.serve_forever(self, timeout=0.1, count=timeout)
            if len(asyncore.socket_map) <= 1:
                # listening socket is the only one left; exit
                break
        FTPServer.close_all()


Again, deciding whether to modify FTPServer class at this point is premature, IMO.
We/you should develop this in a complete different class for now, which spares 
us FTPServer refactoring.
Also, it would be good to provide the documentation which explains how exactly 
to make this work with inetd/xinetd, also because, as I said, I'm ignorant 
about that.

If you want I can create a branch in the SVN repository for you, so that you 
can work properly.
Most likely, you should add a new pyftpdlib/contrib/servers.py file within a 
InetdFTPServer class which implements what discussed so far, providing a 
docstring which explains the usage, possibly also in respect of 
inetd/xinetd/launchd.
If you want to write tests as well, those should go into test/test_contrib.py.
As for other functionalities already implemented, we might also have a 
inetd_ftpserver.py script in demo directory.


Note: adding Ben Timby to CC list as he should know how inet* works.

I think you guys are on the right track, however, I have a couple of 
suggestions.

Like Giampaolo, I would suggest not modifying the library proper, however, 
create a standalone implementation for inclusion in the contrib/ directory.

To support nowait services, main() could accept a command line parameter like 
--nowait. In this case the received socket would be fed directly to an 
FTPHandler (overridden like FTPServer) sys.exit() called when the client goes 
away would complete this.

The only other thing I see is that you are missing OUTPUT. stdout is also 
needed to talk back to the client.

Thus, to have a suitable socket you need both. You can solve this fairly easily 
by creating a socket-like object to serve as stand-in.

The implementation would:

1. Create a socket stand-in.
2. Override FTPServer.__init__() to accept a pre-made socket.
3. Override FTPServer.serve_forever() to die after servicing clients (as 
Giampaolo suggested).
4. Provide a main() function get the socket, create InetFTPServer(), call 
serve_forever().

--
def socket_for_fd(fd):
    # Unfortunately there is no way to tell if the file descriptor
    # represents an IPv6 socket without making the socket object
    ss = socket.fromfd(fd,socket.AF_INET, socket.SOCK_STREAM)
    if len(ss.getsockname()) == 4:
        ss = socket.fromfd(fd,socket.AF_INET6, socket.SOCK_STREAM)
    return ss

class StdSocket():
    def __init__(self, sin, sout):
        self.sin = socket_for_fd(sin)
        self.sout = socket_for_fd(sout)

    def write(...):
        self.sout.write(...)

    def read(...):
        self.sin.read(...)

class InetFTPServer(pyftpdlib.FTPServer):
    def __init__(...):
        pass

    def serve_forever(...):
        pass

class InetFTPHandler(pyftpdlib.FTPHandler):
    def __init__(...):
        pass

def main():
    parser = optparse.OptionParser()

    parser.add_option('-t', '--timeout', default=10, type='int',
                      help="Die after X seconds of inactivity.")
    parser.add_option('-n', '--nowait', default=10, action='store_true',
                      help="Handle a single client connection then die.")

    socket = StdSocket(sys.stdin.fileno(), sys.stdout.fileno())
    if options.nowait:
        handler = InetFTPHandler(socket)
    else:
        server = InetFTPServer(socket)
        server.serve_forever(options.timeout)

if __name__ == '__main__':
    main()
--

Sorry, on last thing, the socket-like object might have to proxy some of it's 
attributes to underlying socket...

class StdSocket:
    def __getattr__(self, attr):
        return getattr(self.sin, attr)

    def __setattr__(self, attr, value):
        setattr(self.sin, attr, value)
        setattr(self.sout, attr, value)

or something...

Ben, I think you a misunderstanding the way that inetd services handle stdin 
and stdout. The file descriptors passed for these are both duplicates of the 
same socket. Thus there is no need to go to all the complexity of constructing 
these complex socket-like objects made out of two descriptors. Since the stdin 
and stdout refer to the same underlying object you can use either one as a 
bi-directional stream socket.

With regard to refactoring FTPServer, this comes down to a philosophical design 
decision. Clearly all the things that I'm trying to do can be accomplished 
through the use of ugly hacks. My motivation for suggesting changes to the 
structure, and potentially to the class hierarchy, of the FTPServer object was 
to allow these things to be done without having to resort to duplicating large 
swaths of code from the parent classes within the sub-classes. If I simply make 
a subclass of the existing FTPServer that creates it's sockets some other way 
then I will have to duplicate all the code in FTPServer.__init__ that does the 
set-up that follows the socket creation.  If you subsequently change this code 
then the sub-class will break. If, on the other hand, the code in 
FTPServer.__init__ is split into to part that creates the socket and a 
separate, callable part that sets up state for a given socket then the 
sub-class will still work as long as you provide a hook that allows a socket to 
be set. It seems to me that this is a much cleaner solution.

I will look into creating a branch in SVN and put together a patch version that 
keeps the changes to FPTServer to a minimum. You can then decide if you care to 
merge it back into the trunk.

Gotcha, did not realize the fd was the actual bi-direction socket, assumed it 
was just a pipe to inetd.

In the name of minimum invasion, the only change needed to FTPServer.__init__() 
*might* be:

- def __init__(self, address, handler):
+ def __init__(self, address, handler, socket=None):
      asyncore.dispatcher.__init__(self)

      self.handler = handler

      self.ip_map = []
+     if socket:
+         self.socket = socket
+         return


I would like to change address to a kwarg, but that would break the interface. 
In this case, your subclass:

def __init__(self, socket):
    FTPServer.__init__(self, None, FTPHandler, socket=socket)

It just pains me to think of passing a dummy_address to be opened and then 
immediately closed (or garbage collected).

I'd be OK with the model of
   def __init__(self, address, handler, socket=None):
This is in fact very similar, but rather cleaner, than the model in the patch I 
put in (in which the address and socket were passed in the same variable and 
the code tested which is was).

Having said that, I would still prefer a model in which I could just pass 
"None" for the address, have no socket created at all, and have a separate call 
that allows the user to set the socket. While this means that the inetd server 
needs one extra line of code, it also provides a very clean way to create an 
FTPServer with no socket at all. The FTPServer object handles both the asyncore 
dispatch and the task scheduling, and if you want to do an inetd 'nowait' 
service you need both of these things, so being able to create an FTPServer 
without any socket is a useful thing to be able to do.

That makes sense... so...

--
  def __init__(self, address, handler):
      asyncore.dispatcher.__init__(self)
      self.handler = handler
      self.ip_map = []
+     if address is None:
+         return

  def set_socket(self, socket):
      self.socket = socket
--

The only downside I see to this is that the socket object is then None if the 
caller does not call set_socket(). Some form of graceful handling would need to 
be implemented.

If no server socket is set _and_ no FTPHandler is attached manually then as far 
as I can tell nothing particularly bad will happen other than calls to 
server_forever() will return immediately when called. If, however, no server 
socket is set but an FTPHandler is added then server_forever() will serve that 
handler for as long as it is active, which is exactly what you want to happen 
for an nowait inetd child process.

> If, on the other hand, the code in FTPServer.__init__ is split into to part that 
> creates the socket and a separate, callable part that sets up state for a given 
> socket then the sub-class will still work as long as you provide a hook that allows 
> a socket to be set. It seems to me that this is a much cleaner solution.

I guess there's no harm in doing this. Feel free to do it in your patch.
Btw, I created a branch. You should be able to access it by doing:

svn checkout https://pyftpdlib.googlecode.com/svn/branches/inetd 
pyftpdlib-inetd --username nickovs@googlemail.com

OK. I have checked in an update on the inetd branch. The patch keeps the 
changes to the FTPServer class to a minimum; the constructor now accepts None 
as the address and won't create a server socket if this is the case, there is a 
new method set_server_socket() that allows the server socket to be provided 
explicitly after creation and I have included the idle_timeout functionality in 
the server_forever() method. The snippet of code for turning a Python file 
handle into a Python socket has been put into a utility function and parked 
with the other utility functions near the start of the file. I have update 
main() to handle being called from inetd in both wait and nowait modes.

I have tested calling the server from launchd in both modes and it works just 
fine. I have not tested it from inetd or xinetd since I don't have easy access 
to a machine that launches services that way but I'll see if I can find someone 
with a Linux box to try this. Once I've done this I will write up a recipe for the Wiki.

I have now added a page to the Wiki describing how to use pyftpdlib from inetd: 
https://code.google.com/p/pyftpdlib/wiki/InetdSupport

Thanks, documentation looks good (note: I've marked it as "deprecated" so that 
it is hidden for now).
As I said earlier, I prefer this to be done in a separate class though, either 
in form of a new contrib/servers.py module or a new demo/inetd_ftpd.py script.

I'm ok with making FTPServer constructor accept None as address but all other 
changes (including the cmdline parser, which should not be modified) should 
take place in the subclass and/or the script.

About r769 : you're passing a brand new FTPHandler object to FTPServer. I think 
you should pass the pre-configured one instead:
- ftpd = FTPServer(None, FTPHandler)
+ ftpd = FTPServer(None, handler)

I also think serve_forever() should be refactored in the subclass (see my 
comment #5 
https://code.google.com/p/pyftpdlib/issues/detail?id=147&colspec=ID%20Summary%20Type%20Status%20Priority%20Milestone%20Opened#c4 ).

Aha, "deprecated". I was wondering what the best way to hide a not-yet-ready 
wiki page and didn't think of marking it as being at the other end of its life cycle.

I'll shift all this stuff to a different class.  That said, I still think that 
having a more flexible way to determine when the service loop should quit would 
be a generally useful function that is orthogonal to the problem of inetd.

Since you want the ability to run this as a stand-alone command line program to 
be moved into a totally different script, with your permission I'd like to 
re-factor the option parser code. It pains me to cut-and-paste all of the 
parsing of the stack of command line options that you already support into a 
different class.  Do you mind if I place the large block of add_options() calls 
into one method and put the interpretation of the options for the authoriser, 
handler and server options into their own methods? That way when people roll 
their own variants of your server all of the standard options will always be 
supported and interpreted in a consistent manner?

As for the FTPHandler that gets passed, in your original code the variable 
'handler' jus refers to the class FTPHandler itself, not a handler object. The 
authoriser, NAT address and passive ports are all attached to the base class 
itself and the diff you suggest make no semantic change. In many ways I think 
the "correct" code should be to make the following change:
  - handler = FTPHandler
  + class handler(FTPHandler):
  +     pass
so that the changes that you then make to 'handler' don't alter the underlying 
FTPServer object for other users, but since you're only doing this in the case 
that the program is run as a stand-along it doesn't make any difference.

Including this as part of the cmdline parser should be low priority for now, 
also because it seems that in order to start the whole thing just a couple of 
lines of code are required.
We first have to define a consistent API, figure out all the possible use cases 
(which should be just 2: wait and not-wait mode) and possibly also write tests.

What I have in mind is this:

- pyftpdlib/contrib/servers.py within a InetdFTPServer class
  - constructor accepts just one argument: the handler
  - serve_forever accepts just one argument: "idle_timeout"
- demo/inetd_ftpd.py which imports InetdFTPServer class and show one of the two usages. 
- wiki/ InetdSupport which explains everything in details (more or less as it 
is right now, just reflecting the new code in accordance)

Is there still interest in this feature? We (at work) have used some adapted 
code from the above when we were running an earlier pyftpdlib version, but with 
1.2 we switched to Multiprocess instead.

We've found we miss the ability to make code changes only apply to new 
connections, but that it's actually reasonably easy to do without changing core code:


import sys
import socket
import os
from pyftpdlib.servers import FTPServer
from pyftpdlib.authorizers import DummyAuthorizer
from pyftpdlib.handlers import FTPHandler

def socket_for_fd(fd):
    ss = socket.fromfd(fd,socket.AF_INET, socket.SOCK_STREAM)
    if len(ss.getsockname()) == 4:
        ss = socket.fromfd(fd,socket.AF_INET6, socket.SOCK_STREAM)
    return ss

if __name__ == "__main__":
    authorizer = DummyAuthorizer()
    authorizer.add_user('user', '12345', os.getcwd(), perm='elradfmw')
    sys.stderr = open('/dev/null', 'w')
    ftp_handler = FTPHandler
    ftp_handler.authorizer = authorizer
    sys.stdout = sys.stderr
    s = socket.socket()
    ftpd = FTPServer(s, FTPHandler)
    ftp_connection = ftp_handler(socket_for_fd(0), ftpd)
    ftp_connection.handle()
    ftpd.ioloop.unregister(s.fileno())
    s.close()
    ftpd.ioloop.loop(None, True)


If I could wrap this up more tidily is it likely to be included in trunk?

It is a long time I don't look at this and I forgot the details.
I should re-read this whole thread in order to figure out where we were at but 
*as of right now* I'm in the process of applying for different jobs so I don't 
have enough time.

AFAICT I would still definitively welcome this feature as long as it gets 
exposed in a nice API so if you can study the problem and provide a patch I 
will try to at least review it.

Some considerations:

- since starting from 1.x FTPServer also accepts a pre-existing socket object 
instead an address tuple we probably don't need e separate InetdFTPServer class 
anymore (as I was suggesting above)

- judging from the code you pasted everything looks pretty straightforward so 
perhaps this might even be provided in form of documentation (as opposed to 
writing new code)?