From 170427b131c7771ce10dd0f19116bffe539a609b Mon Sep 17 00:00:00 2001 From: Randy Reddig Date: Wed, 20 Jun 2018 12:39:09 -0700 Subject: [PATCH 01/37] use 'package goftp' in tests so this fork will work --- examples_test.go | 12 +++++------- parallel_walk_test.go | 10 ++++------ 2 files changed, 9 insertions(+), 13 deletions(-) diff --git a/examples_test.go b/examples_test.go index c389b26..7130f8f 100644 --- a/examples_test.go +++ b/examples_test.go @@ -2,7 +2,7 @@ // Use of this source code is governed by a MIT-style // license that can be found in the LICENSE file. -package goftp_test +package goftp import ( "bytes" @@ -10,13 +10,11 @@ import ( "io/ioutil" "os" "time" - - "github.com/secsy/goftp" ) func Example() { // Create client object with default config - client, err := goftp.Dial("ftp.example.com") + client, err := Dial("ftp.example.com") if err != nil { panic(err) } @@ -45,7 +43,7 @@ func Example() { } func Example_config() { - config := goftp.Config{ + config := Config{ User: "jlpicard", Password: "beverly123", ConnectionsPerHost: 10, @@ -53,7 +51,7 @@ func Example_config() { Logger: os.Stderr, } - client, err := goftp.DialConfig(config, "ftp.example.com") + client, err := DialConfig(config, "ftp.example.com") if err != nil { panic(err) } @@ -69,7 +67,7 @@ func Example_config() { func ExampleClient_OpenRawConn() { // ignore errors for brevity - client, _ := goftp.Dial("ftp.hq.nasa.gov") + client, _ := Dial("ftp.hq.nasa.gov") rawConn, _ := client.OpenRawConn() diff --git a/parallel_walk_test.go b/parallel_walk_test.go index e344e2b..d68932d 100644 --- a/parallel_walk_test.go +++ b/parallel_walk_test.go @@ -2,7 +2,7 @@ // Use of this source code is governed by a MIT-style // license that can be found in the LICENSE file. -package goftp_test +package goftp import ( "fmt" @@ -10,14 +10,12 @@ import ( "path" "path/filepath" "sync/atomic" - - "github.com/secsy/goftp" ) // Just for fun, walk an ftp server in parallel. I make no claim that this is // correct or a good idea. func ExampleClient_ReadDir_parallelWalk() { - client, err := goftp.Dial("ftp.hq.nasa.gov") + client, err := Dial("ftp.hq.nasa.gov") if err != nil { panic(err) } @@ -25,7 +23,7 @@ func ExampleClient_ReadDir_parallelWalk() { Walk(client, "", func(fullPath string, info os.FileInfo, err error) error { if err != nil { // no permissions is okay, keep walking - if err.(goftp.Error).Code() == 550 { + if err.(Error).Code() == 550 { return nil } return err @@ -39,7 +37,7 @@ func ExampleClient_ReadDir_parallelWalk() { // Walk a FTP file tree in parallel with prunability and error handling. // See http://golang.org/pkg/path/filepath/#Walk for interface details. -func Walk(client *goftp.Client, root string, walkFn filepath.WalkFunc) (ret error) { +func Walk(client *Client, root string, walkFn filepath.WalkFunc) (ret error) { dirsToCheck := make(chan string, 100) var workCount int32 = 1 From 4dab2f90adb77de122d311dd91f0f140abf627ff Mon Sep 17 00:00:00 2001 From: Randy Reddig Date: Wed, 20 Jun 2018 15:19:13 -0700 Subject: [PATCH 02/37] Dial with (optional) Config.DialContext This enables clients to specify their own connection logic, to permit creation of connections through a proxy (for IP whitelisting). --- client.go | 29 +++++++++++++++++++++-------- client_test.go | 13 +++++++++++++ persistent_connection.go | 10 +++++++++- 3 files changed, 43 insertions(+), 9 deletions(-) diff --git a/client.go b/client.go index 779cd5e..3eedfdd 100644 --- a/client.go +++ b/client.go @@ -5,6 +5,7 @@ package goftp import ( + "context" "crypto/tls" "errors" "fmt" @@ -105,6 +106,10 @@ type Config struct { // concurrent transfers. ConnectionsPerHost int + // DialContext specifies the dial function for creating unencrypted TCP connections. + // If DialContext is nil, then the client dials using package net. + DialContext func(ctx context.Context, network, addr string) (net.Conn, error) + // Timeout for opening connections, sending control commands, and each read/write // of data transfers. Defaults to 5 seconds. Timeout time.Duration @@ -173,6 +178,11 @@ type Client struct { // Construct and return a new client Conn, setting default config // values as necessary. func newClient(config Config, hosts []string) *Client { + if config.DialContext == nil { + config.DialContext = (&net.Dialer{ + Timeout: config.Timeout, + }).DialContext + } if config.ConnectionsPerHost <= 0 { config.ConnectionsPerHost = 5 @@ -369,15 +379,18 @@ func (c *Client) openConn(idx int, host string) (pconn *persistentConn, err erro var conn net.Conn + ctx := context.Background() + if c.config.Timeout != 0 { + var cancel context.CancelFunc + ctx, cancel = context.WithTimeout(ctx, c.config.Timeout) + defer cancel() + } + + pconn.debug("opening control connection to %s", host) + conn, err = c.config.DialContext(ctx, "tcp", host) 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, - } - 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) + pconn.debug("configuring implicit TLS control connection to %s", host) + conn = tls.Client(conn, pconn.config.TLSConfig) } var ( diff --git a/client_test.go b/client_test.go index ff14c14..710c7c7 100644 --- a/client_test.go +++ b/client_test.go @@ -6,7 +6,9 @@ package goftp import ( "bytes" + "context" "crypto/tls" + "net" "sync" "testing" "time" @@ -40,9 +42,16 @@ func TestTimeoutConnect(t *testing.T) { func TestExplicitTLS(t *testing.T) { for _, addr := range ftpdAddrs { + var gotAddr string config := Config{ User: "goftp", Password: "rocks", + DialContext: func(ctx context.Context, network, addr string) (net.Conn, error) { + if gotAddr == "" { + gotAddr = addr + } + return (&net.Dialer{}).DialContext(ctx, network, addr) + }, TLSConfig: &tls.Config{ InsecureSkipVerify: true, }, @@ -64,6 +73,10 @@ func TestExplicitTLS(t *testing.T) { t.Errorf("Got %v", buf.Bytes()) } + if gotAddr != addr { + t.Errorf("Expected dial to %s, got %s", addr, gotAddr) + } + if c.numOpenConns() != len(c.freeConnCh) { t.Error("Leaked a connection") } diff --git a/persistent_connection.go b/persistent_connection.go index 15f5d74..c76fc69 100644 --- a/persistent_connection.go +++ b/persistent_connection.go @@ -6,6 +6,7 @@ package goftp import ( "bufio" + "context" "crypto/tls" "fmt" "net" @@ -418,8 +419,15 @@ func (pconn *persistentConn) prepareDataConn() (func() (net.Conn, error), error) return nil, err } + ctx := context.Background() + if pconn.config.Timeout != 0 { + var cancel context.CancelFunc + ctx, cancel = context.WithTimeout(ctx, pconn.config.Timeout) + defer cancel() + } + pconn.debug("opening data connection to %s", host) - dc, netErr := net.DialTimeout("tcp", host, pconn.config.Timeout) + dc, netErr := pconn.config.DialContext(ctx, "tcp", host) if netErr != nil { var isTemporary bool From c48a417b6d4acaebaff27509f26587fba4d9f782 Mon Sep 17 00:00:00 2001 From: Randy Reddig Date: Thu, 21 Jun 2018 11:15:45 -0700 Subject: [PATCH 03/37] Use pconn.host instead of pconn.controlConn.RemoteAddr(). Not all net.Conn implementations provide a valid RemoteAddr(), in particular crypto/ssh.Conn: https://godoc.org/golang.org/x/crypto/ssh#Client.Dial --- persistent_connection.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/persistent_connection.go b/persistent_connection.go index c76fc69..4b0ee35 100644 --- a/persistent_connection.go +++ b/persistent_connection.go @@ -311,7 +311,7 @@ func (pconn *persistentConn) requestPassive() (string, error) { goto PASV } - remoteHost, _, err = net.SplitHostPort(pconn.controlConn.RemoteAddr().String()) + remoteHost, _, err = net.SplitHostPort(pconn.host) if err != nil { pconn.debug("failed determining remote host: %s", err) goto PASV From 530ab035ff37b1ef6068f4c027a905f1dbb18bdb Mon Sep 17 00:00:00 2001 From: Randy Reddig Date: Thu, 21 Jun 2018 12:02:01 -0700 Subject: [PATCH 04/37] Add Transport type that implements http.RoundTripper. This enables clients to handle GET requests of ftp:// or ftps:// URLs with a normal http.Client using http.Transport.RegisterProtocol(). --- transport.go | 63 +++++++++++++++++++++++++ transport_test.go | 115 ++++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 178 insertions(+) create mode 100644 transport.go create mode 100644 transport_test.go diff --git a/transport.go b/transport.go new file mode 100644 index 0000000..fa52ab4 --- /dev/null +++ b/transport.go @@ -0,0 +1,63 @@ +package goftp + +import ( + "crypto/tls" + "io" + "net/http" + "strings" +) + +// Transport implements the http.RoundTripper interface. +// Typical usage would be to register a Transport to handle +// ftp:// and/or ftps:// URLs with http.Transport.RegisterProtocol. +type Transport struct { + Config +} + +// RoundTrip implements the http.RoundTripper interface to allow an http.Client +// to handle ftp:// or ftps:// URLs. +func (t *Transport) RoundTrip(req *http.Request) (*http.Response, error) { + config := t.Config + switch req.URL.Scheme { + default: + return nil, http.ErrSkipAltProtocol + case "ftp": + case "ftps": + if config.TLSConfig == nil { + config.TLSConfig = &tls.Config{} + } + } + + if req.URL.User != nil { + if user := req.URL.User.Username(); user != "" { + config.User = user + } + if password, ok := req.URL.User.Password(); ok { + config.Password = password + } + } + + path := strings.TrimPrefix(req.URL.Path, "/") + + client, err := DialConfig(config, req.URL.Host) + if err != nil { + return nil, err + } + + res := &http.Response{} + switch req.Method { + default: + return nil, http.ErrNotSupported + case http.MethodGet: + // Pipe Client.Retrieve to res.Body so enable unbuffered reads + // of large files. + // Errors returned by Client.Retrieve (like the size check) + // will be returned by res.Body.Read(). + r, w := io.Pipe() + res.Body = r + go func() { + w.CloseWithError(client.Retrieve(path, w)) + }() + } + return res, err +} diff --git a/transport_test.go b/transport_test.go new file mode 100644 index 0000000..91a9f3e --- /dev/null +++ b/transport_test.go @@ -0,0 +1,115 @@ +package goftp + +import ( + "bytes" + "crypto/tls" + "io/ioutil" + "net/http" + "net/url" + "testing" + "time" +) + +func TestTransportTimeoutConnect(t *testing.T) { + config := Config{Timeout: 100 * time.Millisecond} + transport := Transport{Config: config} + + req, err := http.NewRequest(http.MethodGet, "ftp://168.254.111.222:2121/subdir/1234.bin", nil) + if err != nil { + t.Fatal(err) + } + + t0 := time.Now() + res, err := transport.RoundTrip(req) + // Transport.RoundTrip calls Client.Retrieve in a goroutine + // so large file reads are unbuffered. + _, err = ioutil.ReadAll(res.Body) + res.Body.Close() + delta := time.Now().Sub(t0) + if err == nil || !err.(Error).Temporary() { + t.Error("Expected a timeout error") + } + + offBy := delta - config.Timeout + if offBy < 0 { + offBy = -offBy + } + if offBy > 50*time.Millisecond { + t.Errorf("Timeout of 100ms was off by %s", offBy) + } +} + +func TestTransportExplicitTLS(t *testing.T) { + for _, addr := range ftpdAddrs { + config := Config{ + TLSConfig: &tls.Config{ + InsecureSkipVerify: true, + }, + TLSMode: TLSExplicit, + } + transport := Transport{Config: config} + + req, err := http.NewRequest(http.MethodGet, "ftp://"+addr+"/subdir/1234.bin", nil) + if err != nil { + t.Fatal(err) + } + + req.URL.User = url.UserPassword("goftp", "rocks") + + res, err := transport.RoundTrip(req) + if err != nil { + t.Fatal(err) + } + + b, err := ioutil.ReadAll(res.Body) + res.Body.Close() + if err != nil { + t.Fatal(err) + } + + if !bytes.Equal([]byte{1, 2, 3, 4}, b) { + t.Errorf("Got %v", b) + } + } +} + +func TestTransportImplicitTLS(t *testing.T) { + closer, err := startPureFTPD(implicitTLSAddrs, "ftpd/pure-ftpd-implicittls") + if err != nil { + t.Fatal(err) + } + + defer closer() + + for _, addr := range implicitTLSAddrs { + config := Config{ + TLSConfig: &tls.Config{ + InsecureSkipVerify: true, + }, + TLSMode: TLSImplicit, + } + transport := Transport{Config: config} + + req, err := http.NewRequest(http.MethodGet, "ftp://"+addr+"/subdir/1234.bin", nil) + if err != nil { + t.Fatal(err) + } + + req.URL.User = url.UserPassword("goftp", "rocks") + + res, err := transport.RoundTrip(req) + if err != nil { + t.Fatal(err) + } + + b, err := ioutil.ReadAll(res.Body) + res.Body.Close() + if err != nil { + t.Fatal(err) + } + + if !bytes.Equal([]byte{1, 2, 3, 4}, b) { + t.Errorf("Got %v", b) + } + } +} From f52b2af6ec86863e5c52e67e18045d30d72a9308 Mon Sep 17 00:00:00 2001 From: Randy Reddig Date: Thu, 21 Jun 2018 14:27:17 -0700 Subject: [PATCH 05/37] test for ErrSkipAltProtocol --- transport_test.go | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/transport_test.go b/transport_test.go index 91a9f3e..0a4b7ad 100644 --- a/transport_test.go +++ b/transport_test.go @@ -10,6 +10,20 @@ import ( "time" ) +func TestTransportSkipAltProtocol(t *testing.T) { + transport := Transport{} + + req, err := http.NewRequest(http.MethodGet, "foo://localhost/foo.txt", nil) + if err != nil { + t.Fatal(err) + } + + _, err = transport.RoundTrip(req) + if err != http.ErrSkipAltProtocol { + t.Errorf("Expected err = %v, got %v", http.ErrSkipAltProtocol, err) + } +} + func TestTransportTimeoutConnect(t *testing.T) { config := Config{Timeout: 100 * time.Millisecond} transport := Transport{Config: config} From ba054c4b3dd14554d33e07b30397efe2e2ffba40 Mon Sep 17 00:00:00 2001 From: Randy Reddig Date: Wed, 11 Jul 2018 09:30:07 -0700 Subject: [PATCH 06/37] Address feedback: set up default DialContext after defaulting Timeout --- client.go | 20 ++++++++++---------- 1 file changed, 10 insertions(+), 10 deletions(-) diff --git a/client.go b/client.go index 3eedfdd..819e612 100644 --- a/client.go +++ b/client.go @@ -106,14 +106,14 @@ type Config struct { // concurrent transfers. ConnectionsPerHost int - // DialContext specifies the dial function for creating unencrypted TCP connections. - // If DialContext is nil, then the client dials using package net. - DialContext func(ctx context.Context, network, addr string) (net.Conn, error) - // Timeout for opening connections, sending control commands, and each read/write // of data transfers. Defaults to 5 seconds. Timeout time.Duration + // DialContext specifies the dial function for creating unencrypted TCP connections. + // If DialContext is nil, then the client dials using package net. + DialContext func(ctx context.Context, network, addr string) (net.Conn, error) + // TLS Config used for FTPS. If provided, it will be an error if the server // does not support TLS. Both the control and data connection will use TLS. TLSConfig *tls.Config @@ -178,12 +178,6 @@ type Client struct { // Construct and return a new client Conn, setting default config // values as necessary. func newClient(config Config, hosts []string) *Client { - if config.DialContext == nil { - config.DialContext = (&net.Dialer{ - Timeout: config.Timeout, - }).DialContext - } - if config.ConnectionsPerHost <= 0 { config.ConnectionsPerHost = 5 } @@ -192,6 +186,12 @@ func newClient(config Config, hosts []string) *Client { config.Timeout = 5 * time.Second } + if config.DialContext == nil { + config.DialContext = (&net.Dialer{ + Timeout: config.Timeout, + }).DialContext + } + if config.User == "" { config.User = "anonymous" } From 9ee536b5172523762d57d9ba7159d15c25463754 Mon Sep 17 00:00:00 2001 From: Randy Reddig Date: Wed, 11 Jul 2018 09:34:00 -0700 Subject: [PATCH 07/37] Address feedback: config.Timeout is always non-zero Also note where parent Context should be wired up in future. --- client.go | 12 ++++-------- persistent_connection.go | 9 +++------ 2 files changed, 7 insertions(+), 14 deletions(-) diff --git a/client.go b/client.go index 819e612..8779099 100644 --- a/client.go +++ b/client.go @@ -377,15 +377,11 @@ func (c *Client) openConn(idx int, host string) (pconn *persistentConn, err erro epsvNotSupported: c.config.DisableEPSV, } - var conn net.Conn - - ctx := context.Background() - if c.config.Timeout != 0 { - var cancel context.CancelFunc - ctx, cancel = context.WithTimeout(ctx, c.config.Timeout) - defer cancel() - } + // FIXME: this method should accept a parent Context + ctx, cancel := context.WithTimeout(context.Background(), c.config.Timeout) + defer cancel() + var conn net.Conn pconn.debug("opening control connection to %s", host) conn, err = c.config.DialContext(ctx, "tcp", host) if c.config.TLSConfig != nil && c.config.TLSMode == TLSImplicit { diff --git a/persistent_connection.go b/persistent_connection.go index 4b0ee35..12d7520 100644 --- a/persistent_connection.go +++ b/persistent_connection.go @@ -419,12 +419,9 @@ func (pconn *persistentConn) prepareDataConn() (func() (net.Conn, error), error) return nil, err } - ctx := context.Background() - if pconn.config.Timeout != 0 { - var cancel context.CancelFunc - ctx, cancel = context.WithTimeout(ctx, pconn.config.Timeout) - defer cancel() - } + // FIXME: this method should accept a parent Context + ctx, cancel := context.WithTimeout(context.Background(), pconn.config.Timeout) + defer cancel() pconn.debug("opening data connection to %s", host) dc, netErr := pconn.config.DialContext(ctx, "tcp", host) From ae0500ee26fb5cf68732605fc1a03d3208b39b79 Mon Sep 17 00:00:00 2001 From: Randy Reddig Date: Wed, 11 Jul 2018 09:38:14 -0700 Subject: [PATCH 08/37] Address feedback: restore 930d480 --- examples_test.go | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-) diff --git a/examples_test.go b/examples_test.go index 7130f8f..c389b26 100644 --- a/examples_test.go +++ b/examples_test.go @@ -2,7 +2,7 @@ // Use of this source code is governed by a MIT-style // license that can be found in the LICENSE file. -package goftp +package goftp_test import ( "bytes" @@ -10,11 +10,13 @@ import ( "io/ioutil" "os" "time" + + "github.com/secsy/goftp" ) func Example() { // Create client object with default config - client, err := Dial("ftp.example.com") + client, err := goftp.Dial("ftp.example.com") if err != nil { panic(err) } @@ -43,7 +45,7 @@ func Example() { } func Example_config() { - config := Config{ + config := goftp.Config{ User: "jlpicard", Password: "beverly123", ConnectionsPerHost: 10, @@ -51,7 +53,7 @@ func Example_config() { Logger: os.Stderr, } - client, err := DialConfig(config, "ftp.example.com") + client, err := goftp.DialConfig(config, "ftp.example.com") if err != nil { panic(err) } @@ -67,7 +69,7 @@ func Example_config() { func ExampleClient_OpenRawConn() { // ignore errors for brevity - client, _ := Dial("ftp.hq.nasa.gov") + client, _ := goftp.Dial("ftp.hq.nasa.gov") rawConn, _ := client.OpenRawConn() From 1855f7cd1f69e32273e75cf74e5504754ff7a069 Mon Sep 17 00:00:00 2001 From: Randy Reddig Date: Wed, 11 Jul 2018 09:41:47 -0700 Subject: [PATCH 09/37] Address feedback: override Config.User,Password in Transport if req.URL.User is non-nil --- transport.go | 10 ++++------ 1 file changed, 4 insertions(+), 6 deletions(-) diff --git a/transport.go b/transport.go index fa52ab4..dd82802 100644 --- a/transport.go +++ b/transport.go @@ -28,13 +28,11 @@ func (t *Transport) RoundTrip(req *http.Request) (*http.Response, error) { } } + // If req.URL.User is non-nil, username and password + // will override config even if empty. if req.URL.User != nil { - if user := req.URL.User.Username(); user != "" { - config.User = user - } - if password, ok := req.URL.User.Password(); ok { - config.Password = password - } + config.User = req.URL.User.Username() + config.Password, _ = req.URL.User.Password() } path := strings.TrimPrefix(req.URL.Path, "/") From 159f81169e8d2843310b61e3b84459feda9af2ee Mon Sep 17 00:00:00 2001 From: Randy Reddig Date: Wed, 11 Jul 2018 09:44:28 -0700 Subject: [PATCH 10/37] Address feedback: document when Config vs URL user/password are used --- transport.go | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/transport.go b/transport.go index dd82802..84e9dfe 100644 --- a/transport.go +++ b/transport.go @@ -10,12 +10,15 @@ import ( // Transport implements the http.RoundTripper interface. // Typical usage would be to register a Transport to handle // ftp:// and/or ftps:// URLs with http.Transport.RegisterProtocol. +// The User and Password fields in Config will be used when connecting +// to the remote FTP server unless the http.Request’s URL.User is non-nil. type Transport struct { Config } // RoundTrip implements the http.RoundTripper interface to allow an http.Client -// to handle ftp:// or ftps:// URLs. +// to handle ftp:// or ftps:// URLs. If req.URL.User is nil, the user and password +// from t.Config will be used instead. func (t *Transport) RoundTrip(req *http.Request) (*http.Response, error) { config := t.Config switch req.URL.Scheme { From ceeaabf61456c051ada4e24a03108a33bd566d42 Mon Sep 17 00:00:00 2001 From: Randy Reddig Date: Sat, 20 Mar 2021 10:26:19 -0700 Subject: [PATCH 11/37] go.mod: Go module support for Go 1.16+ --- go.mod | 3 +++ 1 file changed, 3 insertions(+) create mode 100644 go.mod diff --git a/go.mod b/go.mod new file mode 100644 index 0000000..6bf9320 --- /dev/null +++ b/go.mod @@ -0,0 +1,3 @@ +module github.com/secsy/goftp + +go 1.14 From bd0d320fef5ff5ffdc9204753182188e52bda21f Mon Sep 17 00:00:00 2001 From: Randy Reddig Date: Sat, 20 Mar 2021 10:28:15 -0700 Subject: [PATCH 12/37] build_test_server.sh: update to build on macOS Big Sur - Update proftpd to 1.3.7a and remove monkey patches - Update pure-ftpd to 1.0.49 - Parameterized version numbers - Check-before-download to speed up repeated builds --- build_test_server.sh | 32 +++++++++++++++----------------- 1 file changed, 15 insertions(+), 17 deletions(-) diff --git a/build_test_server.sh b/build_test_server.sh index 53c4f06..f7844f9 100755 --- a/build_test_server.sh +++ b/build_test_server.sh @@ -1,5 +1,10 @@ #!/bin/bash -ex +if [ "$(uname)" == "Darwin" ]; then + cflags=-I/usr/local/opt/openssl/include + ldflags=-L/usr/local/opt/openssl/lib +fi + goftp_dir=`pwd` mkdir -p ftpd @@ -7,20 +12,11 @@ cd ftpd ftpd_dir=`pwd` -curl -O ftp://ftp.proftpd.org/distrib/source/proftpd-1.3.5.tar.gz -tar -xzf proftpd-1.3.5.tar.gz -cd proftpd-1.3.5 - -# fix slow tls data connection handshake (https://github.com/proftpd/proftpd/pull/48) -perl -pi -e 's/(\Qpr_inet_set_proto_nodelay(conn->pool, conn, 1);\E)/$1\n(void) pr_inet_set_proto_cork(conn->wfd, 0);/' contrib/mod_tls.c +proftp_version='1.3.7a' -# fix a segfault on mac -perl -pi -e 's/\Qsstrncpy(cwd, dir, sizeof(cwd));\E/char dircpy[sizeof(cwd)];\nsstrncpy(dircpy, dir, sizeof(dircpy));\nsstrncpy(cwd, dircpy, sizeof(cwd));/' src/fsio.c - -if [ "$(uname)" == "Darwin" ]; then - cflags=-I/usr/local/opt/openssl/include - ldflags=-L/usr/local/opt/openssl/lib -fi +test -f proftpd-${proftp_version}.tar.gz || curl -O ftp://ftp.proftpd.org/distrib/source/proftpd-${proftp_version}.tar.gz +tar -xzf proftpd-${proftp_version}.tar.gz +cd proftpd-${proftp_version} CFLAGS=$cflags LDFLAGS=$ldflags ./configure --with-modules=mod_tls --disable-ident @@ -88,9 +84,11 @@ VpOorURz8ETlfAA= -----END CERTIFICATE----- CERT -curl -O https://download.pureftpd.org/pub/pure-ftpd/releases/obsolete/pure-ftpd-1.0.36.tar.gz -tar -xzf pure-ftpd-1.0.36.tar.gz -cd pure-ftpd-1.0.36 +pure_ftpd_version='1.0.49' + +test -f pure-ftpd-${pure_ftpd_version}.tar.gz || curl -O https://download.pureftpd.org/pub/pure-ftpd/releases/pure-ftpd-${pure_ftpd_version}.tar.gz +tar -xzf pure-ftpd-${pure_ftpd_version}.tar.gz +cd pure-ftpd-${pure_ftpd_version} # build normal binary with explicit tls support CFLAGS=$cflags LDFLAGS=$ldflags ./configure --with-nonroot --with-puredb --with-tls --with-certfile=$ftpd_dir/pure-ftpd.pem @@ -121,4 +119,4 @@ fi chmod 600 users.txt # generate puredb user db file -pure-ftpd-1.0.36/src/pure-pw mkdb users.pdb -f users.txt +pure-ftpd-${pure_ftpd_version}/src/pure-pw mkdb users.pdb -f users.txt From eea0e43447ca466c347fada383cd2632e0cdc5aa Mon Sep 17 00:00:00 2001 From: Randy Reddig Date: Sat, 20 Mar 2021 10:31:12 -0700 Subject: [PATCH 13/37] .github/workflows/go: add GitHub Actions CI --- .github/workflows/go.yaml | 32 ++++++++++++++++++++++++++++++++ 1 file changed, 32 insertions(+) create mode 100644 .github/workflows/go.yaml diff --git a/.github/workflows/go.yaml b/.github/workflows/go.yaml new file mode 100644 index 0000000..3561dc5 --- /dev/null +++ b/.github/workflows/go.yaml @@ -0,0 +1,32 @@ +name: Go + +on: + push: + branches: + - master + pull_request: + +jobs: + test: + name: Test + runs-on: ubuntu-latest + timeout-minutes: 5 + steps: + - name: Checkout repo + uses: actions/checkout@v2 + with: + submodules: recursive + + - name: Set up Go + uses: actions/setup-go@v2 + with: + go-version: ^1 + + - name: Build FTP test servers + run: ./build_test_server.sh + + - name: Test Go code + run: go test -v ./... + + - name: Test with race detector + run: go test -v -race ./... From 34495c4a72783665a2eeb9602263686b504f0efc Mon Sep 17 00:00:00 2001 From: Randy Reddig Date: Sat, 20 Mar 2021 10:36:48 -0700 Subject: [PATCH 14/37] .github/workflows/go: debugging step --- .github/workflows/go.yaml | 3 +++ 1 file changed, 3 insertions(+) diff --git a/.github/workflows/go.yaml b/.github/workflows/go.yaml index 3561dc5..33f7f1d 100644 --- a/.github/workflows/go.yaml +++ b/.github/workflows/go.yaml @@ -25,6 +25,9 @@ jobs: - name: Build FTP test servers run: ./build_test_server.sh + - name: Inspect ftpd dir + run: ls -al ftpd + - name: Test Go code run: go test -v ./... From 76b84a28509af2b71253d7cb01c45dc8779d9eea Mon Sep 17 00:00:00 2001 From: Randy Reddig Date: Sat, 20 Mar 2021 10:53:01 -0700 Subject: [PATCH 15/37] .github/workflows/go: disable IPv6 This is disabled in .travis.yml --- .github/workflows/go.yaml | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/.github/workflows/go.yaml b/.github/workflows/go.yaml index 33f7f1d..f81f3a6 100644 --- a/.github/workflows/go.yaml +++ b/.github/workflows/go.yaml @@ -22,12 +22,12 @@ jobs: with: go-version: ^1 + - name: Disable IPv6 + run: echo 0 | sudo tee /proc/sys/net/ipv6/conf/all/disable_ipv6 + - name: Build FTP test servers run: ./build_test_server.sh - - name: Inspect ftpd dir - run: ls -al ftpd - - name: Test Go code run: go test -v ./... From 9bbc2bd809c76d30ab9b10d5cd39ce14d1a80a28 Mon Sep 17 00:00:00 2001 From: Randy Reddig Date: Sat, 20 Mar 2021 10:56:16 -0700 Subject: [PATCH 16/37] =?UTF-8?q?.github/workflows/go:=20let=E2=80=99s=20t?= =?UTF-8?q?ry=20this=20another=20way?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- .github/workflows/go.yaml | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/.github/workflows/go.yaml b/.github/workflows/go.yaml index f81f3a6..bae47f2 100644 --- a/.github/workflows/go.yaml +++ b/.github/workflows/go.yaml @@ -23,7 +23,11 @@ jobs: go-version: ^1 - name: Disable IPv6 - run: echo 0 | sudo tee /proc/sys/net/ipv6/conf/all/disable_ipv6 + run: | + sudo sysctl -w net.ipv6.conf.all.disable_ipv6=1 + sudo sysctl -w net.ipv6.conf.default.disable_ipv6=1 + sudo sysctl -w net.ipv6.conf.lo.disable_ipv6=1 + ip a - name: Build FTP test servers run: ./build_test_server.sh From 2cdbbdf54abf87b91541f4f26dbc6678d226ba37 Mon Sep 17 00:00:00 2001 From: Randy Reddig Date: Sat, 20 Mar 2021 11:04:04 -0700 Subject: [PATCH 17/37] =?UTF-8?q?.github/workflows/go:=20DON=E2=80=99T=20d?= =?UTF-8?q?isable=20IPv6?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- .github/workflows/go.yaml | 7 ------- 1 file changed, 7 deletions(-) diff --git a/.github/workflows/go.yaml b/.github/workflows/go.yaml index bae47f2..3561dc5 100644 --- a/.github/workflows/go.yaml +++ b/.github/workflows/go.yaml @@ -22,13 +22,6 @@ jobs: with: go-version: ^1 - - name: Disable IPv6 - run: | - sudo sysctl -w net.ipv6.conf.all.disable_ipv6=1 - sudo sysctl -w net.ipv6.conf.default.disable_ipv6=1 - sudo sysctl -w net.ipv6.conf.lo.disable_ipv6=1 - ip a - - name: Build FTP test servers run: ./build_test_server.sh From 0dd5fc3f8a9fa10161ebcab4d52c7d84faaeb5fe Mon Sep 17 00:00:00 2001 From: Randy Reddig Date: Sat, 20 Mar 2021 11:08:24 -0700 Subject: [PATCH 18/37] Use different port for IPv4 and IPv6 listeners --- main_test.go | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/main_test.go b/main_test.go index 22a1254..d3b2790 100644 --- a/main_test.go +++ b/main_test.go @@ -26,9 +26,9 @@ var ftpdAddrs []string var ( // used for implicit tls test - implicitTLSAddrs = []string{"127.0.0.1:2122", "[::1]:2122"} - pureAddrs = []string{"127.0.0.1:2121", "[::1]:2121"} - proAddrs = []string{"127.0.0.1:2124"} + implicitTLSAddrs = []string{"127.0.0.1:2123", "[::1]:2124"} + pureAddrs = []string{"127.0.0.1:2121", "[::1]:2122"} + proAddrs = []string{"127.0.0.1:2124"} // Must match value in proftpd.conf ) func TestMain(m *testing.M) { From 53f68b3a7df75296a6948d958adf36901fe52c3d Mon Sep 17 00:00:00 2001 From: Randy Reddig Date: Sat, 20 Mar 2021 11:15:07 -0700 Subject: [PATCH 19/37] Attempt to test without IPv6 --- main_test.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/main_test.go b/main_test.go index d3b2790..5356e74 100644 --- a/main_test.go +++ b/main_test.go @@ -26,8 +26,8 @@ var ftpdAddrs []string var ( // used for implicit tls test - implicitTLSAddrs = []string{"127.0.0.1:2123", "[::1]:2124"} - pureAddrs = []string{"127.0.0.1:2121", "[::1]:2122"} + implicitTLSAddrs = []string{"127.0.0.1:2123"} + pureAddrs = []string{"127.0.0.1:2121"} proAddrs = []string{"127.0.0.1:2124"} // Must match value in proftpd.conf ) From 1cae9393a7902c4826c0daa99f8e86821168212e Mon Sep 17 00:00:00 2001 From: Randy Reddig Date: Sat, 20 Mar 2021 11:20:53 -0700 Subject: [PATCH 20/37] Revert 0dd5fc3 and 53f68b3 --- main_test.go | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/main_test.go b/main_test.go index 5356e74..22a1254 100644 --- a/main_test.go +++ b/main_test.go @@ -26,9 +26,9 @@ var ftpdAddrs []string var ( // used for implicit tls test - implicitTLSAddrs = []string{"127.0.0.1:2123"} - pureAddrs = []string{"127.0.0.1:2121"} - proAddrs = []string{"127.0.0.1:2124"} // Must match value in proftpd.conf + implicitTLSAddrs = []string{"127.0.0.1:2122", "[::1]:2122"} + pureAddrs = []string{"127.0.0.1:2121", "[::1]:2121"} + proAddrs = []string{"127.0.0.1:2124"} ) func TestMain(m *testing.M) { From 53dec5668a1f194824e8eb84f678059a54d182a8 Mon Sep 17 00:00:00 2001 From: Randy Reddig Date: Sat, 20 Mar 2021 11:33:40 -0700 Subject: [PATCH 21/37] enable logs in testing --- main_test.go | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/main_test.go b/main_test.go index 22a1254..9b2a7b1 100644 --- a/main_test.go +++ b/main_test.go @@ -86,6 +86,8 @@ func startPureFTPD(addrs []string, binary string) (func(), error) { cmd.Env = []string{fmt.Sprintf("FTP_ANON_DIR=%s/testroot", cwd)} + cmd.Stderr = os.Stderr + err = cmd.Start() if err != nil { return nil, fmt.Errorf("error starting pure-ftpd on %s: %s", addr, err) @@ -129,8 +131,7 @@ func startProFTPD() (func(), error) { // "--debug", "10", ) - // cmd.Stdout = os.Stdout - // cmd.Stderr = os.Stderr + cmd.Stderr = os.Stderr err = cmd.Start() if err != nil { From 67f27561c9136d4f63a9dcb7fe0f95a8172ed278 Mon Sep 17 00:00:00 2001 From: Randy Reddig Date: Sat, 20 Mar 2021 11:54:10 -0700 Subject: [PATCH 22/37] Generate server key and cert on the fly --- build_test_server.sh | 41 +++-------------------------------------- 1 file changed, 3 insertions(+), 38 deletions(-) diff --git a/build_test_server.sh b/build_test_server.sh index f7844f9..05e0376 100755 --- a/build_test_server.sh +++ b/build_test_server.sh @@ -47,42 +47,9 @@ TLSOptions NoSessionReuseRequired CONF -cat > server.key <<"KEY" ------BEGIN RSA PRIVATE KEY----- -MIICWwIBAAKBgQDOMHBkxoVgenJqLimeIkztEE9Hp3XcIE7cmZILqkMDuo0kGAVU -Mvldyo+sYqop46aPbobiqPxU3knyrHJJ2H0ucFnb67kUH5ITncYo8iNephtgtuMR -D0JKYneaGtJ0Z+kTWIJV3/9f2GFLK8InY7ipoxZX3hGkSeIVyh6F66CZ5QIDAQAB -AoGAfuC/yMOAf4XZsg0F/xEMVTScFHOvyuz2mjjF7fevlTPOdk9xuAZF/LkQ//sW -ywATFl/lEMT7wR2oU3RaP6bAICCf1vGba51U+yFTS/8T1+VJvRuojMX4RVJhaFU4 -HJx9Fd9a8kLzHTaBkaVtGJzK3GxXpa7mwSlJ+Eeeh+CYYAECQQDq4Mb1tgcEOWFB -v9j9StMby+0FF8uitx5+E78r3xPRRjIjgray6UuFCc41U/pPuw2iqreDCkjLKb5G -SIqQ7BrlAkEA4Ls0cAejrONvO/1+NIeNQn30WIgH7zqBmRYJnLqw8xo0xML51pFU -gXYCEp+M2AtDL6yQ0zSN7D2JhtbzLweTAQJAf7rteAIdnrZ1pYPnRRfD5oHny7U9 -EKf09StX80vFQzGhYp5bLMCiSR8j/OxGW8WljKi6U5DsNU/mIeKhOF6t4QJAfUAZ -E69OS9decYLwyfoagsqMWqNGONDU1itwJAfxAyzB6D/62tmYzaaltRdzeh2czn9R -IEWUK+yIL7yxQK7qAQJABD3UYT2yZaZJDNerK9h9RJRptN1f0vJ29qURkj7OpFbY -2hgLZ/lfrSE5RCSmYQtmk2hyuSzMSe928k85Y14+wg== ------END RSA PRIVATE KEY----- -KEY - -cat > server.cert <<"CERT" ------BEGIN CERTIFICATE----- -MIICdzCCAeCgAwIBAgIJAJls3dJsiITyMA0GCSqGSIb3DQEBBQUAMDIxCzAJBgNV -BAYTAlVTMRMwEQYDVQQIEwpDYWxpZm9ybmlhMQ4wDAYDVQQKEwVHb0ZUUDAeFw0x -NTAyMTYwNTQ0MDhaFw0xNTAzMTgwNTQ0MDhaMDIxCzAJBgNVBAYTAlVTMRMwEQYD -VQQIEwpDYWxpZm9ybmlhMQ4wDAYDVQQKEwVHb0ZUUDCBnzANBgkqhkiG9w0BAQEF -AAOBjQAwgYkCgYEAzjBwZMaFYHpyai4pniJM7RBPR6d13CBO3JmSC6pDA7qNJBgF -VDL5XcqPrGKqKeOmj26G4qj8VN5J8qxySdh9LnBZ2+u5FB+SE53GKPIjXqYbYLbj -EQ9CSmJ3mhrSdGfpE1iCVd//X9hhSyvCJ2O4qaMWV94RpEniFcoeheugmeUCAwEA -AaOBlDCBkTAdBgNVHQ4EFgQUb/FNa79J/POe13rCUSA3eSJviGgwYgYDVR0jBFsw -WYAUb/FNa79J/POe13rCUSA3eSJviGihNqQ0MDIxCzAJBgNVBAYTAlVTMRMwEQYD -VQQIEwpDYWxpZm9ybmlhMQ4wDAYDVQQKEwVHb0ZUUIIJAJls3dJsiITyMAwGA1Ud -EwQFMAMBAf8wDQYJKoZIhvcNAQEFBQADgYEAhUH0UnU46s2XbAGq6RpmKuONjgJX -X4qKrpmBhSg6KS4WkgnLr8+YrvvcFhhPGf9xLpCS1o+RC0W6BuwqiAtM+ckqDnI5 -pb3vMhAhXTjg1bLWDNFn98iI5tSqGSjy9d7RfdC2yyFQsliq2b74yHxCOysC5OW0 -VpOorURz8ETlfAA= ------END CERTIFICATE----- -CERT +# generate a key and certificate +openssl req -x509 -nodes -newkey rsa:4096 -keyout server.key -out server.cert -days 365 -subj '/C=US/ST=California/O=GoFTP' +cat server.key server.cert > pure-ftpd.pem pure_ftpd_version='1.0.49' @@ -104,8 +71,6 @@ mv src/pure-ftpd ../pure-ftpd-implicittls cd .. -cat server.key server.cert > pure-ftpd.pem - # setup up a goftp user for ftp server if [ "$(uname)" == "Darwin" ]; then echo "goftp:_.../HVM0l1lcNKVtiKs:`id -u`:`id -g`::$goftp_dir/testroot/./::::::::::::" > users.txt From 53390d783c6687c89d69868a67a862b057031cd5 Mon Sep 17 00:00:00 2001 From: Randy Reddig Date: Sat, 20 Mar 2021 11:57:25 -0700 Subject: [PATCH 23/37] Remove Travis CI --- .travis.yml | 11 ----------- 1 file changed, 11 deletions(-) delete mode 100644 .travis.yml diff --git a/.travis.yml b/.travis.yml deleted file mode 100644 index 21fef4e..0000000 --- a/.travis.yml +++ /dev/null @@ -1,11 +0,0 @@ -language: go - -go: - - "1.10" - - tip - -install: - - ./build_test_server.sh - -before_script: - - echo 0 | sudo tee /proc/sys/net/ipv6/conf/all/disable_ipv6 From 531cdf9db6b95bb6792285cc954adcd7b0a65ad3 Mon Sep 17 00:00:00 2001 From: Randy Reddig Date: Sat, 20 Mar 2021 12:05:13 -0700 Subject: [PATCH 24/37] revert 170427b --- parallel_walk_test.go | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/parallel_walk_test.go b/parallel_walk_test.go index d68932d..e344e2b 100644 --- a/parallel_walk_test.go +++ b/parallel_walk_test.go @@ -2,7 +2,7 @@ // Use of this source code is governed by a MIT-style // license that can be found in the LICENSE file. -package goftp +package goftp_test import ( "fmt" @@ -10,12 +10,14 @@ import ( "path" "path/filepath" "sync/atomic" + + "github.com/secsy/goftp" ) // Just for fun, walk an ftp server in parallel. I make no claim that this is // correct or a good idea. func ExampleClient_ReadDir_parallelWalk() { - client, err := Dial("ftp.hq.nasa.gov") + client, err := goftp.Dial("ftp.hq.nasa.gov") if err != nil { panic(err) } @@ -23,7 +25,7 @@ func ExampleClient_ReadDir_parallelWalk() { Walk(client, "", func(fullPath string, info os.FileInfo, err error) error { if err != nil { // no permissions is okay, keep walking - if err.(Error).Code() == 550 { + if err.(goftp.Error).Code() == 550 { return nil } return err @@ -37,7 +39,7 @@ func ExampleClient_ReadDir_parallelWalk() { // Walk a FTP file tree in parallel with prunability and error handling. // See http://golang.org/pkg/path/filepath/#Walk for interface details. -func Walk(client *Client, root string, walkFn filepath.WalkFunc) (ret error) { +func Walk(client *goftp.Client, root string, walkFn filepath.WalkFunc) (ret error) { dirsToCheck := make(chan string, 100) var workCount int32 = 1 From ec080cc2018d960ee7d78cf58d8bbc07cf9a542d Mon Sep 17 00:00:00 2001 From: Randy Reddig Date: Sat, 20 Mar 2021 12:11:23 -0700 Subject: [PATCH 25/37] check err before creating tls.Client --- client.go | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/client.go b/client.go index 8779099..5edaf2b 100644 --- a/client.go +++ b/client.go @@ -384,10 +384,6 @@ func (c *Client) openConn(idx int, host string) (pconn *persistentConn, err erro var conn net.Conn pconn.debug("opening control connection to %s", host) conn, err = c.config.DialContext(ctx, "tcp", host) - if c.config.TLSConfig != nil && c.config.TLSMode == TLSImplicit { - pconn.debug("configuring implicit TLS control connection to %s", host) - conn = tls.Client(conn, pconn.config.TLSConfig) - } var ( code int @@ -406,6 +402,11 @@ func (c *Client) openConn(idx int, host string) (pconn *persistentConn, err erro goto Error } + if c.config.TLSConfig != nil && c.config.TLSMode == TLSImplicit { + pconn.debug("configuring implicit TLS control connection to %s", host) + conn = tls.Client(conn, pconn.config.TLSConfig) + } + pconn.setControlConn(conn) code, msg, err = pconn.readResponse() From 08de34cce525e901e07c75c61b4e6e9504688d60 Mon Sep 17 00:00:00 2001 From: Randy Reddig Date: Sat, 20 Mar 2021 12:24:16 -0700 Subject: [PATCH 26/37] Remove Transport type, just add RoundTrip method to Config --- transport.go => round_tripper.go | 16 +++++---------- transport_test.go => round_tripper_test.go | 23 ++++++++++------------ 2 files changed, 15 insertions(+), 24 deletions(-) rename transport.go => round_tripper.go (83%) rename transport_test.go => round_tripper_test.go (79%) diff --git a/transport.go b/round_tripper.go similarity index 83% rename from transport.go rename to round_tripper.go index 84e9dfe..1d1397e 100644 --- a/transport.go +++ b/round_tripper.go @@ -7,20 +7,14 @@ import ( "strings" ) -// Transport implements the http.RoundTripper interface. -// Typical usage would be to register a Transport to handle +// RoundTrip implements the http.RoundTripper interface to allow an http.Client +// to handle ftp:// or ftps:// URLs. If req.URL.User is nil, the user and password +// from config will be used instead. +// Typical usage would be to register a Config to handle // ftp:// and/or ftps:// URLs with http.Transport.RegisterProtocol. // The User and Password fields in Config will be used when connecting // to the remote FTP server unless the http.Request’s URL.User is non-nil. -type Transport struct { - Config -} - -// RoundTrip implements the http.RoundTripper interface to allow an http.Client -// to handle ftp:// or ftps:// URLs. If req.URL.User is nil, the user and password -// from t.Config will be used instead. -func (t *Transport) RoundTrip(req *http.Request) (*http.Response, error) { - config := t.Config +func (config Config) RoundTrip(req *http.Request) (*http.Response, error) { switch req.URL.Scheme { default: return nil, http.ErrSkipAltProtocol diff --git a/transport_test.go b/round_tripper_test.go similarity index 79% rename from transport_test.go rename to round_tripper_test.go index 0a4b7ad..5962196 100644 --- a/transport_test.go +++ b/round_tripper_test.go @@ -10,23 +10,22 @@ import ( "time" ) -func TestTransportSkipAltProtocol(t *testing.T) { - transport := Transport{} +func TestRoundTripperSkipAltProtocol(t *testing.T) { + config := Config{} req, err := http.NewRequest(http.MethodGet, "foo://localhost/foo.txt", nil) if err != nil { t.Fatal(err) } - _, err = transport.RoundTrip(req) + _, err = config.RoundTrip(req) if err != http.ErrSkipAltProtocol { t.Errorf("Expected err = %v, got %v", http.ErrSkipAltProtocol, err) } } -func TestTransportTimeoutConnect(t *testing.T) { +func TestRoundTripperTimeoutConnect(t *testing.T) { config := Config{Timeout: 100 * time.Millisecond} - transport := Transport{Config: config} req, err := http.NewRequest(http.MethodGet, "ftp://168.254.111.222:2121/subdir/1234.bin", nil) if err != nil { @@ -34,8 +33,8 @@ func TestTransportTimeoutConnect(t *testing.T) { } t0 := time.Now() - res, err := transport.RoundTrip(req) - // Transport.RoundTrip calls Client.Retrieve in a goroutine + res, err := config.RoundTrip(req) + // Config.RoundTrip calls Client.Retrieve in a goroutine // so large file reads are unbuffered. _, err = ioutil.ReadAll(res.Body) res.Body.Close() @@ -53,7 +52,7 @@ func TestTransportTimeoutConnect(t *testing.T) { } } -func TestTransportExplicitTLS(t *testing.T) { +func TestRoundTripperExplicitTLS(t *testing.T) { for _, addr := range ftpdAddrs { config := Config{ TLSConfig: &tls.Config{ @@ -61,7 +60,6 @@ func TestTransportExplicitTLS(t *testing.T) { }, TLSMode: TLSExplicit, } - transport := Transport{Config: config} req, err := http.NewRequest(http.MethodGet, "ftp://"+addr+"/subdir/1234.bin", nil) if err != nil { @@ -70,7 +68,7 @@ func TestTransportExplicitTLS(t *testing.T) { req.URL.User = url.UserPassword("goftp", "rocks") - res, err := transport.RoundTrip(req) + res, err := config.RoundTrip(req) if err != nil { t.Fatal(err) } @@ -87,7 +85,7 @@ func TestTransportExplicitTLS(t *testing.T) { } } -func TestTransportImplicitTLS(t *testing.T) { +func TestRoundTripperImplicitTLS(t *testing.T) { closer, err := startPureFTPD(implicitTLSAddrs, "ftpd/pure-ftpd-implicittls") if err != nil { t.Fatal(err) @@ -102,7 +100,6 @@ func TestTransportImplicitTLS(t *testing.T) { }, TLSMode: TLSImplicit, } - transport := Transport{Config: config} req, err := http.NewRequest(http.MethodGet, "ftp://"+addr+"/subdir/1234.bin", nil) if err != nil { @@ -111,7 +108,7 @@ func TestTransportImplicitTLS(t *testing.T) { req.URL.User = url.UserPassword("goftp", "rocks") - res, err := transport.RoundTrip(req) + res, err := config.RoundTrip(req) if err != nil { t.Fatal(err) } From 4ad1a3c7ffdaa61b115fa20ea0b90a6fa8a7705a Mon Sep 17 00:00:00 2001 From: Randy Reddig Date: Sat, 20 Mar 2021 12:26:25 -0700 Subject: [PATCH 27/37] use time.Since --- client.go | 2 +- client_test.go | 2 +- persistent_connection.go | 2 +- round_tripper_test.go | 2 +- 4 files changed, 4 insertions(+), 4 deletions(-) diff --git a/client.go b/client.go index 5edaf2b..53c683b 100644 --- a/client.go +++ b/client.go @@ -250,7 +250,7 @@ func (c *Client) debug(f string, args ...interface{}) { } fmt.Fprintf(c.config.Logger, "goftp: %.3f %s\n", - time.Now().Sub(c.t0).Seconds(), + time.Since(c.t0).Seconds(), fmt.Sprintf(f, args...), ) } diff --git a/client_test.go b/client_test.go index 710c7c7..3e758b1 100644 --- a/client_test.go +++ b/client_test.go @@ -21,7 +21,7 @@ func TestTimeoutConnect(t *testing.T) { t0 := time.Now() _, err = c.ReadDir("") - delta := time.Now().Sub(t0) + delta := time.Since(t0) if err == nil || !err.(Error).Temporary() { t.Error("Expected a timeout error") diff --git a/persistent_connection.go b/persistent_connection.go index 12d7520..7d2be65 100644 --- a/persistent_connection.go +++ b/persistent_connection.go @@ -188,7 +188,7 @@ func (pconn *persistentConn) debug(f string, args ...interface{}) { } fmt.Fprintf(pconn.config.Logger, "goftp: %.3f #%d %s\n", - time.Now().Sub(pconn.t0).Seconds(), + time.Since(pconn.t0).Seconds(), pconn.idx, fmt.Sprintf(f, args...), ) diff --git a/round_tripper_test.go b/round_tripper_test.go index 5962196..df02dc7 100644 --- a/round_tripper_test.go +++ b/round_tripper_test.go @@ -38,7 +38,7 @@ func TestRoundTripperTimeoutConnect(t *testing.T) { // so large file reads are unbuffered. _, err = ioutil.ReadAll(res.Body) res.Body.Close() - delta := time.Now().Sub(t0) + delta := time.Since(t0) if err == nil || !err.(Error).Temporary() { t.Error("Expected a timeout error") } From 51b9ef2a4fac774227be38ae0a0009053fb484d6 Mon Sep 17 00:00:00 2001 From: Randy Reddig Date: Sat, 20 Mar 2021 12:30:03 -0700 Subject: [PATCH 28/37] Fix Go static check warnings --- file_system.go | 2 +- round_tripper_test.go | 2 +- transfer.go | 3 +-- 3 files changed, 3 insertions(+), 4 deletions(-) diff --git a/file_system.go b/file_system.go index d80bf40..6661c36 100644 --- a/file_system.go +++ b/file_system.go @@ -212,7 +212,7 @@ func (c *Client) controlStringList(f string, args ...interface{}) ([]string, err cmd := fmt.Sprintf(f, args...) - code, msg, err := pconn.sendCommand(cmd) + code, msg, _ := pconn.sendCommand(cmd) if !positiveCompletionReply(code) { pconn.debug("unexpected response to %s: %d-%s", cmd, code, msg) diff --git a/round_tripper_test.go b/round_tripper_test.go index df02dc7..321dcbf 100644 --- a/round_tripper_test.go +++ b/round_tripper_test.go @@ -33,7 +33,7 @@ func TestRoundTripperTimeoutConnect(t *testing.T) { } t0 := time.Now() - res, err := config.RoundTrip(req) + res, _ := config.RoundTrip(req) // Config.RoundTrip calls Client.Retrieve in a goroutine // so large file reads are unbuffered. _, err = ioutil.ReadAll(res.Body) diff --git a/transfer.go b/transfer.go index b3ff8d3..032e990 100644 --- a/transfer.go +++ b/transfer.go @@ -7,7 +7,6 @@ package goftp import ( "fmt" "io" - "os" "strconv" ) @@ -90,7 +89,7 @@ func (c *Client) Store(path string, src io.Reader) error { } } - _, seekErr := seeker.Seek(size, os.SEEK_SET) + _, seekErr := seeker.Seek(size, io.SeekStart) if seekErr != nil { c.debug("failed seeking to %d while resuming upload to %s: %s", size, From b98cc350fc2d101e469001328b7bf868c4b9a224 Mon Sep 17 00:00:00 2001 From: Randy Reddig Date: Sat, 20 Mar 2021 12:36:16 -0700 Subject: [PATCH 29/37] =?UTF-8?q?.github/workflows/go:=20test=20against=20?= =?UTF-8?q?Go=201.12=E2=80=931.16?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- .github/workflows/go.yaml | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/.github/workflows/go.yaml b/.github/workflows/go.yaml index 3561dc5..4b20ba2 100644 --- a/.github/workflows/go.yaml +++ b/.github/workflows/go.yaml @@ -9,6 +9,14 @@ on: jobs: test: name: Test + strategy: + matrix: + go_version: + - ^1.12 + - ^1.13 + - ^1.14 + - ^1.15 + - ^1.16 runs-on: ubuntu-latest timeout-minutes: 5 steps: @@ -20,7 +28,7 @@ jobs: - name: Set up Go uses: actions/setup-go@v2 with: - go-version: ^1 + go-version: ${{ matrix.go_version }} - name: Build FTP test servers run: ./build_test_server.sh From 94163bf3305ac8da2e8250a967446f1970662091 Mon Sep 17 00:00:00 2001 From: Randy Reddig Date: Sat, 20 Mar 2021 12:40:47 -0700 Subject: [PATCH 30/37] .github/workflows/go: no ^ prefix in Go version matrix --- .github/workflows/go.yaml | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/.github/workflows/go.yaml b/.github/workflows/go.yaml index 4b20ba2..4b3a765 100644 --- a/.github/workflows/go.yaml +++ b/.github/workflows/go.yaml @@ -12,11 +12,11 @@ jobs: strategy: matrix: go_version: - - ^1.12 - - ^1.13 - - ^1.14 - - ^1.15 - - ^1.16 + - 1.12 + - 1.13 + - 1.14 + - 1.15 + - 1.16 runs-on: ubuntu-latest timeout-minutes: 5 steps: From 8af54cf9db1b5a04d51a96c795f48b6788eddb62 Mon Sep 17 00:00:00 2001 From: Randy Reddig Date: Sat, 20 Mar 2021 12:44:25 -0700 Subject: [PATCH 31/37] .github/workflows/go: increase timeout to 15m --- .github/workflows/go.yaml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/go.yaml b/.github/workflows/go.yaml index 4b3a765..4048cbf 100644 --- a/.github/workflows/go.yaml +++ b/.github/workflows/go.yaml @@ -18,7 +18,7 @@ jobs: - 1.15 - 1.16 runs-on: ubuntu-latest - timeout-minutes: 5 + timeout-minutes: 15 steps: - name: Checkout repo uses: actions/checkout@v2 From f7cdb1bb833f5f5a1de3810c630bd6c85142f602 Mon Sep 17 00:00:00 2001 From: Randy Reddig Date: Sat, 20 Mar 2021 12:48:04 -0700 Subject: [PATCH 32/37] use different port for http.RoundTripper tests with implicit TLS --- main_test.go | 1 + round_tripper_test.go | 4 ++-- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/main_test.go b/main_test.go index 9b2a7b1..4ddf441 100644 --- a/main_test.go +++ b/main_test.go @@ -27,6 +27,7 @@ var ftpdAddrs []string var ( // used for implicit tls test implicitTLSAddrs = []string{"127.0.0.1:2122", "[::1]:2122"} + implicitRTAddrs = []string{"127.0.0.1:2123", "[::1]:2123"} pureAddrs = []string{"127.0.0.1:2121", "[::1]:2121"} proAddrs = []string{"127.0.0.1:2124"} ) diff --git a/round_tripper_test.go b/round_tripper_test.go index 321dcbf..51256d7 100644 --- a/round_tripper_test.go +++ b/round_tripper_test.go @@ -86,14 +86,14 @@ func TestRoundTripperExplicitTLS(t *testing.T) { } func TestRoundTripperImplicitTLS(t *testing.T) { - closer, err := startPureFTPD(implicitTLSAddrs, "ftpd/pure-ftpd-implicittls") + closer, err := startPureFTPD(implicitRTAddrs, "ftpd/pure-ftpd-implicittls") if err != nil { t.Fatal(err) } defer closer() - for _, addr := range implicitTLSAddrs { + for _, addr := range implicitRTAddrs { config := Config{ TLSConfig: &tls.Config{ InsecureSkipVerify: true, From c531c0bb377184c0858288dfc4442bf946343b8e Mon Sep 17 00:00:00 2001 From: Randy Reddig Date: Sat, 20 Mar 2021 12:54:07 -0700 Subject: [PATCH 33/37] Just run the implicit TLS server in TestMain --- client_test.go | 7 ------- main_test.go | 8 +++++++- round_tripper_test.go | 9 +-------- 3 files changed, 8 insertions(+), 16 deletions(-) diff --git a/client_test.go b/client_test.go index 3e758b1..d9aa380 100644 --- a/client_test.go +++ b/client_test.go @@ -84,13 +84,6 @@ func TestExplicitTLS(t *testing.T) { } func TestImplicitTLS(t *testing.T) { - closer, err := startPureFTPD(implicitTLSAddrs, "ftpd/pure-ftpd-implicittls") - if err != nil { - t.Fatal(err) - } - - defer closer() - for _, addr := range implicitTLSAddrs { config := Config{ TLSConfig: &tls.Config{ diff --git a/main_test.go b/main_test.go index 4ddf441..ba4ac27 100644 --- a/main_test.go +++ b/main_test.go @@ -27,12 +27,17 @@ var ftpdAddrs []string var ( // used for implicit tls test implicitTLSAddrs = []string{"127.0.0.1:2122", "[::1]:2122"} - implicitRTAddrs = []string{"127.0.0.1:2123", "[::1]:2123"} pureAddrs = []string{"127.0.0.1:2121", "[::1]:2121"} proAddrs = []string{"127.0.0.1:2124"} ) func TestMain(m *testing.M) { + implicitCloser, err := startPureFTPD(implicitTLSAddrs, "ftpd/pure-ftpd-implicittls") + + if err != nil { + log.Fatal(err) + } + pureCloser, err := startPureFTPD(pureAddrs, "ftpd/pure-ftpd") ftpdAddrs = append(ftpdAddrs, pureAddrs...) @@ -50,6 +55,7 @@ func TestMain(m *testing.M) { var ret int func() { + defer implicitCloser() defer pureCloser() defer proCloser() ret = m.Run() diff --git a/round_tripper_test.go b/round_tripper_test.go index 51256d7..ad0a089 100644 --- a/round_tripper_test.go +++ b/round_tripper_test.go @@ -86,14 +86,7 @@ func TestRoundTripperExplicitTLS(t *testing.T) { } func TestRoundTripperImplicitTLS(t *testing.T) { - closer, err := startPureFTPD(implicitRTAddrs, "ftpd/pure-ftpd-implicittls") - if err != nil { - t.Fatal(err) - } - - defer closer() - - for _, addr := range implicitRTAddrs { + for _, addr := range implicitTLSAddrs { config := Config{ TLSConfig: &tls.Config{ InsecureSkipVerify: true, From 4532bb1032263da91b2368eb2893a236dc34bb47 Mon Sep 17 00:00:00 2001 From: Randy Reddig Date: Wed, 24 Mar 2021 14:53:58 -0700 Subject: [PATCH 34/37] Simulate HTTP semantics (200 OK) in response --- round_tripper.go | 3 +++ round_tripper_test.go | 14 ++++++++++++++ 2 files changed, 17 insertions(+) diff --git a/round_tripper.go b/round_tripper.go index 1d1397e..53e0a18 100644 --- a/round_tripper.go +++ b/round_tripper.go @@ -44,6 +44,9 @@ func (config Config) RoundTrip(req *http.Request) (*http.Response, error) { default: return nil, http.ErrNotSupported case http.MethodGet: + // Simulate HTTP response semantics + res.StatusCode = 200 + res.Status = http.StatusText(res.StatusCode) // Pipe Client.Retrieve to res.Body so enable unbuffered reads // of large files. // Errors returned by Client.Retrieve (like the size check) diff --git a/round_tripper_test.go b/round_tripper_test.go index ad0a089..636fbdc 100644 --- a/round_tripper_test.go +++ b/round_tripper_test.go @@ -73,6 +73,13 @@ func TestRoundTripperExplicitTLS(t *testing.T) { t.Fatal(err) } + if want, got := http.StatusOK, res.StatusCode; want != got { + t.Errorf("res.StatusCode: want: %v got: %v", want, got) + } + if want, got := http.StatusText(http.StatusOK), res.Status; want != got { + t.Errorf("res.Status: want: %v got: %v", want, got) + } + b, err := ioutil.ReadAll(res.Body) res.Body.Close() if err != nil { @@ -106,6 +113,13 @@ func TestRoundTripperImplicitTLS(t *testing.T) { t.Fatal(err) } + if want, got := http.StatusOK, res.StatusCode; want != got { + t.Errorf("res.StatusCode: want: %v got: %v", want, got) + } + if want, got := http.StatusText(http.StatusOK), res.Status; want != got { + t.Errorf("res.Status: want: %v got: %v", want, got) + } + b, err := ioutil.ReadAll(res.Body) res.Body.Close() if err != nil { From 00d39f2e174e88df41b8330a66295f1264488a97 Mon Sep 17 00:00:00 2001 From: Randy Reddig Date: Wed, 24 Mar 2021 15:28:51 -0700 Subject: [PATCH 35/37] Use bufio.Reader to peek at first byte of downloaded file to synchronously get error and set res.Status --- round_tripper.go | 26 ++++++++++++++++++++++---- round_tripper_test.go | 28 ++++++++++++++++++++++++++++ 2 files changed, 50 insertions(+), 4 deletions(-) diff --git a/round_tripper.go b/round_tripper.go index 53e0a18..0a566b3 100644 --- a/round_tripper.go +++ b/round_tripper.go @@ -1,6 +1,7 @@ package goftp import ( + "bufio" "crypto/tls" "io" "net/http" @@ -44,18 +45,35 @@ func (config Config) RoundTrip(req *http.Request) (*http.Response, error) { default: return nil, http.ErrNotSupported case http.MethodGet: - // Simulate HTTP response semantics - res.StatusCode = 200 - res.Status = http.StatusText(res.StatusCode) // Pipe Client.Retrieve to res.Body so enable unbuffered reads // of large files. // Errors returned by Client.Retrieve (like the size check) // will be returned by res.Body.Read(). r, w := io.Pipe() - res.Body = r + brc := &bufferedReadCloser{bufio.NewReader(r), r} + res.Body = brc go func() { w.CloseWithError(client.Retrieve(path, w)) }() + _, err = brc.Peek(1) // Get error immediately on bad read + + // Simulate HTTP response semantics + if err, ok := err.(ftpError); ok { + res.StatusCode = err.Code() + res.Status = err.Message() + } else { + res.StatusCode = 200 + res.Status = http.StatusText(res.StatusCode) + } } return res, err } + +type bufferedReadCloser struct { + *bufio.Reader + rc io.ReadCloser +} + +func (rc *bufferedReadCloser) Close() error { + return rc.rc.Close() +} diff --git a/round_tripper_test.go b/round_tripper_test.go index 636fbdc..33a1476 100644 --- a/round_tripper_test.go +++ b/round_tripper_test.go @@ -89,6 +89,20 @@ func TestRoundTripperExplicitTLS(t *testing.T) { if !bytes.Equal([]byte{1, 2, 3, 4}, b) { t.Errorf("Got %v", b) } + + // Test nonexistent file + req, err = http.NewRequest(http.MethodGet, "ftp://"+addr+"/nonexistent.file", nil) + if err != nil { + t.Fatal(err) + } + req.URL.User = url.UserPassword("goftp", "rocks") + res, err = config.RoundTrip(req) + if err == nil { + t.Errorf("expected non-nil err") + } + if want, got := replyFileError, res.StatusCode; want != got { + t.Errorf("res.StatusCode: want: %v got: %v", want, got) + } } } @@ -129,5 +143,19 @@ func TestRoundTripperImplicitTLS(t *testing.T) { if !bytes.Equal([]byte{1, 2, 3, 4}, b) { t.Errorf("Got %v", b) } + + // Test nonexistent file + req, err = http.NewRequest(http.MethodGet, "ftp://"+addr+"/nonexistent.file", nil) + if err != nil { + t.Fatal(err) + } + req.URL.User = url.UserPassword("goftp", "rocks") + res, err = config.RoundTrip(req) + if err == nil { + t.Errorf("expected non-nil err") + } + if want, got := replyFileError, res.StatusCode; want != got { + t.Errorf("res.StatusCode: want: %v got: %v", want, got) + } } } From bb93b68955a843accc1dc1a87e466193d242ff89 Mon Sep 17 00:00:00 2001 From: Randy Reddig Date: Wed, 24 Mar 2021 15:29:03 -0700 Subject: [PATCH 36/37] silence test ftpd logs --- main_test.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/main_test.go b/main_test.go index ba4ac27..c57a7ec 100644 --- a/main_test.go +++ b/main_test.go @@ -93,7 +93,7 @@ func startPureFTPD(addrs []string, binary string) (func(), error) { cmd.Env = []string{fmt.Sprintf("FTP_ANON_DIR=%s/testroot", cwd)} - cmd.Stderr = os.Stderr + // cmd.Stderr = os.Stderr err = cmd.Start() if err != nil { @@ -138,7 +138,7 @@ func startProFTPD() (func(), error) { // "--debug", "10", ) - cmd.Stderr = os.Stderr + // cmd.Stderr = os.Stderr err = cmd.Start() if err != nil { From 54f578753a9e6c525c7662e350a967e32e4c339c Mon Sep 17 00:00:00 2001 From: Randy Reddig Date: Wed, 24 Mar 2021 16:01:24 -0700 Subject: [PATCH 37/37] http.RoundTripper should return res or err, not both --- round_tripper.go | 9 ++++----- round_tripper_test.go | 22 +++++++++------------- 2 files changed, 13 insertions(+), 18 deletions(-) diff --git a/round_tripper.go b/round_tripper.go index 0a566b3..5784965 100644 --- a/round_tripper.go +++ b/round_tripper.go @@ -56,12 +56,11 @@ func (config Config) RoundTrip(req *http.Request) (*http.Response, error) { w.CloseWithError(client.Retrieve(path, w)) }() _, err = brc.Peek(1) // Get error immediately on bad read - - // Simulate HTTP response semantics - if err, ok := err.(ftpError); ok { - res.StatusCode = err.Code() - res.Status = err.Message() + if err != nil { + res.Body.Close() + res = nil } else { + // Simulate HTTP response semantics res.StatusCode = 200 res.Status = http.StatusText(res.StatusCode) } diff --git a/round_tripper_test.go b/round_tripper_test.go index 33a1476..c8bf49d 100644 --- a/round_tripper_test.go +++ b/round_tripper_test.go @@ -33,14 +33,10 @@ func TestRoundTripperTimeoutConnect(t *testing.T) { } t0 := time.Now() - res, _ := config.RoundTrip(req) - // Config.RoundTrip calls Client.Retrieve in a goroutine - // so large file reads are unbuffered. - _, err = ioutil.ReadAll(res.Body) - res.Body.Close() + _, err = config.RoundTrip(req) delta := time.Since(t0) if err == nil || !err.(Error).Temporary() { - t.Error("Expected a timeout error") + t.Errorf("Expected a timeout error: %v", err) } offBy := delta - config.Timeout @@ -97,11 +93,11 @@ func TestRoundTripperExplicitTLS(t *testing.T) { } req.URL.User = url.UserPassword("goftp", "rocks") res, err = config.RoundTrip(req) - if err == nil { - t.Errorf("expected non-nil err") + if res != nil { + t.Errorf("expected nil http.Response: %v", res) } - if want, got := replyFileError, res.StatusCode; want != got { - t.Errorf("res.StatusCode: want: %v got: %v", want, got) + if err == nil { + t.Errorf("expected non-nil error") } } } @@ -151,11 +147,11 @@ func TestRoundTripperImplicitTLS(t *testing.T) { } req.URL.User = url.UserPassword("goftp", "rocks") res, err = config.RoundTrip(req) + if res != nil { + t.Errorf("expected nil http.Response: %v", res) + } if err == nil { t.Errorf("expected non-nil err") } - if want, got := replyFileError, res.StatusCode; want != got { - t.Errorf("res.StatusCode: want: %v got: %v", want, got) - } } }