Skip to content

Config.DialContext hook and http.RoundTripper support #31

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
ydnar wants to merge 37 commits into
base: master
Choose a base branch
from
Open

Config.DialContext hook and http.RoundTripper support #31

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
37 commits
Select commit Hold shift + click to select a range
170427b
use 'package goftp' in tests so this fork will work
ydnar Jun 20, 2018
4dab2f9
Dial with (optional) Config.DialContext
ydnar Jun 20, 2018
c48a417
Use pconn.host instead of pconn.controlConn.RemoteAddr().
ydnar Jun 21, 2018
530ab03
Add Transport type that implements http.RoundTripper.
ydnar Jun 21, 2018
f52b2af
test for ErrSkipAltProtocol
ydnar Jun 21, 2018
ba054c4
Address feedback: set up default DialContext after defaulting Timeout
ydnar Jul 11, 2018
9ee536b
Address feedback: config.Timeout is always non-zero
ydnar Jul 11, 2018
ae0500e
Address feedback: restore 930d480
ydnar Jul 11, 2018
1855f7c
Address feedback: override Config.User,Password in Transport if req.U…
ydnar Jul 11, 2018
159f811
Address feedback: document when Config vs URL user/password are used
ydnar Jul 11, 2018
ceeaabf
go.mod: Go module support for Go 1.16+
Mar 20, 2021
bd0d320
build_test_server.sh: update to build on macOS Big Sur
ydnar Mar 20, 2021
eea0e43
.github/workflows/go: add GitHub Actions CI
ydnar Mar 20, 2021
34495c4
.github/workflows/go: debugging step
ydnar Mar 20, 2021
76b84a2
.github/workflows/go: disable IPv6
ydnar Mar 20, 2021
9bbc2bd
.github/workflows/go: let’s try this another way
ydnar Mar 20, 2021
2cdbbdf
.github/workflows/go: DON’T disable IPv6
ydnar Mar 20, 2021
0dd5fc3
Use different port for IPv4 and IPv6 listeners
ydnar Mar 20, 2021
53f68b3
Attempt to test without IPv6
ydnar Mar 20, 2021
1cae939
Revert 0dd5fc3 and 53f68b3
ydnar Mar 20, 2021
53dec56
enable logs in testing
ydnar Mar 20, 2021
67f2756
Generate server key and cert on the fly
ydnar Mar 20, 2021
53390d7
Remove Travis CI
ydnar Mar 20, 2021
531cdf9
revert 170427b
ydnar Mar 20, 2021
ec080cc
check err before creating tls.Client
ydnar Mar 20, 2021
08de34c
Remove Transport type, just add RoundTrip method to Config
ydnar Mar 20, 2021
4ad1a3c
use time.Since
ydnar Mar 20, 2021
51b9ef2
Fix Go static check warnings
ydnar Mar 20, 2021
b98cc35
.github/workflows/go: test against Go 1.12–1.16
ydnar Mar 20, 2021
94163bf
.github/workflows/go: no ^ prefix in Go version matrix
ydnar Mar 20, 2021
8af54cf
.github/workflows/go: increase timeout to 15m
ydnar Mar 20, 2021
f7cdb1b
use different port for http.RoundTripper tests with implicit TLS
ydnar Mar 20, 2021
c531c0b
Just run the implicit TLS server in TestMain
ydnar Mar 20, 2021
4532bb1
Simulate HTTP semantics (200 OK) in response
ydnar Mar 24, 2021
00d39f2
Use bufio.Reader to peek at first byte of downloaded file to synchron…
ydnar Mar 24, 2021
bb93b68
silence test ftpd logs
ydnar Mar 24, 2021
54f5787
http.RoundTripper should return res or err, not both
ydnar Mar 24, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
40 changes: 40 additions & 0 deletions .github/workflows/go.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,40 @@
name: Go

on:
push:
branches:
- master
pull_request:

jobs:
test:
name: Test
strategy:
matrix:
go_version:
- 1.12
- 1.13
- 1.14
- 1.15
- 1.16
runs-on: ubuntu-latest
timeout-minutes: 15
steps:
- name: Checkout repo
uses: actions/checkout@v2
with:
submodules: recursive

- name: Set up Go
uses: actions/setup-go@v2
with:
go-version: ${{ matrix.go_version }}

- 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 ./...
11 changes: 0 additions & 11 deletions .travis.yml
View file
Open in desktop

This file was deleted.

75 changes: 19 additions & 56 deletions build_test_server.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,26 +1,22 @@
#!/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
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

Expand Down Expand Up @@ -51,46 +47,15 @@ TLSOptions NoSessionReuseRequired
</IfModule>
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

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
# 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'

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
Expand All @@ -106,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
Expand All @@ -121,4 +84,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
36 changes: 23 additions & 13 deletions client.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
package goftp

import (
"context"
"crypto/tls"
"errors"
"fmt"
Expand Down Expand Up @@ -109,6 +110,10 @@ type Config struct {
// 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
Expand Down Expand Up @@ -173,7 +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.ConnectionsPerHost <= 0 {
config.ConnectionsPerHost = 5
}
Expand All @@ -182,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"
}
Expand Down Expand Up @@ -240,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...),
)
}
Expand Down Expand Up @@ -367,18 +377,13 @@ func (c *Client) openConn(idx int, host string) (pconn *persistentConn, err erro
epsvNotSupported: c.config.DisableEPSV,
}

var conn net.Conn
// FIXME: this method should accept a parent Context
ctx, cancel := context.WithTimeout(context.Background(), c.config.Timeout)
defer cancel()

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)
}
var conn net.Conn
pconn.debug("opening control connection to %s", host)
conn, err = c.config.DialContext(ctx, "tcp", host)

var (
code int
Expand All @@ -397,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()
Expand Down
22 changes: 14 additions & 8 deletions client_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,9 @@ package goftp

import (
"bytes"
"context"
"crypto/tls"
"net"
"sync"
"testing"
"time"
Expand All @@ -19,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")
Expand All @@ -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,
},
Expand All @@ -64,20 +73,17 @@ 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")
}
}
}

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{
Expand Down
2 changes: 1 addition & 1 deletion file_system.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
3 changes: 3 additions & 0 deletions go.mod
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
module github.com/secsy/goftp

go 1.14
10 changes: 9 additions & 1 deletion main_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,12 @@ var (
)

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...)

Expand All @@ -49,6 +55,7 @@ func TestMain(m *testing.M) {

var ret int
func() {
defer implicitCloser()
defer pureCloser()
defer proCloser()
ret = m.Run()
Expand Down Expand Up @@ -86,6 +93,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)
Expand Down Expand Up @@ -129,7 +138,6 @@ func startProFTPD() (func(), error) {
// "--debug", "10",
)

// cmd.Stdout = os.Stdout
// cmd.Stderr = os.Stderr

err = cmd.Start()
Expand Down
11 changes: 8 additions & 3 deletions persistent_connection.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ package goftp

import (
"bufio"
"context"
"crypto/tls"
"fmt"
"net"
Expand Down Expand Up @@ -187,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...),
)
Expand Down Expand Up @@ -310,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
Expand Down Expand Up @@ -418,8 +419,12 @@ func (pconn *persistentConn) prepareDataConn() (func() (net.Conn, error), error)
return nil, err
}

// 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 := net.DialTimeout("tcp", host, pconn.config.Timeout)
dc, netErr := pconn.config.DialContext(ctx, "tcp", host)

if netErr != nil {
var isTemporary bool
Expand Down
Loading