Close the data connection when a transfer command fails

[christhomas]

Fixes #46.

prepareDataConn opens the data connection before the transfer command is sent — it has to, because active mode needs the port in hand to tell the server about. Nothing closes it until the getter has handed it over, and the deferred dc.Close() is only set up after that.

A command the server refuses returns in between, exactly as @yehaoyu-e pointed out:

connGetter, err := pconn.prepareDataConn()   // connection is open from here
...
err = pconn.sendCommandExpected(replyGroupPreliminaryReply, "%s %s", cmd, path)
if err != nil {
    return 0, err                            // ...and lost here
}

dc, err := connGetter()
...
defer dc.Close()                             // only reached on success

Measured rather than estimated

Counting /proc/self/fd across 25 failed retrieves:

pure-ftpd: 9 descriptors before, 34 after  — a leak of 25
proftpd:  35 descriptors before, 60 after  — a leak of 25

One per failed transfer, on both ends of the wire.

The fix

prepareDataConn returns an abort alongside the getter, and the internal callers defer it. It is a no-op once the getter has run, since the caller owns the connection from then on. Both the active-mode listener and the passive-mode connection are covered.

RawConn.PrepareDataConn keeps its signature. Widening it would break every implementation of that interface, and a caller driving the protocol by hand owns the connection either way — that is now said in a comment where someone would look for it.

dataStringList in file_system.go had the same shape and the same leak. Found while fixing the reported one, and fixed with it.

One note on running the test: it counts descriptors via /proc/self/fd, so it skips on anything but Linux. This branch is off master, so TestMain still needs ./build_test_server.sh; I verified via the container harness in #73.