From 792ab9d3d0dc5fbf442cea894fe97f7bc4e39a4c Mon Sep 17 00:00:00 2001 From: Abdullah Gharaibeh Date: Thu, 16 May 2019 10:37:56 -0400 Subject: [PATCH 1/3] Added a unit test to verify that host header is preserved after probe redirect. --- pkg/probe/http/http_test.go | 36 ++++++++++++++++++++++++++++++++++++ 1 file changed, 36 insertions(+) diff --git a/pkg/probe/http/http_test.go b/pkg/probe/http/http_test.go index 096cb430439..70c4230a393 100644 --- a/pkg/probe/http/http_test.go +++ b/pkg/probe/http/http_test.go @@ -314,3 +314,39 @@ func TestHTTPProbeChecker_NonLocalRedirects(t *testing.T) { }) } } + +func TestHTTPProbeChecker_HostHeaderPreservedAfterRedirect(t *testing.T) { + hostHeader := "www.example.com" + + handler := http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + switch r.URL.Path { + case "/redirect": + http.Redirect(w, r, "/success", http.StatusFound) + case "/success": + if r.Host != hostHeader { + w.WriteHeader(http.StatusOK) + } else { + http.Error(w, "", http.StatusBadRequest) + } + default: + http.Error(w, "", http.StatusInternalServerError) + } + }) + server := httptest.NewServer(handler) + defer server.Close() + + t.Run("local", func(t *testing.T) { + prober := New(false) + target, err := url.Parse(server.URL + "/redirect") + require.NoError(t, err) + result, _, _ := prober.Probe(target, nil, wait.ForeverTestTimeout) + assert.Equal(t, probe.Success, result) + }) + t.Run("nonlocal", func(t *testing.T) { + prober := New(true) + target, err := url.Parse(server.URL + "/redirect") + require.NoError(t, err) + result, _, _ := prober.Probe(target, nil, wait.ForeverTestTimeout) + assert.Equal(t, probe.Success, result) + }) +} From 71b1565f2e7a7982859975ef6aff15d5af912fb2 Mon Sep 17 00:00:00 2001 From: Abdullah Gharaibeh Date: Thu, 16 May 2019 18:21:51 -0400 Subject: [PATCH 2/3] Fixed the unit test and added a failure case. --- pkg/probe/http/http_test.go | 44 +++++++++++++++++++++++-------------- 1 file changed, 28 insertions(+), 16 deletions(-) diff --git a/pkg/probe/http/http_test.go b/pkg/probe/http/http_test.go index 70c4230a393..062fe8f03e9 100644 --- a/pkg/probe/http/http_test.go +++ b/pkg/probe/http/http_test.go @@ -316,14 +316,15 @@ func TestHTTPProbeChecker_NonLocalRedirects(t *testing.T) { } func TestHTTPProbeChecker_HostHeaderPreservedAfterRedirect(t *testing.T) { - hostHeader := "www.example.com" + successHostHeader := "www.success.com" + failHostHeader := "www.fail.com" handler := http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { switch r.URL.Path { case "/redirect": http.Redirect(w, r, "/success", http.StatusFound) case "/success": - if r.Host != hostHeader { + if r.Host == successHostHeader { w.WriteHeader(http.StatusOK) } else { http.Error(w, "", http.StatusBadRequest) @@ -335,18 +336,29 @@ func TestHTTPProbeChecker_HostHeaderPreservedAfterRedirect(t *testing.T) { server := httptest.NewServer(handler) defer server.Close() - t.Run("local", func(t *testing.T) { - prober := New(false) - target, err := url.Parse(server.URL + "/redirect") - require.NoError(t, err) - result, _, _ := prober.Probe(target, nil, wait.ForeverTestTimeout) - assert.Equal(t, probe.Success, result) - }) - t.Run("nonlocal", func(t *testing.T) { - prober := New(true) - target, err := url.Parse(server.URL + "/redirect") - require.NoError(t, err) - result, _, _ := prober.Probe(target, nil, wait.ForeverTestTimeout) - assert.Equal(t, probe.Success, result) - }) + testCases := map[string]struct { + hostHeader string + expectedResult probe.Result + }{ + "success": {successHostHeader, probe.Success}, + "fail": {failHostHeader, probe.Failure}, + } + for desc, test := range testCases { + headers := http.Header{} + headers.Add("Host", test.hostHeader) + t.Run(desc+"local", func(t *testing.T) { + prober := New(false) + target, err := url.Parse(server.URL + "/redirect") + require.NoError(t, err) + result, _, _ := prober.Probe(target, headers, wait.ForeverTestTimeout) + assert.Equal(t, test.expectedResult, result) + }) + t.Run(desc+"nonlocal", func(t *testing.T) { + prober := New(true) + target, err := url.Parse(server.URL + "/redirect") + require.NoError(t, err) + result, _, _ := prober.Probe(target, headers, wait.ForeverTestTimeout) + assert.Equal(t, test.expectedResult, result) + }) + } } From ef2e5bb8ed1ba3e28c3d9b75fb923be64c9b5961 Mon Sep 17 00:00:00 2001 From: Abdullah Gharaibeh Date: Fri, 17 May 2019 10:48:43 -0400 Subject: [PATCH 3/3] Addressed reviewers comments. --- pkg/probe/http/http_test.go | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/pkg/probe/http/http_test.go b/pkg/probe/http/http_test.go index 062fe8f03e9..9f9afc30d6f 100644 --- a/pkg/probe/http/http_test.go +++ b/pkg/probe/http/http_test.go @@ -347,14 +347,16 @@ func TestHTTPProbeChecker_HostHeaderPreservedAfterRedirect(t *testing.T) { headers := http.Header{} headers.Add("Host", test.hostHeader) t.Run(desc+"local", func(t *testing.T) { - prober := New(false) + followNonLocalRedirects := false + prober := New(followNonLocalRedirects) target, err := url.Parse(server.URL + "/redirect") require.NoError(t, err) result, _, _ := prober.Probe(target, headers, wait.ForeverTestTimeout) assert.Equal(t, test.expectedResult, result) }) t.Run(desc+"nonlocal", func(t *testing.T) { - prober := New(true) + followNonLocalRedirects := true + prober := New(followNonLocalRedirects) target, err := url.Parse(server.URL + "/redirect") require.NoError(t, err) result, _, _ := prober.Probe(target, headers, wait.ForeverTestTimeout)