From d0bf3e1647bf42b841740c3ae131cc146f203267 Mon Sep 17 00:00:00 2001 From: elcore Date: Sun, 27 Nov 2016 07:44:15 +0100 Subject: [PATCH 1/4] Fix network listener address comparison, fixes #1258 (#1273) * Fix issue #1258 * address comments * add a test * address comments 2 --- caddy.go | 4 ++-- caddy_test.go | 42 +++++++++++++++++++++++++++++++++++++++++- 2 files changed, 43 insertions(+), 3 deletions(-) diff --git a/caddy.go b/caddy.go index 415c59cff..39a3358ec 100644 --- a/caddy.go +++ b/caddy.go @@ -232,7 +232,7 @@ func HasListenerWithAddress(addr string) bool { func listenerAddrEqual(ln net.Listener, addr string) bool { lnAddr := ln.Addr().String() hostname, port, err := net.SplitHostPort(addr) - if err != nil || hostname != "" { + if err != nil { return lnAddr == addr } if lnAddr == net.JoinHostPort("::", port) { @@ -241,7 +241,7 @@ func listenerAddrEqual(ln net.Listener, addr string) bool { if lnAddr == net.JoinHostPort("0.0.0.0", port) { return true } - return false + return hostname != "" && lnAddr == addr } // TCPServer is a type that can listen and serve connections. diff --git a/caddy_test.go b/caddy_test.go index 51e93c8ac..bbb93090f 100644 --- a/caddy_test.go +++ b/caddy_test.go @@ -1,6 +1,10 @@ package caddy -import "testing" +import ( + "net" + "strconv" + "testing" +) /* // TODO @@ -56,3 +60,39 @@ func TestIsLoopback(t *testing.T) { } } } + +func TestListenerAddrEqual(t *testing.T) { + ln1, err := net.Listen("tcp", "[::]:0") + if err != nil { + t.Fatal(err) + } + defer ln1.Close() + + ln1port := strconv.Itoa(ln1.Addr().(*net.TCPAddr).Port) + + ln2, err := net.Listen("tcp", "127.0.0.1:0") + if err != nil { + t.Fatal(err) + } + defer ln2.Close() + + ln2port := strconv.Itoa(ln2.Addr().(*net.TCPAddr).Port) + + for i, test := range []struct { + ln net.Listener + addr string + expect bool + }{ + {ln1, ":1234", false}, + {ln1, "0.0.0.0:1234", false}, + {ln1, ":" + ln1port + "", true}, + {ln1, "0.0.0.0:" + ln1port + "", true}, + {ln2, "127.0.0.1:1234", false}, + {ln2, ":" + ln2port + "", false}, + {ln2, "127.0.0.1:" + ln2port + "", true}, + } { + if got, want := listenerAddrEqual(test.ln, test.addr), test.expect; got != want { + t.Errorf("Test %d (%s == %s): expected %v but was %v", i, test.addr, test.ln.Addr().String(), want, got) + } + } +} From fec840a8617e7f3730d7e98954713c5259e2151f Mon Sep 17 00:00:00 2001 From: Eldin Hadzic Date: Sun, 27 Nov 2016 08:55:17 +0000 Subject: [PATCH 2/4] Increase code coverage --- caddy_test.go | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/caddy_test.go b/caddy_test.go index bbb93090f..8bcd40f31 100644 --- a/caddy_test.go +++ b/caddy_test.go @@ -85,10 +85,12 @@ func TestListenerAddrEqual(t *testing.T) { }{ {ln1, ":1234", false}, {ln1, "0.0.0.0:1234", false}, + {ln1, "0.0.0.0", false}, {ln1, ":" + ln1port + "", true}, {ln1, "0.0.0.0:" + ln1port + "", true}, - {ln2, "127.0.0.1:1234", false}, {ln2, ":" + ln2port + "", false}, + {ln2, "127.0.0.1:1234", false}, + {ln2, "127.0.0.1", false}, {ln2, "127.0.0.1:" + ln2port + "", true}, } { if got, want := listenerAddrEqual(test.ln, test.addr), test.expect; got != want { From 36f8759a7b59df7175d2cbbbb3dd1fc77693bd93 Mon Sep 17 00:00:00 2001 From: Matthew Holt Date: Mon, 28 Nov 2016 22:26:54 -0700 Subject: [PATCH 3/4] Ensure some tests remove temporary directories they created --- caddyhttp/httpserver/context_test.go | 4 +++- caddyhttp/proxy/proxy_test.go | 20 ++++++++++++-------- 2 files changed, 15 insertions(+), 9 deletions(-) diff --git a/caddyhttp/httpserver/context_test.go b/caddyhttp/httpserver/context_test.go index 2e51e7d78..fdf263684 100644 --- a/caddyhttp/httpserver/context_test.go +++ b/caddyhttp/httpserver/context_test.go @@ -746,14 +746,16 @@ func TestFiles(t *testing.T) { // Create directory / files from test case. if test.fileNames != nil { - dirPath, err = ioutil.TempDir(fmt.Sprintf("%s", context.Root), "caddy_test") + dirPath, err = ioutil.TempDir(fmt.Sprintf("%s", context.Root), "caddy_ctxtest") if err != nil { + os.RemoveAll(dirPath) t.Fatalf(testPrefix+"Expected no error creating directory, got: '%s'", err.Error()) } for _, name := range test.fileNames { absFilePath := filepath.Join(dirPath, name) if err = ioutil.WriteFile(absFilePath, []byte(""), os.ModePerm); err != nil { + os.RemoveAll(dirPath) t.Fatalf(testPrefix+"Expected no error creating file, got: '%s'", err.Error()) } } diff --git a/caddyhttp/proxy/proxy_test.go b/caddyhttp/proxy/proxy_test.go index fe3772786..40c291c4d 100644 --- a/caddyhttp/proxy/proxy_test.go +++ b/caddyhttp/proxy/proxy_test.go @@ -229,10 +229,11 @@ func TestUnixSocketProxy(t *testing.T) { })) // Get absolute path for unix: socket - dir, err := ioutil.TempDir("", "caddy_test") + dir, err := ioutil.TempDir("", "caddy_proxytest") if err != nil { t.Fatalf("Failed to make temp dir to contain unix socket. %v", err) } + defer os.RemoveAll(dir) socketPath := filepath.Join(dir, "test_socket") // Change httptest.Server listener to listen to unix: socket @@ -283,20 +284,21 @@ func GetHTTPProxy(messageFormat string, prefix string) (*Proxy, *httptest.Server return newPrefixedWebSocketTestProxy(ts.URL, prefix), ts } -func GetSocketProxy(messageFormat string, prefix string) (*Proxy, *httptest.Server, error) { +func GetSocketProxy(messageFormat string, prefix string) (*Proxy, *httptest.Server, string, error) { ts := httptest.NewUnstartedServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { fmt.Fprintf(w, messageFormat, r.URL.String()) })) - dir, err := ioutil.TempDir("", "caddy_test") + dir, err := ioutil.TempDir("", "caddy_proxytest") if err != nil { - return nil, nil, fmt.Errorf("Failed to make temp dir to contain unix socket. %v", err) + return nil, nil, dir, fmt.Errorf("Failed to make temp dir to contain unix socket. %v", err) } socketPath := filepath.Join(dir, "test_socket") ln, err := net.Listen("unix", socketPath) if err != nil { - return nil, nil, fmt.Errorf("Unable to listen: %v", err) + os.RemoveAll(dir) + return nil, nil, dir, fmt.Errorf("Unable to listen: %v", err) } ts.Listener = ln @@ -304,7 +306,7 @@ func GetSocketProxy(messageFormat string, prefix string) (*Proxy, *httptest.Serv tsURL := strings.Replace(ts.URL, "http://", "unix:", 1) - return newPrefixedWebSocketTestProxy(tsURL, prefix), ts, nil + return newPrefixedWebSocketTestProxy(tsURL, prefix), ts, dir, nil } func GetTestServerMessage(p *Proxy, ts *httptest.Server, path string) (string, error) { @@ -370,8 +372,7 @@ func TestUnixSocketProxyPaths(t *testing.T) { } for _, test := range tests { - p, ts, err := GetSocketProxy(greeting, test.prefix) - + p, ts, tmpdir, err := GetSocketProxy(greeting, test.prefix) if err != nil { t.Fatalf("Getting socket proxy failed - %v", err) } @@ -379,12 +380,15 @@ func TestUnixSocketProxyPaths(t *testing.T) { actualMsg, err := GetTestServerMessage(p, ts, test.url) if err != nil { + os.RemoveAll(tmpdir) t.Fatalf("Getting server message failed - %v", err) } if actualMsg != test.expected { t.Errorf("Expected '%s' but got '%s' instead", test.expected, actualMsg) } + + os.RemoveAll(tmpdir) } } From 80eb45fcfb73be7ecf55caba443c0c2fcfbd278c Mon Sep 17 00:00:00 2001 From: Matthew Holt Date: Mon, 28 Nov 2016 23:37:22 -0700 Subject: [PATCH 4/4] tls: Improve flaky test depending on CPU scheduling (I think) --- caddytls/user_test.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/caddytls/user_test.go b/caddytls/user_test.go index 77d04d4aa..0b7490a68 100644 --- a/caddytls/user_test.go +++ b/caddytls/user_test.go @@ -166,12 +166,12 @@ func TestGetEmail(t *testing.T) { t.Fatalf("Error saving user %d: %v", i, err) } - // Change modified time so they're all different, so the test becomes deterministic + // Change modified time so they're all different and the test becomes more deterministic f, err := os.Stat(testStorage.user(eml)) if err != nil { t.Fatalf("Could not access user folder for '%s': %v", eml, err) } - chTime := f.ModTime().Add(-(time.Duration(i) * time.Second)) + chTime := f.ModTime().Add(-(time.Duration(i) * time.Hour)) // 1 second isn't always enough space! if err := os.Chtimes(testStorage.user(eml), chTime, chTime); err != nil { t.Fatalf("Could not change user folder mod time for '%s': %v", eml, err) }