From dc2d6a58fdf00fd3668797a3a0afc695f66d6b1f Mon Sep 17 00:00:00 2001 From: HunDunDM Date: Mon, 18 Jul 2022 16:27:20 +0800 Subject: [PATCH 1/4] add test Signed-off-by: HunDunDM --- pkg/dashboard/adapter/redirector_test.go | 112 +++++++++++++++++++++++ 1 file changed, 112 insertions(+) create mode 100644 pkg/dashboard/adapter/redirector_test.go diff --git a/pkg/dashboard/adapter/redirector_test.go b/pkg/dashboard/adapter/redirector_test.go new file mode 100644 index 00000000000..2b4ac78b17e --- /dev/null +++ b/pkg/dashboard/adapter/redirector_test.go @@ -0,0 +1,112 @@ +// Copyright 2022 TiKV Project Authors. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package adapter + +import ( + "io" + "net/http" + "net/http/httptest" + "testing" + + "github.com/stretchr/testify/require" + "github.com/stretchr/testify/suite" +) + +type redirectorTestSuite struct { + suite.Suite + + tempText string + tempServer *httptest.Server + + testName string + redirector *Redirector + + noRedirectHTTPClient *http.Client +} + +func TestRedirectorTestSuite(t *testing.T) { + suite.Run(t, new(redirectorTestSuite)) +} + +func (suite *redirectorTestSuite) SetupSuite() { + suite.tempText = "temp1" + suite.tempServer = httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + _, _ = io.WriteString(w, suite.tempText) + })) + + suite.testName = "test1" + suite.redirector = NewRedirector(suite.testName, nil) + suite.noRedirectHTTPClient = &http.Client{ + CheckRedirect: func(req *http.Request, via []*http.Request) error { + // ErrUseLastResponse can be returned by Client.CheckRedirect hooks to + // control how redirects are processed. If returned, the next request + // is not sent and the most recent response is returned with its body + // unclosed. + return http.ErrUseLastResponse + }, + } +} + +func (suite *redirectorTestSuite) TearDownSuite() { + suite.tempServer.Close() + suite.noRedirectHTTPClient.CloseIdleConnections() +} + +func (suite *redirectorTestSuite) TestReverseProxy() { + redirectorServer := httptest.NewServer(http.HandlerFunc(suite.redirector.ReverseProxy)) + defer redirectorServer.Close() + + suite.redirector.SetAddress(suite.tempServer.URL) + // Test normal forwarding + req, err := http.NewRequest("GET", redirectorServer.URL, nil) + suite.NoError(err) + checkHTTPRequest(suite.Require(), suite.noRedirectHTTPClient, req, http.StatusOK, suite.tempText) + // Test the requests that are forwarded by others + req, err = http.NewRequest("GET", redirectorServer.URL, nil) + suite.NoError(err) + req.Header.Set(proxyHeader, "other") + checkHTTPRequest(suite.Require(), suite.noRedirectHTTPClient, req, http.StatusOK, suite.tempText) + // Test LoopDetected + suite.redirector.SetAddress(redirectorServer.URL) + req, err = http.NewRequest("GET", redirectorServer.URL, nil) + suite.NoError(err) + checkHTTPRequest(suite.Require(), suite.noRedirectHTTPClient, req, http.StatusLoopDetected, "") +} + +func (suite *redirectorTestSuite) TestTemporaryRedirect() { + redirectorServer := httptest.NewServer(http.HandlerFunc(suite.redirector.TemporaryRedirect)) + defer redirectorServer.Close() + suite.redirector.SetAddress(suite.tempServer.URL) + // Test TemporaryRedirect + req, err := http.NewRequest("GET", redirectorServer.URL, nil) + suite.NoError(err) + checkHTTPRequest(suite.Require(), suite.noRedirectHTTPClient, req, http.StatusTemporaryRedirect, "") + // Test Response + req, err = http.NewRequest("GET", redirectorServer.URL, nil) + suite.NoError(err) + checkHTTPRequest(suite.Require(), http.DefaultClient, req, http.StatusOK, suite.tempText) +} + +func checkHTTPRequest(re *require.Assertions, client *http.Client, req *http.Request, expectedCode int, expectedText string) { + resp, err := client.Do(req) + re.NoError(err) + defer resp.Body.Close() + re.Equal(expectedCode, resp.StatusCode) + if expectedCode >= http.StatusOK && expectedCode <= http.StatusAlreadyReported { + text, err := io.ReadAll(resp.Body) + re.NoError(err) + re.Equal(expectedText, string(text)) + } +} From 97d140b549b7e5e464e3fdba1b202853cf348d21 Mon Sep 17 00:00:00 2001 From: HunDunDM Date: Mon, 18 Jul 2022 16:39:51 +0800 Subject: [PATCH 2/4] dashboard: fix a bug that could not handle proxy requests correctly Signed-off-by: HunDunDM --- pkg/dashboard/adapter/redirector.go | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-) diff --git a/pkg/dashboard/adapter/redirector.go b/pkg/dashboard/adapter/redirector.go index d070ec68de4..d32e59b587f 100644 --- a/pkg/dashboard/adapter/redirector.go +++ b/pkg/dashboard/adapter/redirector.go @@ -76,7 +76,7 @@ func (h *Redirector) SetAddress(addr string) { defaultDirector := h.proxy.Director h.proxy.Director = func(r *http.Request) { defaultDirector(r) - r.Header.Set(proxyHeader, h.name) + r.Header.Add(proxyHeader, h.name) } if h.tlsConfig != nil { @@ -118,9 +118,12 @@ func (h *Redirector) ReverseProxy(w http.ResponseWriter, r *http.Request) { return } - if len(r.Header.Get(proxyHeader)) > 0 { - w.WriteHeader(http.StatusLoopDetected) - return + proxySources := r.Header.Values(proxyHeader) + for _, proxySource := range proxySources { + if proxySource == h.name { + w.WriteHeader(http.StatusLoopDetected) + return + } } proxy.ServeHTTP(w, r) From 28dd7fe128140dfb6cd93a3d7c3320a216d09309 Mon Sep 17 00:00:00 2001 From: HunDunDM Date: Tue, 19 Jul 2022 02:52:57 +0800 Subject: [PATCH 3/4] address comment Signed-off-by: HunDunDM --- pkg/dashboard/adapter/redirector_test.go | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/pkg/dashboard/adapter/redirector_test.go b/pkg/dashboard/adapter/redirector_test.go index 2b4ac78b17e..f192fbeb1f2 100644 --- a/pkg/dashboard/adapter/redirector_test.go +++ b/pkg/dashboard/adapter/redirector_test.go @@ -70,17 +70,17 @@ func (suite *redirectorTestSuite) TestReverseProxy() { suite.redirector.SetAddress(suite.tempServer.URL) // Test normal forwarding - req, err := http.NewRequest("GET", redirectorServer.URL, nil) + req, err := http.NewRequest(http.MethodGet, redirectorServer.URL, nil) suite.NoError(err) checkHTTPRequest(suite.Require(), suite.noRedirectHTTPClient, req, http.StatusOK, suite.tempText) // Test the requests that are forwarded by others - req, err = http.NewRequest("GET", redirectorServer.URL, nil) + req, err = http.NewRequest(http.MethodGet, redirectorServer.URL, nil) suite.NoError(err) req.Header.Set(proxyHeader, "other") checkHTTPRequest(suite.Require(), suite.noRedirectHTTPClient, req, http.StatusOK, suite.tempText) // Test LoopDetected suite.redirector.SetAddress(redirectorServer.URL) - req, err = http.NewRequest("GET", redirectorServer.URL, nil) + req, err = http.NewRequest(http.MethodGet, redirectorServer.URL, nil) suite.NoError(err) checkHTTPRequest(suite.Require(), suite.noRedirectHTTPClient, req, http.StatusLoopDetected, "") } @@ -90,11 +90,11 @@ func (suite *redirectorTestSuite) TestTemporaryRedirect() { defer redirectorServer.Close() suite.redirector.SetAddress(suite.tempServer.URL) // Test TemporaryRedirect - req, err := http.NewRequest("GET", redirectorServer.URL, nil) + req, err := http.NewRequest(http.MethodGet, redirectorServer.URL, nil) suite.NoError(err) checkHTTPRequest(suite.Require(), suite.noRedirectHTTPClient, req, http.StatusTemporaryRedirect, "") // Test Response - req, err = http.NewRequest("GET", redirectorServer.URL, nil) + req, err = http.NewRequest(http.MethodGet, redirectorServer.URL, nil) suite.NoError(err) checkHTTPRequest(suite.Require(), http.DefaultClient, req, http.StatusOK, suite.tempText) } From 67e9cf92ac9ce3f002b9e5d2c15a0043fb1f013f Mon Sep 17 00:00:00 2001 From: nolouch Date: Tue, 20 Sep 2022 15:28:31 +0800 Subject: [PATCH 4/4] fix test Signed-off-by: nolouch --- go.mod | 1 + 1 file changed, 1 insertion(+) diff --git a/go.mod b/go.mod index fbdf1a4d218..bcf025df8ac 100644 --- a/go.mod +++ b/go.mod @@ -40,6 +40,7 @@ require ( github.com/sirupsen/logrus v1.2.0 github.com/spf13/cobra v1.0.0 github.com/spf13/pflag v1.0.5 + github.com/stretchr/testify v1.7.0 github.com/swaggo/http-swagger v0.0.0-20200308142732-58ac5e232fba github.com/swaggo/swag v1.6.6-0.20200529100950-7c765ddd0476 github.com/syndtr/goleveldb v1.0.1-0.20190318030020-c3a204f8e965