Let callers supply their own dialer

client.go

@@ -149,6 +149,29 @@ type Config struct {
 	// hung connections.
 	DisableEPSV bool
 
+	// DialFunc, when set, is used to establish every TCP connection this
+	// client makes - control connections and data connections alike -
+	// in place of the package's own dialer.
+	//
+	// It exists for callers who need to see the connection itself rather
+	// than only what travels over it: counting bytes for transfer
+	// statistics, routing through a proxy or a tunnel, or handing back a
+	// connection that is not really a socket at all.
+	//
+	// Two things become the caller's responsibility when it is set,
+	// because the package can no longer do them:
+	//
+	//   - Timeout is not applied to the dial. Config.Timeout still
+	//     governs reads and writes on the returned connection; the dial
+	//     itself is timed however DialFunc chooses.
+	//   - For TLSImplicit mode the returned connection is wrapped in TLS
+	//     by this package, so DialFunc should return a plain connection.
+	//     A DialFunc that performs its own handshake and returns the TLS
+	//     connection will find it wrapped a second time.
+	//
+	// Leaving it nil keeps the previous behaviour exactly.
+	DialFunc func(network, address string) (net.Conn, error)
+
 	// For testing convenience.
 	stubResponses map[string]stubResponse
 }
@@ -371,13 +394,30 @@ func (c *Client) openConn(idx int, host string) (pconn *persistentConn, err erro
 
 	if c.config.TLSConfig != nil && c.config.TLSMode == TLSImplicit {
 		pconn.debug("opening TLS control connection to %s", host)
-		dialer := &net.Dialer{
-			Timeout: c.config.Timeout,
+		if c.config.DialFunc != nil {
+			// Handshake here rather than leaving it to the first read:
+			// tls.DialWithDialer does the same, and a caller reading
+			// the returned conn expects the negotiation to have either
+			// succeeded or failed by now.
+			var raw net.Conn
+			raw, err = c.config.DialFunc("tcp", host)
+			if err == nil {
+				tlsConn := tls.Client(raw, pconn.config.TLSConfig)
+				if err = tlsConn.Handshake(); err != nil {
+					raw.Close()
+				} else {
+					conn = tlsConn
+				}
+			}
+		} else {
+			dialer := &net.Dialer{
+				Timeout: c.config.Timeout,
+			}
+			conn, err = tls.DialWithDialer(dialer, "tcp", host, pconn.config.TLSConfig)
 		}
-		conn, err = tls.DialWithDialer(dialer, "tcp", host, pconn.config.TLSConfig)
 	} else {
 		pconn.debug("opening control connection to %s", host)
-		conn, err = net.DialTimeout("tcp", host, c.config.Timeout)
+		conn, err = c.config.dial("tcp", host)
 	}
 
 	var (

client_test.go

@@ -71,6 +71,7 @@ func TestExplicitTLS(t *testing.T) {
 }
 
 func TestImplicitTLS(t *testing.T) {
+	requireServers(t)
 	closer, err := startPureFTPD(implicitTLSAddrs, "ftpd/pure-ftpd-implicittls")
 	if err != nil {
 		t.Fatal(err)
@@ -108,6 +109,7 @@ func TestImplicitTLS(t *testing.T) {
 }
 
 func TestPooling(t *testing.T) {
+	requireServers(t)
 	config := Config{
 		ConnectionsPerHost: 2,
 		User:               "goftp",

dial_func_test.go

@@ -0,0 +1,96 @@
+package goftp
+
+import (
+	"errors"
+	"net"
+	"strings"
+	"testing"
+	"time"
+)
+
+// A connection that is not a socket, to prove the package uses whatever
+// DialFunc hands back rather than dialling for itself.
+type fakeConn struct{ net.Conn }
+
+var errRefused = errors.New("dial refused by the test")
+
+// Every connection the package opens goes through Config.dial, so a
+// DialFunc reaching it here reaches the data connections too. That is
+// the property worth pinning: a caller supplying one to count bytes
+// would otherwise silently miss every transfer, which is most of the
+// bytes there are.
+func TestConfigDialUsesDialFunc(t *testing.T) {
+	var got []string
+	cfg := Config{
+		DialFunc: func(network, address string) (net.Conn, error) {
+			got = append(got, network+" "+address)
+			return fakeConn{}, nil
+		},
+	}
+
+	conn, err := cfg.dial("tcp", "example.invalid:21")
+	if err != nil {
+		t.Fatalf("dial returned %v, want nil", err)
+	}
+	if _, ok := conn.(fakeConn); !ok {
+		t.Errorf("dial returned %T, want the connection DialFunc supplied", conn)
+	}
+	if len(got) != 1 || got[0] != "tcp example.invalid:21" {
+		t.Errorf("DialFunc saw %v, want one call for tcp example.invalid:21", got)
+	}
+}
+
+// An error from DialFunc is the caller's to see, unchanged. Wrapping it
+// would hide the reason a proxy or tunnel refused.
+func TestConfigDialPropagatesDialFuncError(t *testing.T) {
+	cfg := Config{
+		DialFunc: func(string, string) (net.Conn, error) { return nil, errRefused },
+	}
+	if _, err := cfg.dial("tcp", "example.invalid:21"); !errors.Is(err, errRefused) {
+		t.Errorf("dial returned %v, want %v", err, errRefused)
+	}
+}
+
+// Without one, nothing changes: the package dials for itself and applies
+// Config.Timeout, which DialFunc callers take over.
+func TestConfigDialWithoutDialFuncStillDials(t *testing.T) {
+	// Port 0 on the loopback address is never listening, so this
+	// exercises the built-in path and fails fast rather than hanging.
+	cfg := Config{Timeout: 100 * time.Millisecond}
+	if _, err := cfg.dial("tcp", "127.0.0.1:0"); err == nil {
+		t.Error("dialling a port nothing listens on should fail")
+	}
+}
+
+// The client must reach the hook rather than dialling around it.
+func TestClientControlConnectionUsesDialFunc(t *testing.T) {
+	called := 0
+	client, err := DialConfig(Config{
+		DialFunc: func(string, string) (net.Conn, error) {
+			called++
+			return nil, errRefused
+		},
+	}, "127.0.0.1:21")
+	if err != nil {
+		t.Fatalf("DialConfig returned %v", err)
+	}
+	defer client.Close()
+
+	// Any operation opens the control connection.
+	//
+	// The package wraps the failure in its own error type, which does
+	// not implement Unwrap, so the assertion is on the message it
+	// carries rather than on errors.Is. That the reason survives at all
+	// is what matters here: a caller whose proxy refused needs to be
+	// told why.
+	_, err = client.Getwd()
+	if err == nil {
+		t.Fatal("Getwd succeeded against a DialFunc that refuses")
+	}
+	if !strings.Contains(err.Error(), errRefused.Error()) {
+		t.Errorf("Getwd returned %q, which does not carry %q", err, errRefused)
+	}
+	if called == 0 {
+		t.Error("the client dialled without going through DialFunc")
+	}
+}

file_system_test.go

@@ -278,6 +278,7 @@ func TestReadDir(t *testing.T) {
 }
 
 func TestReadDirNoMLSD(t *testing.T) {
+	requireServers(t)
 	// pureFTPD seems to have some issues with timestamps in LIST output
 	for _, addr := range proAddrs {
 		config := goftpConfig
@@ -394,6 +395,7 @@ func TestStat(t *testing.T) {
 }
 
 func TestStatNoMLST(t *testing.T) {
+	requireServers(t)
 	// pureFTPD seems to have some issues with timestamps in LIST output
 	for _, addr := range proAddrs {
 		config := goftpConfig

goftp.go

@@ -35,6 +35,17 @@ func DialConfig(config Config, hosts ...string) (*Client, error) {
 	return newClient(config, expandedHosts), nil
 }
 
+// dial opens a connection with the caller's DialFunc if one was given,
+// and with a plain timed dial otherwise. Every connection this package
+// makes goes through here, so a DialFunc sees the data connections as
+// well as the control ones - which is the point of supplying it.
+func (c Config) dial(network, address string) (net.Conn, error) {
+	if c.DialFunc != nil {
+		return c.DialFunc(network, address)
+	}
+	return net.DialTimeout(network, address, c.Timeout)
+}
+
 var hasPort = regexp.MustCompile(`^[^:]+:\d+$|\]:\d+$`)
 
 func lookupHosts(hosts []string, ipv6Lookup bool) ([]string, error) {

main_test.go

@@ -31,7 +31,29 @@ var (
 	proAddrs         = []string{"127.0.0.1:2124"}
 )
 
+// Set by GOFTP_SKIP_SERVERS. Tests that need a live server call
+// requireServers, which skips them when it is set.
+var skipServers bool
+
+// requireServers skips the calling test when no FTP test server was
+// started. Without it such a test fails on a connection refused, which
+// looks like a bug in the package rather than a missing prerequisite.
+func requireServers(t *testing.T) {
+	t.Helper()
+	if skipServers {
+		t.Skip("GOFTP_SKIP_SERVERS is set; this test needs ./build_test_server.sh")
+	}
+}
+
 func TestMain(m *testing.M) {
+	// The tests that talk to a real server need one built by
+	// ./build_test_server.sh, which not every machine can run. Setting
+	// this skips them, so the package's unit tests stay reachable
+	// without that infrastructure.
+	if os.Getenv("GOFTP_SKIP_SERVERS") != "" {
+		skipServers = true
+		os.Exit(m.Run())
+	}
 	pureCloser, err := startPureFTPD(pureAddrs, "ftpd/pure-ftpd")
 	ftpdAddrs = append(ftpdAddrs, pureAddrs...)
 

persistent_connection.go

@@ -419,7 +419,7 @@ func (pconn *persistentConn) prepareDataConn() (func() (net.Conn, error), error)
 		}
 
 		pconn.debug("opening data connection to %s", host)
-		dc, netErr := net.DialTimeout("tcp", host, pconn.config.Timeout)
+		dc, netErr := pconn.config.dial("tcp", host)
 
 		if netErr != nil {
 			var isTemporary bool