Skip to content

Commit

Permalink
Fix hang when using ExplicitTLS to certain servers. (#283)
Browse files Browse the repository at this point in the history
In #282 it was discovered that doing the tls Handshake immediately on
connection causes some FTP servers (proftpd and pureftpd) to hang.

The exact cause of this is unknown, but this patch works around the
problem by not doing the Handsake initially, and only doing it at the
end if we were attempting to upload a zero length file.

Doing the Handshake at the end was originally added in
a4e9650 however it got reverted in 212daf2 which
used tls.DialWithDialer to do the handshake. Unfortunately
tls.DialWithDialer seems to trigger the hanging bug.

See: https://forum.rclone.org/t/rclone-ftps-explicit-rclone-touch-empty-files-proftpd-unable-to-build-data-connection-operation-not-permitted/22522
See: rclone/rclone#6426 (comment)
Fixes #282
  • Loading branch information
ncw authored Feb 8, 2023
1 parent 58cb524 commit 9e39e2c
Showing 1 changed file with 32 additions and 2 deletions.
34 changes: 32 additions & 2 deletions ftp.go
Original file line number Diff line number Diff line change
Expand Up @@ -559,7 +559,24 @@ func (c *ServerConn) openDataConn() (net.Conn, error) {
}

if c.options.tlsConfig != nil {
return tls.DialWithDialer(&c.options.dialer, "tcp", addr, c.options.tlsConfig)
// We don't use tls.DialWithDialer here (which does Dial, create
// the Client and then do the Handshake) because it seems to
// hang with some FTP servers, namely proftpd and pureftpd.
//
// Instead we do Dial, create the Client and wait for the first
// Read or Write to trigger the Handshake.
//
// This means that if we are uploading a zero sized file, we
// need to make sure we do the Handshake explicitly as Write
// won't have been called. This is done in StorFrom().
//
// See: https://github.com/jlaffaye/ftp/issues/282
conn, err := c.options.dialer.Dial("tcp", addr)
if err != nil {
return nil, err
}
tlsConn := tls.Client(conn, c.options.tlsConfig)
return tlsConn, nil
}

return c.options.dialer.Dial("tcp", addr)
Expand Down Expand Up @@ -912,8 +929,21 @@ func (c *ServerConn) StorFrom(path string, r io.Reader, offset uint64) error {
// response otherwise if the failure is not due to a connection problem,
// for example the server denied the upload for quota limits, we miss
// the response and we cannot use the connection to send other commands.
if _, err := io.Copy(conn, r); err != nil {
if n, err := io.Copy(conn, r); err != nil {
errs = multierror.Append(errs, err)
} else if n == 0 {
// If we wrote no bytes and got no error, make sure we call
// tls.Handshake on the connection as it won't get called
// unless Write() is called. (See comment in openDataConn()).
//
// ProFTP doesn't like this and returns "Unable to build data
// connection: Operation not permitted" when trying to upload
// an empty file without this.
if do, ok := conn.(interface{ Handshake() error }); ok {
if err := do.Handshake(); err != nil {
errs = multierror.Append(errs, err)
}
}
}

if err := conn.Close(); err != nil {
Expand Down

0 comments on commit 9e39e2c

Please sign in to comment.