From 58864d889b55b7cf44ecc6fff245d4113cbe19e1 Mon Sep 17 00:00:00 2001 From: Davide D'Agostino Date: Mon, 1 May 2017 18:18:54 -0700 Subject: [PATCH 1/4] Avoid forever lock If we close the connection two times the second time will hang forever waiting for a server code. --- ftp.go | 15 ++++++++++----- 1 file changed, 10 insertions(+), 5 deletions(-) diff --git a/ftp.go b/ftp.go index 567e2fd..cafb986 100644 --- a/ftp.go +++ b/ftp.go @@ -47,8 +47,9 @@ type Entry struct { // Response represents a data-connection type Response struct { - conn net.Conn - c *ServerConn + conn net.Conn + c *ServerConn + connClosed bool } // Connect is an alias to Dial, for backward compatibility @@ -337,7 +338,7 @@ func (c *ServerConn) NameList(path string) (entries []string, err error) { return } - r := &Response{conn, c} + r := &Response{conn, c, false} defer r.Close() scanner := bufio.NewScanner(r) @@ -368,7 +369,7 @@ func (c *ServerConn) List(path string) (entries []*Entry, err error) { return } - r := &Response{conn, c} + r := &Response{conn, c, false} defer r.Close() scanner := bufio.NewScanner(r) @@ -445,7 +446,7 @@ func (c *ServerConn) RetrFrom(path string, offset uint64) (*Response, error) { return nil, err } - return &Response{conn, c}, nil + return &Response{conn, c, false}, nil } // Stor issues a STOR FTP command to store a file to the remote FTP server. @@ -537,11 +538,15 @@ func (r *Response) Read(buf []byte) (int, error) { // Close implements the io.Closer interface on a FTP data connection. func (r *Response) Close() error { + if r.connClosed == true { + return nil + } err := r.conn.Close() _, _, err2 := r.c.conn.ReadResponse(StatusClosingDataConnection) if err2 != nil { err = err2 } + r.connClosed = true return err } From 7e3820b35dcf17f546d5d70d284f92bd12775f53 Mon Sep 17 00:00:00 2001 From: Davide D'Agostino Date: Thu, 4 May 2017 13:03:00 -0700 Subject: [PATCH 2/4] Address comments --- ftp.go | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/ftp.go b/ftp.go index cafb986..317002d 100644 --- a/ftp.go +++ b/ftp.go @@ -338,7 +338,7 @@ func (c *ServerConn) NameList(path string) (entries []string, err error) { return } - r := &Response{conn, c, false} + r := &Response{conn: conn, c: c} defer r.Close() scanner := bufio.NewScanner(r) @@ -369,7 +369,7 @@ func (c *ServerConn) List(path string) (entries []*Entry, err error) { return } - r := &Response{conn, c, false} + r := &Response{conn: conn, c: c} defer r.Close() scanner := bufio.NewScanner(r) @@ -446,7 +446,7 @@ func (c *ServerConn) RetrFrom(path string, offset uint64) (*Response, error) { return nil, err } - return &Response{conn, c, false}, nil + return &Response{conn: conn, c: c}, nil } // Stor issues a STOR FTP command to store a file to the remote FTP server. @@ -538,7 +538,7 @@ func (r *Response) Read(buf []byte) (int, error) { // Close implements the io.Closer interface on a FTP data connection. func (r *Response) Close() error { - if r.connClosed == true { + if r.connClosed { return nil } err := r.conn.Close() From cb362c410118164e9c6f2db0e16d095525f9ef94 Mon Sep 17 00:00:00 2001 From: Davide D'Agostino Date: Thu, 4 May 2017 17:46:29 -0700 Subject: [PATCH 3/4] Rename connClosed -> closed --- client_test.go | 1 + ftp.go | 10 +++++----- 2 files changed, 6 insertions(+), 5 deletions(-) diff --git a/client_test.go b/client_test.go index 0409da0..8297e49 100644 --- a/client_test.go +++ b/client_test.go @@ -81,6 +81,7 @@ func testConn(t *testing.T, disableEPSV bool) { t.Errorf("'%s'", buf) } r.Close() + r.Close() // test we can close two times } // Read with deadline diff --git a/ftp.go b/ftp.go index 317002d..17fe35a 100644 --- a/ftp.go +++ b/ftp.go @@ -47,9 +47,9 @@ type Entry struct { // Response represents a data-connection type Response struct { - conn net.Conn - c *ServerConn - connClosed bool + conn net.Conn + c *ServerConn + closed bool } // Connect is an alias to Dial, for backward compatibility @@ -538,7 +538,7 @@ func (r *Response) Read(buf []byte) (int, error) { // Close implements the io.Closer interface on a FTP data connection. func (r *Response) Close() error { - if r.connClosed { + if r.closed { return nil } err := r.conn.Close() @@ -546,7 +546,7 @@ func (r *Response) Close() error { if err2 != nil { err = err2 } - r.connClosed = true + r.closed = true return err } From 980e2e09a582c080954c2c5a44627099fe3eea53 Mon Sep 17 00:00:00 2001 From: Davide D'Agostino Date: Thu, 4 May 2017 19:03:56 -0700 Subject: [PATCH 4/4] Document that closing two times doesn't do anything --- ftp.go | 1 + 1 file changed, 1 insertion(+) diff --git a/ftp.go b/ftp.go index 17fe35a..c29cb38 100644 --- a/ftp.go +++ b/ftp.go @@ -537,6 +537,7 @@ func (r *Response) Read(buf []byte) (int, error) { } // Close implements the io.Closer interface on a FTP data connection. +// After the first call, Close will do nothing and return nil. func (r *Response) Close() error { if r.closed { return nil