brouzouf/ftp
1
0
Fork 0
Code Issues Pull Requests Actions 1 Packages Projects Releases Wiki Activity

Fix hang when using ExplicitTLS to certain servers. (#283)

Browse Source 
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
a4e9650823 however it got reverted in 212daf295f 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: https://github.com/rclone/rclone/issues/6426#issuecomment-1243993039
Fixes #282
This commit is contained in:
Nick Craig-Wood 2023-02-08 16:46:41 +00:00 committed by GitHub
parent 58cb524052
commit 9e39e2c406
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
1 changed files with 32 additions and 2 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

34
ftp.go
View File

@ -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)
@ -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 {