Feature - Single Port Mode (#1)

* Single Port Mode

Single port mode multiplexes all client communication onto the server's main listening port. Tracks connections with a map of strings to channels. The string representation of the remote address is
used as the key. ACK, DATA, and ERRORs are sent on the connection's channel. Calls to conn.readFromNet read off the channel instead of a network connection.

* Clean-up duplicate code. Add timeouts and error handling.
* Read data directly into conn.rx instead of through conn.buf
* Prevent creating a new timer on everyread from reqChan
* Refactor incoming datagram dispatch
* Improved handling out of sequence ACKs.
* Close connection before removing addr from reqMap.
* Correct existing tests
* Test client against single port server
* Prevent Travis CI using uppercase vCabbage in GOPATH
* Update README with information about single port mode
* Correct misspellings

.travis.yml

@@ -1,5 +1,6 @@
 language: go
 sudo: false
+go_import_path: github.com/vcabbage/trivialt
 go:
  - 1.5.4
  - 1.6.2
@@ -12,7 +13,7 @@ before_install:
   - go get golang.org/x/tools/cmd/cover
   - go get github.com/modocache/gover
 script:
-  - go test -v -covermode=count -coverprofile=trivialt.coverprofile . 
-  - go test -v -covermode=count -coverprofile=netascii.coverprofile ./netascii 
+  - go test -v -covermode=count -coverprofile=trivialt.coverprofile .
+  - go test -v -covermode=count -coverprofile=netascii.coverprofile ./netascii
   - gover
   - goveralls -coverprofile=gover.coverprofile -service=travis-ci

README.md

@@ -15,12 +15,30 @@ trivialt is a cross-platform, concurrent TFTP server and client. It can be used
 
 - [X] Binary Transfer ([RFC 1350](https://tools.ietf.org/html/rfc1350))
 - [X] Netascii Transfer ([RFC 1350](https://tools.ietf.org/html/rfc1350))
-- [X] Option Extention ([RFC 2347](https://tools.ietf.org/html/rfc2347))
+- [X] Option Extension ([RFC 2347](https://tools.ietf.org/html/rfc2347))
 - [X] Blocksize Option ([RFC 2348](https://tools.ietf.org/html/rfc2348))
 - [X] Timeout Interval Option ([RFC 2349](https://tools.ietf.org/html/rfc2349))
 - [X] Transfer Size Option ([RFC 2349](https://tools.ietf.org/html/rfc2349))
 - [X] Windowsize Option ([RFC 7440](https://tools.ietf.org/html/rfc7440))
 
+### Unique Features
+
+- __Single Port Mode__
+
+    TL;DR: It allows TFTP to work through firewalls.
+
+    A standard TFTP server implementation receives requests on port 69 and allocates a new high port (over 1024) dedicated to that request.
+    In single port mode, trivialt receives and responds to requests on the same port. If trivialt is started on port 69, all communication will
+    be done on port 69.
+
+    The primary use case of this feature is to play nicely with firewalls. Most firewalls will prevent the typical case where the server responds
+    back on a random port because they have no way of knowing that it is in response to a request that went out on port 69. In single port mode,
+    the firewall will see a request go out to a server on port 69 and that server respond back on the same port, which most firewalls will allow.
+
+    Of course if the firewall in question is configured to block TFTP connections, this setting won't help you.
+
+    Enable single port mode with the `--single-port` flag. This is currently marked experimental as is diverges from the TFTP standard.
+
 ## Installation
 
 If you have the Go toolchain installed you can simply `go get` the packages. This will download the source into your `$GOPATH` and install the binary to `$GOPATH/bin/trivialt`.
@@ -52,7 +70,8 @@ DESCRIPTION:
    the current directory.
 
 OPTIONS:
-   --writeable, -w	Enable file upload.
+   --writeable, -w	    Enable file upload.
+   --single-port, --sp	Enable single port mode. [Experimental]
 ```
 
 ```
@@ -127,10 +146,10 @@ trivialt's API was inspired by Go's well-known net/http API. If you can write a
 
 ### Configuration Functions
 
-One area that is noticably different from net/http is the configuration of clients and servers. trivialt uses "configuration functions" rather than the direct modification of the
+One area that is noticeably different from net/http is the configuration of clients and servers. trivialt uses "configuration functions" rather than the direct modification of the
 Client/Server struct or a configuration struct passed into the factory functions.
 
-A few explainations of this pattern:
+A few explanations of this pattern:
 * [Self-referential functions and the design of options](http://commandcenter.blogspot.com/2014/01/self-referential-functions-and-design.html) by Rob Pike
 * [Functional options for friendly APIs](https://www.youtube.com/watch?v=24lFtGHWxAQ) by Dave Cheney [video]
 

client_test.go

@@ -398,57 +398,61 @@ func TestClient_Get(t *testing.T) {
 	}
 
 	for label, c := range cases {
-		if (c.windowsOnly && runtime.GOOS != "windows") || (c.nixOnly && runtime.GOOS == "windows") {
-			t.Logf("skipping case %q marked windowsOnly:%t; nixOnly:%t; GOOS: %q", label, c.windowsOnly, c.nixOnly, runtime.GOOS)
-			continue
-		}
+		for _, singlePort := range []bool{true, false} {
+			label := fmt.Sprintf("%s, single port mode: %t", label, singlePort)
 
-		ip, port, close := newTestServer(t, func(w ReadRequest) {
-			if c.sendServerError {
-				w.WriteError(ErrCodeAccessViolation, "server error")
-				return
+			if (c.windowsOnly && runtime.GOOS != "windows") || (c.nixOnly && runtime.GOOS == "windows") {
+				t.Logf("skipping case %q marked windowsOnly:%t; nixOnly:%t; GOOS: %q", label, c.windowsOnly, c.nixOnly, runtime.GOOS)
+				continue
 			}
 
-			if !c.omitSize {
-				w.WriteSize(int64(len(c.response)))
-			}
-			w.Write([]byte(c.response))
-		}, nil)
-		defer close()
+			ip, port, close := newTestServer(t, singlePort, func(w ReadRequest) {
+				if c.sendServerError {
+					w.WriteError(ErrCodeAccessViolation, "server error")
+					return
+				}
 
-		client, err := NewClient(c.opts...)
-		if err != nil {
-			t.Fatal(err)
-		}
+				if !c.omitSize {
+					w.WriteSize(int64(len(c.response)))
+				}
+				w.Write([]byte(c.response))
+			}, nil)
+			defer close()
+
+			client, err := NewClient(c.opts...)
+			if err != nil {
+				t.Fatal(err)
+			}
 
-		url := strings.Replace(c.url, "#host#", ip, 1)
-		url = strings.Replace(url, "#port#", strconv.Itoa(port), 1)
+			url := strings.Replace(c.url, "#host#", ip, 1)
+			url = strings.Replace(url, "#port#", strconv.Itoa(port), 1)
 
-		file, err := client.Get(url)
-		if err != nil {
-			if match, _ := regexp.MatchString(c.expectedError, ErrorCause(err).Error()); !match {
-				t.Errorf("%s: expected error %q, got %q", label, c.expectedError, ErrorCause(err).Error())
+			file, err := client.Get(url)
+			if err != nil {
+				if match, _ := regexp.MatchString(c.expectedError, ErrorCause(err).Error()); !match {
+					t.Errorf("%s: expected error %q, got %q", label, c.expectedError, ErrorCause(err).Error())
+				}
+				continue
 			}
-			continue
-		}
 
-		response, err := ioutil.ReadAll(file)
-		if err != nil {
-			t.Fatal(label, err)
-		}
+			response, err := ioutil.ReadAll(file)
+			if err != nil {
+				t.Fatal(label, err)
+			}
 
-		// Data
-		if !reflect.DeepEqual(response, c.expectedResponse) {
-			if len(response) > 1000 || len(c.expectedResponse) > 1000 {
-				t.Errorf("%s: Response didn't match (over 1000 characters, omitting)", label)
-			} else {
-				t.Errorf("%s: Expected response to be %q, but it was %q", label, c.expectedResponse, response)
+			// Data
+			if !reflect.DeepEqual(response, c.expectedResponse) {
+				if len(response) > 1000 || len(c.expectedResponse) > 1000 {
+					t.Errorf("%s: Response didn't match (over 1000 characters, omitting)", label)
+				} else {
+					t.Errorf("%s: Expected response to be %q, but it was %q", label, c.expectedResponse, response)
+				}
 			}
-		}
 
-		// Size
-		if i, _ := file.Size(); i != c.expectedSize {
-			t.Errorf("%s: Expected size to be %d, but it was %d", label, c.expectedSize, i)
+			// Size
+			if i, _ := file.Size(); i != c.expectedSize {
+				t.Errorf("%s: Expected size to be %d, but it was %d", label, c.expectedSize, i)
+			}
 		}
 	}
 }
@@ -637,67 +641,71 @@ func TestClient_Put(t *testing.T) {
 	}
 
 	for label, c := range cases {
-		if (c.windowsOnly && runtime.GOOS != "windows") || (c.nixOnly && runtime.GOOS == "windows") {
-			t.Logf("skipping case %q marked windowsOnly:%t; nixOnly:%t; GOOS: %q", label, c.windowsOnly, c.nixOnly, runtime.GOOS)
-			continue
-		}
+		for _, singlePort := range []bool{true, false} {
+			label := fmt.Sprintf("%s, single port mode: %t", label, singlePort)
 
-		var wr WriteRequest
-		var data []byte
-		var mu sync.Mutex
-
-		ip, port, close := newTestServer(t, nil, func(w WriteRequest) {
-			mu.Lock()
-			defer mu.Unlock()
-			if c.sendServerError {
-				w.WriteError(ErrCodeAccessViolation, "server error")
-				return
+			if (c.windowsOnly && runtime.GOOS != "windows") || (c.nixOnly && runtime.GOOS == "windows") {
+				t.Logf("skipping case %q marked windowsOnly:%t; nixOnly:%t; GOOS: %q", label, c.windowsOnly, c.nixOnly, runtime.GOOS)
+				continue
 			}
-			wr = w
 
-			d, err := ioutil.ReadAll(w)
+			var wr WriteRequest
+			var data []byte
+			var mu sync.Mutex
+
+			ip, port, close := newTestServer(t, singlePort, nil, func(w WriteRequest) {
+				mu.Lock()
+				defer mu.Unlock()
+				if c.sendServerError {
+					w.WriteError(ErrCodeAccessViolation, "server error")
+					return
+				}
+				wr = w
+
+				d, err := ioutil.ReadAll(w)
+				if err != nil {
+					t.Fatal(err)
+				}
+				data = d
+			})
+			defer close()
+
+			client, err := NewClient(c.opts...)
 			if err != nil {
 				t.Fatal(err)
 			}
-			data = d
-		})
-		defer close()
 
-		client, err := NewClient(c.opts...)
-		if err != nil {
-			t.Fatal(err)
-		}
-
-		size := 0
-		if !c.omitSize {
-			size = len(c.send)
-		}
+			size := 0
+			if !c.omitSize {
+				size = len(c.send)
+			}
 
-		url := strings.Replace(c.url, "#host#", ip, 1)
-		url = strings.Replace(url, "#port#", strconv.Itoa(port), 1)
+			url := strings.Replace(c.url, "#host#", ip, 1)
+			url = strings.Replace(url, "#port#", strconv.Itoa(port), 1)
 
-		err = client.Put(url, bytes.NewReader(c.send), int64(size))
-		mu.Lock()
-		mu.Unlock()
-		if err != nil {
-			if match, _ := regexp.MatchString(c.expectedError, ErrorCause(err).Error()); !match {
-				t.Errorf("%s: expected error %q, got %q", label, c.expectedError, ErrorCause(err).Error())
+			err = client.Put(url, bytes.NewReader(c.send), int64(size))
+			mu.Lock()
+			mu.Unlock()
+			if err != nil {
+				if match, _ := regexp.MatchString(c.expectedError, ErrorCause(err).Error()); !match {
+					t.Errorf("%s: expected error %q, got %q", label, c.expectedError, ErrorCause(err).Error())
+				}
+				continue
 			}
-			continue
-		}
 
-		// Data
-		if !reflect.DeepEqual(data, c.expectedData) {
-			if len(data) > 1000 || len(c.expectedData) > 1000 {
-				t.Errorf("%s: Response didn't match (over 1000 characters, omitting)", label)
-			} else {
-				t.Errorf("%s: Expected response to be %q, but it was %q", label, c.expectedData, data)
+			// Data
+			if !reflect.DeepEqual(data, c.expectedData) {
+				if len(data) > 1000 || len(c.expectedData) > 1000 {
+					t.Errorf("%s: Response didn't match (over 1000 characters, omitting)", label)
+				} else {
+					t.Errorf("%s: Expected response to be %q, but it was %q", label, c.expectedData, data)
+				}
 			}
-		}
 
-		// Size
-		if size, _ := wr.Size(); size != c.expectedSize {
-			t.Errorf("%s: Expected size to be %d, but it was %d", label, c.expectedSize, size)
+			// Size
+			if size, _ := wr.Size(); size != c.expectedSize {
+				t.Errorf("%s: Expected size to be %d, but it was %d", label, c.expectedSize, size)
+			}
 		}
 	}
 }
@@ -801,8 +809,9 @@ func TestClient_parseURL(t *testing.T) {
 	}
 }
 
-func newTestServer(t *testing.T, rh ReadHandlerFunc, wh WriteHandlerFunc) (string, int, func()) {
-	s, err := NewServer("127.0.0.1:0")
+func newTestServer(t *testing.T, singlePort bool, rh ReadHandlerFunc, wh WriteHandlerFunc) (string, int, func()) {
+	s, err := NewServer("127.0.0.1:0", ServerSinglePort(singlePort))
+
 	if err != nil {
 		t.Fatalf("newTestServer: %v\n", err)
 	}

cmd/trivialt/main.go

@@ -80,6 +80,10 @@ func main() {
 					Name:  "writable, w",
 					Usage: "Enable file upload.",
 				},
+				cli.BoolFlag{
+					Name:  "single-port, sp",
+					Usage: "Enable single port mode. [Experimental]",
+				},
 			},
 			Description: `Serves files from the local file systemd.
    
@@ -119,6 +123,7 @@ func cmdServe(c *cli.Context) {
 	addr := c.Args().First()
 	path := c.Args().Get(1)
 	writable := c.Bool("writable")
+	singlePort := c.Bool("single-port")
 
 	if addr == "" {
 		addr = ":69"
@@ -132,7 +137,7 @@ func cmdServe(c *cli.Context) {
 	log.Printf("Starting TFTP Server on %q, serving %q\n", addr, root)
 	fs := &server{trivialt.FileServer(root)}
 
-	server, err := trivialt.NewServer(addr)
+	server, err := trivialt.NewServer(addr, trivialt.ServerSinglePort(singlePort))
 	if err != nil {
 		log.Fatalln(err)
 	}