Commit 50b39b74 authored by David Fifield's avatar David Fifield
Browse files

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.
parent d433318f

examples/dummy-client/dummy-client.go

+3 −3
Original line number Original line Diff line number Diff line
@@ -71,11 +71,11 @@ func acceptLoop(ln *pt.SocksListener) error { 
	for {

	for {

		conn, err := ln.AcceptSocks()

		conn, err := ln.AcceptSocks()

		if err != nil {

		if err != nil {

			if e, ok := err.(net.Error); ok && !e.Temporary() {

			if e, ok := err.(net.Error); ok && e.Temporary() {

				return err

			}

				continue

				continue

			}

			}

			return err

		}

		go handler(conn)

		go handler(conn)

	}

	}

}

}

examples/dummy-server/dummy-server.go

+3 −3
Original line number Original line Diff line number Diff line
@@ -68,11 +68,11 @@ func acceptLoop(ln net.Listener) error { 
	for {

	for {

		conn, err := ln.Accept()

		conn, err := ln.Accept()

		if err != nil {

		if err != nil {

			if e, ok := err.(net.Error); ok && !e.Temporary() {

			if e, ok := err.(net.Error); ok && e.Temporary() {

				return err

			}

				continue

				continue

			}

			}

			return err

		}

		go handler(conn)

		go handler(conn)

	}

	}

}

}

pt.go

+6 −6
Original line number Original line Diff line number Diff line
@@ -23,11 +23,11 @@ 
// 		for {

// 		for {

// 			conn, err := ln.AcceptSocks()

// 			conn, err := ln.AcceptSocks()

// 			if err != nil {

// 			if err != nil {

// 				if e, ok := err.(net.Error); ok && !e.Temporary() {

// 				if e, ok := err.(net.Error); ok && e.Temporary() {

// 					return err

// 				}

// 					continue

// 					continue

// 				}

// 				}

// 				return err

// 			}

// 			go handler(conn)

// 			go handler(conn)

// 		}

// 		}

// 		return nil

// 		return nil
@@ -80,11 +80,11 @@ 
// 		for {

// 		for {

// 			conn, err := ln.Accept()

// 			conn, err := ln.Accept()

// 			if err != nil {

// 			if err != nil {

// 				if e, ok := err.(net.Error); ok && !e.Temporary() {

// 				if e, ok := err.(net.Error); ok && e.Temporary() {

// 					return err

// 				}

// 					continue

// 					continue

// 				}

// 				}

// 				return err

// 			}

// 			go handler(conn)

// 			go handler(conn)

// 		}

// 		}

// 		return nil

// 		return nil

socks.go

+12 −11
Original line number Original line Diff line number Diff line
@@ -73,11 +73,11 @@ func (conn *SocksConn) Reject() error { 
// 		conn, err := ln.AcceptSocks()

// 		conn, err := ln.AcceptSocks()

// 		if err != nil {

// 		if err != nil {

// 			log.Printf("accept error: %s", err)

// 			log.Printf("accept error: %s", err)

// 			if e, ok := err.(net.Error); ok && !e.Temporary() {

// 			if e, ok := err.(net.Error); ok && e.Temporary() {

// 				break

// 			}

// 				continue

// 				continue

// 			}

// 			}

// 			break

// 		}

// 		go handleConn(conn)

// 		go handleConn(conn)

// 	}

// 	}

type SocksListener struct {

type SocksListener struct {
@@ -118,16 +118,17 @@ func (ln *SocksListener) Accept() (net.Conn, error) { 
// 	for {

// 	for {

// 		conn, err := ln.AcceptSocks()

// 		conn, err := ln.AcceptSocks()

// 		if err != nil {

// 		if err != nil {

// 			if e, ok := err.(net.Error); ok && !e.Temporary() {

// 			if e, ok := err.(net.Error); ok && e.Temporary() {

// 				log.Printf("permanent accept error; giving up: %s", err)

// 				break

// 			}

// 				log.Printf("temporary accept error; trying again: %s", err)

// 				log.Printf("temporary accept error; trying again: %s", err)

// 				continue

// 				continue

// 			}

// 			}

// 			log.Printf("permanent accept error; giving up: %s", err)

// 			break

// 		}

// 		go handleConn(conn)

// 		go handleConn(conn)

// 	}

// 	}

func (ln *SocksListener) AcceptSocks() (*SocksConn, error) {

func (ln *SocksListener) AcceptSocks() (*SocksConn, error) {

retry:

	c, err := ln.Listener.Accept()

	c, err := ln.Listener.Accept()

	if err != nil {

	if err != nil {

		return nil, err

		return nil, err
@@ -137,17 +138,17 @@ func (ln *SocksListener) AcceptSocks() (*SocksConn, error) { 
	err = conn.SetDeadline(time.Now().Add(socksRequestTimeout))

	err = conn.SetDeadline(time.Now().Add(socksRequestTimeout))

	if err != nil {

	if err != nil {

		conn.Close()

		conn.Close()

		return nil, err

		goto retry

	}

	}

	conn.Req, err = readSocks4aConnect(conn)

	conn.Req, err = readSocks4aConnect(conn)

	if err != nil {

	if err != nil {

		conn.Close()

		conn.Close()

		return nil, err

		goto retry

	}

	}

	err = conn.SetDeadline(time.Time{})

	err = conn.SetDeadline(time.Time{})

	if err != nil {

	if err != nil {

		conn.Close()

		conn.Close()

		return nil, err

		goto retry

	}

	}

	return conn, nil

	return conn, nil

}

}