[tor-commits] [goptlib/master] Don't report errors that are not caused by Accept in AcceptSocks.

[dcf at torproject.org]

commit 50b39b746c6ff34bf31977b658848d876ee84fbf
Author: David Fifield <david at bamsoftware.com>
Date:   Wed Jun 24 01:58:25 2015 -0700

    Don't report errors that are not caused by Accept in AcceptSocks.
    
    This means that anything that is not a net.Error, or is a net.Error but
    is not Temporary, should be considered to be a permanent error by the
    caller. Sample code now shows the new error-checking convention.
    Previously, we used the convention that a non-net.Error should be
    considered temporary, because it could have been caused by a failed
    SOCKS negotiation, for example. Now those errors are simply not returned
    to the caller. See https://trac.torproject.org/projects/tor/ticket/14135.
    
    In summary, previous behavior is this:
    	net.Error, Temporary:     caller should try again
    	net.Error, non-Temporary: caller should quit
    	other errors:             caller should try again
    It is now this:
    	net.Error, Temporary:     caller should try again
    	net.Error, non-Temporary: caller should quit
    	other errors:             caller should quit
    But now the "other errors" such as those caused by a bad SOCKS
    negotiation will not be reported by AcceptSocks.
    
    The practical effect of this change is almost nil; even if callers don't
    update their error-checking code, the only change is in the "other
    errors" that don't arise in normal use.
---
 examples/dummy-client/dummy-client.go |    6 +++---
 examples/dummy-server/dummy-server.go |    6 +++---
 pt.go                                 |   12 ++++++------
 socks.go                              |   23 ++++++++++++-----------
 4 files changed, 24 insertions(+), 23 deletions(-)

diff --git a/examples/dummy-client/dummy-client.go b/examples/dummy-client/dummy-client.go
index 5d99b0c..045a8f2 100644
--- a/examples/dummy-client/dummy-client.go
+++ b/examples/dummy-client/dummy-client.go
@@ -71,10 +71,10 @@ func acceptLoop(ln *pt.SocksListener) error {
 	for {
 		conn, err := ln.AcceptSocks()
 		if err != nil {
-			if e, ok := err.(net.Error); ok && !e.Temporary() {
-				return err
+			if e, ok := err.(net.Error); ok && e.Temporary() {
+				continue
 			}
-			continue
+			return err
 		}
 		go handler(conn)
 	}
diff --git a/examples/dummy-server/dummy-server.go b/examples/dummy-server/dummy-server.go
index ea91be9..70251ea 100644
--- a/examples/dummy-server/dummy-server.go
+++ b/examples/dummy-server/dummy-server.go
@@ -68,10 +68,10 @@ func acceptLoop(ln net.Listener) error {
 	for {
 		conn, err := ln.Accept()
 		if err != nil {
-			if e, ok := err.(net.Error); ok && !e.Temporary() {
-				return err
+			if e, ok := err.(net.Error); ok && e.Temporary() {
+				continue
 			}
-			continue
+			return err
 		}
 		go handler(conn)
 	}
diff --git a/pt.go b/pt.go
index fc2141e..a8c815e 100644
--- a/pt.go
+++ b/pt.go
@@ -23,10 +23,10 @@
 // 		for {
 // 			conn, err := ln.AcceptSocks()
 // 			if err != nil {
-// 				if e, ok := err.(net.Error); ok && !e.Temporary() {
-// 					return err
+// 				if e, ok := err.(net.Error); ok && e.Temporary() {
+// 					continue
 // 				}
-// 				continue
+// 				return err
 // 			}
 // 			go handler(conn)
 // 		}
@@ -80,10 +80,10 @@
 // 		for {
 // 			conn, err := ln.Accept()
 // 			if err != nil {
-// 				if e, ok := err.(net.Error); ok && !e.Temporary() {
-// 					return err
+// 				if e, ok := err.(net.Error); ok && e.Temporary() {
+// 					continue
 // 				}
-// 				continue
+// 				return err
 // 			}
 // 			go handler(conn)
 // 		}
diff --git a/socks.go b/socks.go
index eba318b..9a764b8 100644
--- a/socks.go
+++ b/socks.go
@@ -73,10 +73,10 @@ func (conn *SocksConn) Reject() error {
 // 		conn, err := ln.AcceptSocks()
 // 		if err != nil {
 // 			log.Printf("accept error: %s", err)
-// 			if e, ok := err.(net.Error); ok && !e.Temporary() {
-// 				break
+// 			if e, ok := err.(net.Error); ok && e.Temporary() {
+// 				continue
 // 			}
-// 			continue
+// 			break
 // 		}
 // 		go handleConn(conn)
 // 	}
@@ -118,16 +118,17 @@ func (ln *SocksListener) Accept() (net.Conn, error) {
 // 	for {
 // 		conn, err := ln.AcceptSocks()
 // 		if err != nil {
-// 			if e, ok := err.(net.Error); ok && !e.Temporary() {
-// 				log.Printf("permanent accept error; giving up: %s", err)
-// 				break
+// 			if e, ok := err.(net.Error); ok && e.Temporary() {
+// 				log.Printf("temporary accept error; trying again: %s", err)
+// 				continue
 // 			}
-// 			log.Printf("temporary accept error; trying again: %s", err)
-// 			continue
+// 			log.Printf("permanent accept error; giving up: %s", err)
+// 			break
 // 		}
 // 		go handleConn(conn)
 // 	}
 func (ln *SocksListener) AcceptSocks() (*SocksConn, error) {
+retry:
 	c, err := ln.Listener.Accept()
 	if err != nil {
 		return nil, err
@@ -137,17 +138,17 @@ func (ln *SocksListener) AcceptSocks() (*SocksConn, error) {
 	err = conn.SetDeadline(time.Now().Add(socksRequestTimeout))
 	if err != nil {
 		conn.Close()
-		return nil, err
+		goto retry
 	}
 	conn.Req, err = readSocks4aConnect(conn)
 	if err != nil {
 		conn.Close()
-		return nil, err
+		goto retry
 	}
 	err = conn.SetDeadline(time.Time{})
 	if err != nil {
 		conn.Close()
-		return nil, err
+		goto retry
 	}
 	return conn, nil
 }