From 9f960e038189e243b1a589ecfac5a45437a7f05c Mon Sep 17 00:00:00 2001 From: Juan Calderon-Perez Date: Mon, 16 Jan 2023 13:46:56 -0800 Subject: [PATCH 1/2] Fix panic when default config is empty. Add unit-test for filter, regex and default configs Signed-off-by: Juan Calderon-Perez --- main.go | 8 +- main_test.go | 222 ++++++++++++++++++++++++++++++++++++++++++++++----- 2 files changed, 207 insertions(+), 23 deletions(-) diff --git a/main.go b/main.go index b602275..4c15614 100644 --- a/main.go +++ b/main.go @@ -37,20 +37,26 @@ type Config struct { func New(config ...Config) fiber.Handler { // Init config var cfg Config + if len(config) > 0 { cfg = config[0] + } else { + cfg = Config{} } + if cfg.StatusCode == 0 { cfg.StatusCode = 302 // Temporary Redirect } - cfg = config[0] + cfg.rulesRegex = map[*regexp.Regexp]string{} + // Initialize for k, v := range cfg.Rules { k = strings.Replace(k, "*", "(.*)", -1) k = k + "$" cfg.rulesRegex[regexp.MustCompile(k)] = v } + // Middleware function return func(c *fiber.Ctx) error { // Filter request to skip middleware diff --git a/main_test.go b/main_test.go index 6f7cbfc..f9f3bd8 100644 --- a/main_test.go +++ b/main_test.go @@ -9,6 +9,7 @@ import ( "testing" "github.com/gofiber/fiber/v2" + "github.com/gofiber/fiber/v2/utils" ) func Test_Redirect(t *testing.T) { @@ -18,32 +19,32 @@ func Test_Redirect(t *testing.T) { Rules: map[string]string{ "/default": "google.com", }, - StatusCode: 301, + StatusCode: StatusMovedPermanently, })) app.Use(New(Config{ Rules: map[string]string{ "/default/*": "fiber.wiki", }, - StatusCode: 307, + StatusCode: StatusTemporaryRedirect, })) app.Use(New(Config{ Rules: map[string]string{ "/redirect/*": "$1", }, - StatusCode: 303, + StatusCode: StatusSeeOther, })) app.Use(New(Config{ Rules: map[string]string{ "/pattern/*": "golang.org", }, - StatusCode: 302, + StatusCode: StatusFound, })) app.Use(New(Config{ Rules: map[string]string{ "/": "/swagger", }, - StatusCode: 301, + StatusCode: StatusMovedPermanently, })) app.Get("/api/*", func(c *fiber.Ctx) error { @@ -61,49 +62,49 @@ func Test_Redirect(t *testing.T) { statusCode int }{ { - name: "should be returns status 302 without a wildcard", + name: "should be returns status StatusFound without a wildcard", url: "/default", redirectTo: "google.com", - statusCode: 301, + statusCode: StatusMovedPermanently, }, { - name: "should be returns status 307 using wildcard", + name: "should be returns status StatusTemporaryRedirect using wildcard", url: "/default/xyz", redirectTo: "fiber.wiki", - statusCode: 307, + statusCode: StatusTemporaryRedirect, }, { - name: "should be returns status 303 without set redirectTo to use the default", + name: "should be returns status StatusSeeOther without set redirectTo to use the default", url: "/redirect/github.com/gofiber/redirect", redirectTo: "github.com/gofiber/redirect", - statusCode: 303, + statusCode: StatusSeeOther, }, { name: "should return the status code default", url: "/pattern/xyz", redirectTo: "golang.org", - statusCode: 302, + statusCode: StatusFound, }, { name: "access URL without rule", url: "/new", - statusCode: 200, + statusCode: StatusOK, }, { name: "redirect to swagger route", url: "/", redirectTo: "/swagger", - statusCode: 301, + statusCode: StatusMovedPermanently, }, { name: "no redirect to swagger route", url: "/api/", - statusCode: 200, + statusCode: StatusOK, }, { name: "no redirect to swagger route #2", url: "/api/test", - statusCode: 200, + statusCode: StatusOK, }, } for _, tt := range tests { @@ -111,16 +112,193 @@ func Test_Redirect(t *testing.T) { req, _ := http.NewRequest("GET", tt.url, nil) req.Header.Set("Location", "github.com/gofiber/redirect") resp, err := app.Test(req) + if err != nil { t.Fatalf(`%s: %s`, t.Name(), err) } - if resp.StatusCode != tt.statusCode { - t.Fatalf(`%s: StatusCode: got %v - expected %v`, t.Name(), resp.StatusCode, tt.statusCode) - } - if resp.Header.Get("Location") != tt.redirectTo { - t.Fatalf(`%s: Expecting Location: %s`, t.Name(), tt.redirectTo) - } + + utils.AssertEqual(t, tt.statusCode, resp.StatusCode) + utils.AssertEqual(t, tt.redirectTo, resp.Header.Get("Location")) }) } +} + +func Test_Filter(t *testing.T) { + //Case 1 : filter function always returns true + app := *fiber.New() + app.Use(New(Config{ + Filter: func(*fiber.Ctx) bool { + return true + }, + Rules: map[string]string{ + "/default": "google.com", + }, + StatusCode: fiber.StatusMovedPermanently, + })) + + app.Use(func(c *fiber.Ctx) error { + return c.SendStatus(fiber.StatusOK) + }) + + req, _ := http.NewRequest("GET", "/default", nil) + resp, err := app.Test(req) + + if err != nil { + t.Error(err) + } + + utils.AssertEqual(t, fiber.StatusOK, resp.StatusCode) + + //Case 2 : filter function always returns false + app = *fiber.New() + app.Use(New(Config{ + Filter: func(*fiber.Ctx) bool { + return false + }, + Rules: map[string]string{ + "/default": "google.com", + }, + StatusCode: fiber.StatusMovedPermanently, + })) + + req, _ = http.NewRequest("GET", "/default", nil) + resp, err = app.Test(req) + + if err != nil { + t.Error(err) + } + + utils.AssertEqual(t, fiber.StatusMovedPermanently, resp.StatusCode) + utils.AssertEqual(t, "google.com", resp.Header.Get("Location")) +} + +func Test_NoRules(t *testing.T) { + // Case 1: No rules with default route defined + app := *fiber.New() + + app.Use(New(Config{ + StatusCode: fiber.StatusMovedPermanently, + })) + + app.Use(func(c *fiber.Ctx) error { + return c.SendStatus(fiber.StatusOK) + }) + + req, _ := http.NewRequest("GET", "/default", nil) + resp, err := app.Test(req) + + if err != nil { + t.Error(err) + } + + utils.AssertEqual(t, fiber.StatusOK, resp.StatusCode) + + // Case 2: No rules and no default route defined + app = *fiber.New() + + app.Use(New(Config{ + StatusCode: fiber.StatusMovedPermanently, + })) + + req, _ = http.NewRequest("GET", "/default", nil) + resp, err = app.Test(req) + + if err != nil { + t.Error(err) + } + + utils.AssertEqual(t, fiber.StatusNotFound, resp.StatusCode) +} + +func Test_DefaultConfig(t *testing.T) { + // Case 1: Default config and no default route + app := *fiber.New() + + app.Use(New()) + + req, _ := http.NewRequest("GET", "/default", nil) + resp, err := app.Test(req) + + if err != nil { + t.Error(err) + } + + utils.AssertEqual(t, fiber.StatusNotFound, resp.StatusCode) + + // Case 2: Default config and default route + app = *fiber.New() + + app.Use(New()) + app.Use(func(c *fiber.Ctx) error { + return c.SendStatus(fiber.StatusOK) + }) + + req, _ = http.NewRequest("GET", "/default", nil) + resp, err = app.Test(req) + + if err != nil { + t.Error(err) + } + + utils.AssertEqual(t, fiber.StatusOK, resp.StatusCode) +} + +func Test_RegexRules(t *testing.T) { + // Case 1: Rules regex is empty + app := *fiber.New() + app.Use(New(Config{ + Rules: map[string]string{}, + StatusCode: StatusMovedPermanently, + })) + + app.Use(func(c *fiber.Ctx) error { + return c.SendStatus(fiber.StatusOK) + }) + + req, _ := http.NewRequest("GET", "/default", nil) + resp, err := app.Test(req) + + if err != nil { + t.Error(err) + } + utils.AssertEqual(t, fiber.StatusOK, resp.StatusCode) + + // Case 2: Rules regex map contains valid regex and well-formed replacement URLs + app = *fiber.New() + app.Use(New(Config{ + Rules: map[string]string{ + "/default": "google.com", + }, + StatusCode: StatusMovedPermanently, + })) + app.Use(func(c *fiber.Ctx) error { + return c.SendStatus(fiber.StatusOK) + }) + + req, _ = http.NewRequest("GET", "/default", nil) + resp, err = app.Test(req) + + if err != nil { + t.Error(err) + } + + utils.AssertEqual(t, fiber.StatusMovedPermanently, resp.StatusCode) + utils.AssertEqual(t, "google.com", resp.Header.Get("Location")) + + // Case 3: Test invalid regex throws panic + defer func() { + if r := recover(); r != nil { + t.Log("Recovered from invalid regex: ", r) + } + }() + + app = *fiber.New() + app.Use(New(Config{ + Rules: map[string]string{ + "(": "google.com", + }, + StatusCode: StatusMovedPermanently, + })) + t.Error("Expected panic, got nil") } From 15f75cc6d8b7f25ba3f560ddc138abb6bcddc535 Mon Sep 17 00:00:00 2001 From: Juan Calderon-Perez Date: Mon, 16 Jan 2023 13:53:49 -0800 Subject: [PATCH 2/2] Fix syntax issue. Add go 1.19.x to test suite. Signed-off-by: Juan Calderon-Perez --- .github/workflows/test.yml | 1 + main_test.go | 32 ++++++++++++++++---------------- 2 files changed, 17 insertions(+), 16 deletions(-) diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml index be87d45..edc9827 100644 --- a/.github/workflows/test.yml +++ b/.github/workflows/test.yml @@ -13,6 +13,7 @@ jobs: go-version: - 1.16.x - 1.18.x + - 1.19.x platform: - ubuntu-latest - windows-latest diff --git a/main_test.go b/main_test.go index f9f3bd8..c59a1d4 100644 --- a/main_test.go +++ b/main_test.go @@ -19,32 +19,32 @@ func Test_Redirect(t *testing.T) { Rules: map[string]string{ "/default": "google.com", }, - StatusCode: StatusMovedPermanently, + StatusCode: fiber.StatusMovedPermanently, })) app.Use(New(Config{ Rules: map[string]string{ "/default/*": "fiber.wiki", }, - StatusCode: StatusTemporaryRedirect, + StatusCode: fiber.StatusTemporaryRedirect, })) app.Use(New(Config{ Rules: map[string]string{ "/redirect/*": "$1", }, - StatusCode: StatusSeeOther, + StatusCode: fiber.StatusSeeOther, })) app.Use(New(Config{ Rules: map[string]string{ "/pattern/*": "golang.org", }, - StatusCode: StatusFound, + StatusCode: fiber.StatusFound, })) app.Use(New(Config{ Rules: map[string]string{ "/": "/swagger", }, - StatusCode: StatusMovedPermanently, + StatusCode: fiber.StatusMovedPermanently, })) app.Get("/api/*", func(c *fiber.Ctx) error { @@ -65,46 +65,46 @@ func Test_Redirect(t *testing.T) { name: "should be returns status StatusFound without a wildcard", url: "/default", redirectTo: "google.com", - statusCode: StatusMovedPermanently, + statusCode: fiber.StatusMovedPermanently, }, { name: "should be returns status StatusTemporaryRedirect using wildcard", url: "/default/xyz", redirectTo: "fiber.wiki", - statusCode: StatusTemporaryRedirect, + statusCode: fiber.StatusTemporaryRedirect, }, { name: "should be returns status StatusSeeOther without set redirectTo to use the default", url: "/redirect/github.com/gofiber/redirect", redirectTo: "github.com/gofiber/redirect", - statusCode: StatusSeeOther, + statusCode: fiber.StatusSeeOther, }, { name: "should return the status code default", url: "/pattern/xyz", redirectTo: "golang.org", - statusCode: StatusFound, + statusCode: fiber.StatusFound, }, { name: "access URL without rule", url: "/new", - statusCode: StatusOK, + statusCode: fiber.StatusOK, }, { name: "redirect to swagger route", url: "/", redirectTo: "/swagger", - statusCode: StatusMovedPermanently, + statusCode: fiber.StatusMovedPermanently, }, { name: "no redirect to swagger route", url: "/api/", - statusCode: StatusOK, + statusCode: fiber.StatusOK, }, { name: "no redirect to swagger route #2", url: "/api/test", - statusCode: StatusOK, + statusCode: fiber.StatusOK, }, } for _, tt := range tests { @@ -248,7 +248,7 @@ func Test_RegexRules(t *testing.T) { app := *fiber.New() app.Use(New(Config{ Rules: map[string]string{}, - StatusCode: StatusMovedPermanently, + StatusCode: fiber.StatusMovedPermanently, })) app.Use(func(c *fiber.Ctx) error { @@ -269,7 +269,7 @@ func Test_RegexRules(t *testing.T) { Rules: map[string]string{ "/default": "google.com", }, - StatusCode: StatusMovedPermanently, + StatusCode: fiber.StatusMovedPermanently, })) app.Use(func(c *fiber.Ctx) error { @@ -298,7 +298,7 @@ func Test_RegexRules(t *testing.T) { Rules: map[string]string{ "(": "google.com", }, - StatusCode: StatusMovedPermanently, + StatusCode: fiber.StatusMovedPermanently, })) t.Error("Expected panic, got nil") }