[tor-commits] [goptlib/master] Add tests for net.Error temporary error handling in AcceptSocks.

[dcf at torproject.org]

commit 2a2a9b7cbff0dc9ff452a39a4199cec264e76dfe
Author: David Fifield <david at bamsoftware.com>
Date:   Wed Jun 24 01:38:04 2015 -0700

    Add tests for net.Error temporary error handling in AcceptSocks.
    
    See https://trac.torproject.org/projects/tor/ticket/14135. We're going
    to make the recommended way to check for an error from AcceptSocks match
    the way in the golang standard library:
    	if e, ok := err.(net.Error); ok && e.Temporary() {
    		continue
    	} else {
    		break
    	}
    The new tests check that 1) AcceptSocks faithfully represents an
    underlying Accept error with respect to its net.Error-ness and its
    Temporary-ness, and 2) errors that happen post-Accept in AcceptSocks are
    either silently swallowed or else reported as a Temporary net.Error.
    
    The test fails at the moment: "socks_test.go:267: AcceptSocks returned
    non-net.Error: unexpected EOF".
---
 socks_test.go |  107 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 107 insertions(+)

diff --git a/socks_test.go b/socks_test.go
index 18d141a..7fee46a 100644
--- a/socks_test.go
+++ b/socks_test.go
@@ -2,8 +2,11 @@ package pt
 
 import (
 	"bytes"
+	"errors"
+	"io"
 	"net"
 	"testing"
+	"time"
 )
 
 func TestReadSocks4aConnect(t *testing.T) {
@@ -160,3 +163,107 @@ func TestSendSocks4aResponse(t *testing.T) {
 		}
 	}
 }
+
+var fakeListenerDistinguishedError = errors.New("distinguished error")
+
+// fakeListener is a fake dummy net.Listener that returns the given net.Conn and
+// error the first time Accept is called. After the first call, it returns
+// (nil, fakeListenerDistinguishedError).
+type fakeListener struct {
+	c   net.Conn
+	err error
+}
+
+func (ln *fakeListener) Accept() (net.Conn, error) {
+	c := ln.c
+	err := ln.err
+	ln.c = nil
+	ln.err = fakeListenerDistinguishedError
+	return c, err
+}
+
+func (ln *fakeListener) Close() error {
+	return nil
+}
+
+func (ln *fakeListener) Addr() net.Addr {
+	return &net.TCPAddr{IP: net.ParseIP("127.0.0.1"), Port: 0, Zone: ""}
+}
+
+// A trivial net.Error that lets you control whether it is considered Temporary.
+type netError struct {
+	errString string
+	temporary bool
+}
+
+func (e *netError) Error() string {
+	return e.errString
+}
+
+func (e *netError) Temporary() bool {
+	return e.temporary
+}
+
+func (e *netError) Timeout() bool {
+	return false
+}
+
+// The purpose of ignoreDeadlineConn is to wrap net.Pipe so that the deadline
+// functions don't return an error ("net.Pipe does not support deadlines").
+type ignoreDeadlineConn struct {
+	net.Conn
+}
+
+func (c *ignoreDeadlineConn) SetDeadline(t time.Time) error {
+	return nil
+}
+
+func (c *ignoreDeadlineConn) SetReadDeadline(t time.Time) error {
+	return nil
+}
+
+func (c *ignoreDeadlineConn) SetWriteDeadline(t time.Time) error {
+	return nil
+}
+
+func TestAcceptErrors(t *testing.T) {
+	// Check that AcceptSocks accurately reflects net.Errors returned by the
+	// underlying call to Accept. This is important for the handling of
+	// Temporary and non-Temporary errors. The loop iterates over
+	// non-net.Error, non-Temporary net.Error, and Temporary net.Error.
+	for _, expectedErr := range []error{io.EOF, &netError{"non-temp", false}, &netError{"temp", true}} {
+		ln := NewSocksListener(&fakeListener{nil, expectedErr})
+		_, err := ln.AcceptSocks()
+		if expectedNerr, ok := expectedErr.(net.Error); ok {
+			nerr, ok := err.(net.Error)
+			if !ok {
+				t.Errorf("AcceptSocks returned non-net.Error %v", nerr)
+			} else {
+				if expectedNerr.Temporary() != expectedNerr.Temporary() {
+					t.Errorf("AcceptSocks did not keep Temporary status of net.Error: %v", nerr)
+				}
+			}
+		}
+	}
+
+	c1, c2 := net.Pipe()
+	go func() {
+		// Bogus request: SOCKS 5 then EOF.
+		c2.Write([]byte("\x05\x01\x00"))
+		c2.Close()
+	}()
+	ln := NewSocksListener(&fakeListener{c: &ignoreDeadlineConn{c1}, err: nil})
+	_, err := ln.AcceptSocks()
+	// The error in parsing the SOCKS request must be either silently
+	// ignored, or else must be a Temporary net.Error. I.e., it must not be
+	// the io.ErrUnexpectedEOF caused by the short request.
+	if err == fakeListenerDistinguishedError {
+		// Was silently ignored.
+	} else if nerr, ok := err.(net.Error); ok {
+		if !nerr.Temporary() {
+			t.Errorf("AcceptSocks returned non-Temporary net.Error: %v", nerr)
+		}
+	} else {
+		t.Errorf("AcceptSocks returned non-net.Error: %v", err)
+	}
+}