From 6152f310cc27a649b5eda83e521179d653bd9cb7 Mon Sep 17 00:00:00 2001 From: Juan Calderon-Perez Date: Fri, 20 Dec 2024 21:00:35 -0500 Subject: [PATCH 01/11] Initial improvements --- router.go | 123 ++++++++++++++++++++++++------------------------- router_test.go | 23 +++++++++ 2 files changed, 83 insertions(+), 63 deletions(-) diff --git a/router.go b/router.go index 2091cfc6cb..246f6cba88 100644 --- a/router.go +++ b/router.go @@ -5,11 +5,11 @@ package fiber import ( + "bytes" "errors" "fmt" "html" "sort" - "strings" "sync/atomic" "github.com/gofiber/utils/v2" @@ -65,10 +65,12 @@ type Route struct { func (r *Route) match(detectionPath, path string, params *[maxParams]string) bool { // root detectionPath check - if r.root && detectionPath == "/" { + if r.root && len(detectionPath) == 1 && detectionPath[0] == '/' { return true - // '*' wildcard matches any detectionPath - } else if r.star { + } + + // '*' wildcard matches any detectionPath + if r.star { if len(path) > 1 { params[0] = path[1:] } else { @@ -76,24 +78,35 @@ func (r *Route) match(detectionPath, path string, params *[maxParams]string) boo } return true } - // Does this route have parameters + + // Does this route have parameters? if len(r.Params) > 0 { - // Match params - if match := r.routeParser.getMatch(detectionPath, path, params, r.use); match { - // Get params from the path detectionPath - return match + // Match params using precomputed routeParser + if r.routeParser.getMatch(detectionPath, path, params, r.use) { + return true } } - // Is this route a Middleware? + + // Middleware route? if r.use { - // Single slash will match or detectionPath prefix - if r.root || strings.HasPrefix(detectionPath, r.path) { + // Single slash or prefix match + plen := len(r.path) + if r.root { + // If r.root is '/', it matches everything starting at '/' + // Actually, if it's a middleware root, it should match always at '/' + if len(detectionPath) > 0 && detectionPath[0] == '/' { + return true + } + } else if len(detectionPath) >= plen && detectionPath[:plen] == r.path { + return true + } + } else { + // Check exact match + if len(r.path) == len(detectionPath) && detectionPath == r.path { return true } - // Check for a simple detectionPath match - } else if len(r.path) == len(detectionPath) && r.path == detectionPath { - return true } + // No match return false } @@ -202,7 +215,7 @@ func (app *App) next(c *DefaultCtx) (bool, error) { } func (app *App) requestHandler(rctx *fasthttp.RequestCtx) { - // Handler for default ctxs + // Acquire context var c CustomCtx var ok bool if app.newCtxFunc != nil { @@ -224,8 +237,9 @@ func (app *App) requestHandler(rctx *fasthttp.RequestCtx) { return } - // check flash messages - if strings.Contains(utils.UnsafeString(c.Request().Header.RawHeaders()), FlashCookieName) { + // Efficient flash cookie check using bytes.Index + rawHeaders := c.Request().Header.RawHeaders() + if len(rawHeaders) > 0 && bytes.Index(rawHeaders, []byte(FlashCookieName)) != -1 { c.Redirect().parseAndClearFlashMessages() } @@ -236,6 +250,7 @@ func (app *App) requestHandler(rctx *fasthttp.RequestCtx) { } else { _, err = app.next(c.(*DefaultCtx)) //nolint:errcheck // It is fine to ignore the error here } + if err != nil { if catch := c.App().ErrorHandler(c, err); catch != nil { _ = c.SendStatus(StatusInternalServerError) //nolint:errcheck // It is fine to ignore the error here @@ -295,81 +310,63 @@ func (app *App) register(methods []string, pathRaw string, group *Group, handler handlers = append(handlers, handler) } + // Precompute path normalization ONCE + if pathRaw == "" { + pathRaw = "/" + } + if pathRaw[0] != '/' { + pathRaw = "/" + pathRaw + } + pathPretty := pathRaw + if !app.config.CaseSensitive { + pathPretty = utils.ToLower(pathPretty) + } + if !app.config.StrictRouting && len(pathPretty) > 1 { + pathPretty = utils.TrimRight(pathPretty, '/') + } + pathClean := RemoveEscapeChar(pathPretty) + + parsedRaw := parseRoute(pathRaw, app.customConstraints...) + parsedPretty := parseRoute(pathPretty, app.customConstraints...) + for _, method := range methods { - // Uppercase HTTP methods method = utils.ToUpper(method) - // Check if the HTTP method is valid unless it's USE if method != methodUse && app.methodInt(method) == -1 { panic(fmt.Sprintf("add: invalid http method %s\n", method)) } - // is mounted app + isMount := group != nil && group.app != app - // A route requires atleast one ctx handler if len(handlers) == 0 && !isMount { panic(fmt.Sprintf("missing handler/middleware in route: %s\n", pathRaw)) } - // Cannot have an empty path - if pathRaw == "" { - pathRaw = "/" - } - // Path always start with a '/' - if pathRaw[0] != '/' { - pathRaw = "/" + pathRaw - } - // Create a stripped path in case-sensitive / trailing slashes - pathPretty := pathRaw - // Case-sensitive routing, all to lowercase - if !app.config.CaseSensitive { - pathPretty = utils.ToLower(pathPretty) - } - // Strict routing, remove trailing slashes - if !app.config.StrictRouting && len(pathPretty) > 1 { - pathPretty = utils.TrimRight(pathPretty, '/') - } - // Is layer a middleware? + isUse := method == methodUse - // Is path a direct wildcard? - isStar := pathPretty == "/*" - // Is path a root slash? - isRoot := pathPretty == "/" - // Parse path parameters - parsedRaw := parseRoute(pathRaw, app.customConstraints...) - parsedPretty := parseRoute(pathPretty, app.customConstraints...) - - // Create route metadata without pointer + isStar := pathClean == "/*" + isRoot := pathClean == "/" + route := Route{ - // Router booleans use: isUse, mount: isMount, star: isStar, root: isRoot, - // Path data - path: RemoveEscapeChar(pathPretty), + path: pathClean, routeParser: parsedPretty, Params: parsedRaw.params, + group: group, - // Group data - group: group, - - // Public data Path: pathRaw, Method: method, Handlers: handlers, } - // Increment global handler count - atomic.AddUint32(&app.handlersCount, uint32(len(handlers))) //nolint:gosec // Not a concern - // Middleware route matches all HTTP methods + atomic.AddUint32(&app.handlersCount, uint32(len(handlers))) if isUse { - // Add route to all HTTP methods stack for _, m := range app.config.RequestMethods { - // Create a route copy to avoid duplicates during compression r := route app.addRoute(m, &r, isMount) } } else { - // Add route to stack app.addRoute(method, &route, isMount) } } diff --git a/router_test.go b/router_test.go index 5509039c66..2e42505691 100644 --- a/router_test.go +++ b/router_test.go @@ -591,6 +591,29 @@ func Benchmark_Router_Next_Default(b *testing.B) { } } +// go test -benchmem -run=^$ -bench ^Benchmark_Router_Next_Default_Parallel$ github.com/gofiber/fiber/v3 -count=1 +func Benchmark_Router_Next_Default_Parallel(b *testing.B) { + app := New() + app.Get("/", func(_ Ctx) error { + return nil + }) + + h := app.Handler() + + b.ReportAllocs() + b.ResetTimer() + + b.RunParallel(func(pb *testing.PB) { + for pb.Next() { + fctx := &fasthttp.RequestCtx{} + fctx.Request.Header.SetMethod(MethodGet) + fctx.Request.SetRequestURI("/") + + h(fctx) + } + }) +} + // go test -v ./... -run=^$ -bench=Benchmark_Route_Match -benchmem -count=4 func Benchmark_Route_Match(b *testing.B) { var match bool From 6e1836e7e23e4ade5fa5e127f2a757590ee8e953 Mon Sep 17 00:00:00 2001 From: Juan Calderon-Perez Date: Fri, 20 Dec 2024 22:49:13 -0500 Subject: [PATCH 02/11] Update test --- router_test.go | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/router_test.go b/router_test.go index 2e42505691..fe5b3429e0 100644 --- a/router_test.go +++ b/router_test.go @@ -604,11 +604,11 @@ func Benchmark_Router_Next_Default_Parallel(b *testing.B) { b.ResetTimer() b.RunParallel(func(pb *testing.PB) { - for pb.Next() { - fctx := &fasthttp.RequestCtx{} - fctx.Request.Header.SetMethod(MethodGet) - fctx.Request.SetRequestURI("/") + fctx := &fasthttp.RequestCtx{} + fctx.Request.Header.SetMethod(MethodGet) + fctx.Request.SetRequestURI("/") + for pb.Next() { h(fctx) } }) From 23e90c029e2ece48c6f33d47516e8f6138ebba7c Mon Sep 17 00:00:00 2001 From: Juan Calderon-Perez Date: Sat, 21 Dec 2024 15:10:22 -0500 Subject: [PATCH 03/11] Improve RemoveEscapeChar performance --- path.go | 12 +++++++++--- 1 file changed, 9 insertions(+), 3 deletions(-) diff --git a/path.go b/path.go index 00105d5cc0..282073ec04 100644 --- a/path.go +++ b/path.go @@ -620,10 +620,16 @@ func GetTrimmedParam(param string) string { // RemoveEscapeChar remove escape characters func RemoveEscapeChar(word string) string { - if strings.IndexByte(word, escapeChar) != -1 { - return strings.ReplaceAll(word, string(escapeChar), "") + b := []byte(word) + dst := 0 + for src := 0; src < len(b); src++ { + if b[src] == '\\' { + continue + } + b[dst] = b[src] + dst++ } - return word + return string(b[:dst]) } func getParamConstraintType(constraintPart string) TypeConstraint { From fe894251fbbf8caa14b683a320b9dd41497d4bea Mon Sep 17 00:00:00 2001 From: Juan Calderon-Perez Date: Mon, 23 Dec 2024 23:38:20 -0500 Subject: [PATCH 04/11] Fix lint issues --- .github/workflows/linter.yml | 2 +- Makefile | 2 +- router.go | 10 ++++------ 3 files changed, 6 insertions(+), 8 deletions(-) diff --git a/.github/workflows/linter.yml b/.github/workflows/linter.yml index bd2c0bce4c..beed212610 100644 --- a/.github/workflows/linter.yml +++ b/.github/workflows/linter.yml @@ -37,4 +37,4 @@ jobs: uses: golangci/golangci-lint-action@v6 with: # NOTE: Keep this in sync with the version from .golangci.yml - version: v1.62.0 + version: v1.62.2 diff --git a/Makefile b/Makefile index 4b348cd574..669b3fbee4 100644 --- a/Makefile +++ b/Makefile @@ -35,7 +35,7 @@ markdown: ## lint: 🚨 Run lint checks .PHONY: lint lint: - go run github.com/golangci/golangci-lint/cmd/golangci-lint@v1.62.0 run ./... + go run github.com/golangci/golangci-lint/cmd/golangci-lint@v1.62.2 run ./... ## test: 🚦 Execute all tests .PHONY: test diff --git a/router.go b/router.go index 246f6cba88..630ece8d34 100644 --- a/router.go +++ b/router.go @@ -100,11 +100,9 @@ func (r *Route) match(detectionPath, path string, params *[maxParams]string) boo } else if len(detectionPath) >= plen && detectionPath[:plen] == r.path { return true } - } else { + } else if len(r.path) == len(detectionPath) && detectionPath == r.path { // Check exact match - if len(r.path) == len(detectionPath) && detectionPath == r.path { - return true - } + return true } // No match @@ -239,7 +237,7 @@ func (app *App) requestHandler(rctx *fasthttp.RequestCtx) { // Efficient flash cookie check using bytes.Index rawHeaders := c.Request().Header.RawHeaders() - if len(rawHeaders) > 0 && bytes.Index(rawHeaders, []byte(FlashCookieName)) != -1 { + if len(rawHeaders) > 0 && bytes.Contains(rawHeaders, []byte(FlashCookieName)) { c.Redirect().parseAndClearFlashMessages() } @@ -360,7 +358,7 @@ func (app *App) register(methods []string, pathRaw string, group *Group, handler Handlers: handlers, } - atomic.AddUint32(&app.handlersCount, uint32(len(handlers))) + atomic.AddUint32(&app.handlersCount, uint32(len(handlers))) //nolint:gosec // Not a concern if isUse { for _, m := range app.config.RequestMethods { r := route From 0040507a2129d65beafe4dc788bf7a2d77654f32 Mon Sep 17 00:00:00 2001 From: Juan Calderon-Perez Date: Wed, 25 Dec 2024 21:38:15 -0500 Subject: [PATCH 05/11] Re-add comments --- router.go | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/router.go b/router.go index 630ece8d34..9b2f09e7b3 100644 --- a/router.go +++ b/router.go @@ -93,7 +93,6 @@ func (r *Route) match(detectionPath, path string, params *[maxParams]string) boo plen := len(r.path) if r.root { // If r.root is '/', it matches everything starting at '/' - // Actually, if it's a middleware root, it should match always at '/' if len(detectionPath) > 0 && detectionPath[0] == '/' { return true } @@ -235,7 +234,7 @@ func (app *App) requestHandler(rctx *fasthttp.RequestCtx) { return } - // Efficient flash cookie check using bytes.Index + // check flash messages rawHeaders := c.Request().Header.RawHeaders() if len(rawHeaders) > 0 && bytes.Contains(rawHeaders, []byte(FlashCookieName)) { c.Redirect().parseAndClearFlashMessages() @@ -358,13 +357,19 @@ func (app *App) register(methods []string, pathRaw string, group *Group, handler Handlers: handlers, } + // Increment global handler count atomic.AddUint32(&app.handlersCount, uint32(len(handlers))) //nolint:gosec // Not a concern + + // Middleware route matches all HTTP methods if isUse { + // Add route to all HTTP methods stack for _, m := range app.config.RequestMethods { + // Create a route copy to avoid duplicates during compression r := route app.addRoute(m, &r, isMount) } } else { + // Add route to stack app.addRoute(method, &route, isMount) } } From 97de5d3ba1007133f09eb03ef6ed834ecea28d83 Mon Sep 17 00:00:00 2001 From: Juan Calderon-Perez Date: Fri, 27 Dec 2024 21:28:46 -0500 Subject: [PATCH 06/11] Add dedicated request handlers --- app.go | 16 ++++++++-- ctx_interface_gen.go | 3 ++ router.go | 71 +++++++++++++++++++++++++++----------------- 3 files changed, 61 insertions(+), 29 deletions(-) diff --git a/app.go b/app.go index 5e5475b5f1..7026240082 100644 --- a/app.go +++ b/app.go @@ -616,6 +616,10 @@ func (app *App) handleTrustedProxy(ipAddress string) { // Note: It doesn't allow adding new methods, only customizing exist methods. func (app *App) NewCtxFunc(function func(app *App) CustomCtx) { app.newCtxFunc = function + + if app.server != nil { + app.server.Handler = app.customRequestHandler + } } // RegisterCustomConstraint allows to register custom constraint. @@ -868,7 +872,11 @@ func (app *App) Config() Config { func (app *App) Handler() fasthttp.RequestHandler { //revive:disable-line:confusing-naming // Having both a Handler() (uppercase) and a handler() (lowercase) is fine. TODO: Use nolint:revive directive instead. See https://github.com/golangci/golangci-lint/issues/3476 // prepare the server for the start app.startupProcess() - return app.requestHandler + if app.newCtxFunc != nil { + return app.customRequestHandler + } else { + return app.defaultRequestHandler + } } // Stack returns the raw router stack. @@ -1057,7 +1065,11 @@ func (app *App) init() *App { } // fasthttp server settings - app.server.Handler = app.requestHandler + if app.newCtxFunc != nil { + app.server.Handler = app.customRequestHandler + } else { + app.server.Handler = app.defaultRequestHandler + } app.server.Name = app.config.ServerHeader app.server.Concurrency = app.config.Concurrency app.server.NoDefaultDate = app.config.DisableDefaultDate diff --git a/ctx_interface_gen.go b/ctx_interface_gen.go index d7f8bbc615..cc48576efb 100644 --- a/ctx_interface_gen.go +++ b/ctx_interface_gen.go @@ -350,5 +350,8 @@ type Ctx interface { setIndexRoute(route int) setMatched(matched bool) setRoute(route *Route) + // Drop closes the underlying connection without sending any response headers or body. + // This can be useful for silently terminating client connections, such as in DDoS mitigation + // or when blocking access to sensitive endpoints. Drop() error } diff --git a/router.go b/router.go index 9b2f09e7b3..fa6d1d0930 100644 --- a/router.go +++ b/router.go @@ -6,7 +6,6 @@ package fiber import ( "bytes" - "errors" "fmt" "html" "sort" @@ -211,48 +210,66 @@ func (app *App) next(c *DefaultCtx) (bool, error) { return false, err } -func (app *App) requestHandler(rctx *fasthttp.RequestCtx) { - // Acquire context - var c CustomCtx - var ok bool - if app.newCtxFunc != nil { - c, ok = app.AcquireCtx(rctx).(CustomCtx) - if !ok { - panic(errors.New("requestHandler: failed to type-assert to CustomCtx")) - } - } else { - c, ok = app.AcquireCtx(rctx).(*DefaultCtx) - if !ok { - panic(errors.New("requestHandler: failed to type-assert to *DefaultCtx")) +func (app *App) defaultRequestHandler(rctx *fasthttp.RequestCtx) { + // Acquire DefaultCtx from the pool + ctx, ok := app.AcquireCtx(rctx).(*DefaultCtx) + if !ok { + // Should not happen, but just in case: + panic("defaultRequestHandler: failed to type-assert *DefaultCtx") + } + + defer app.ReleaseCtx(ctx) + + // Check if the HTTP method is valid + // (e.g., methodInt returns -1 if it's not found in app.config.RequestMethods) + if ctx.methodINT == -1 { + _ = ctx.SendStatus(StatusNotImplemented) + return + } + + // Optional: Check flash messages + rawHeaders := ctx.Request().Header.RawHeaders() + if len(rawHeaders) > 0 && bytes.Contains(rawHeaders, []byte(FlashCookieName)) { + ctx.Redirect().parseAndClearFlashMessages() + } + + // Attempt to match a route and execute the chain + _, err := app.next(ctx) + if err != nil { + if catch := ctx.App().ErrorHandler(ctx, err); catch != nil { + _ = ctx.SendStatus(StatusInternalServerError) } } +} + +func (app *App) customRequestHandler(rctx *fasthttp.RequestCtx) { + // Acquire CustomCtx from the pool + c, ok := app.AcquireCtx(rctx).(CustomCtx) + if !ok { + // Should not happen, but just in case: + panic("customRequestHandler: failed to type-assert CustomCtx") + } + defer app.ReleaseCtx(c) - // handle invalid http method directly + // Check if the HTTP method is valid if app.methodInt(c.Method()) == -1 { - _ = c.SendStatus(StatusNotImplemented) //nolint:errcheck // Always return nil + _ = c.SendStatus(StatusNotImplemented) return } - // check flash messages + // Optional: Check flash messages rawHeaders := c.Request().Header.RawHeaders() if len(rawHeaders) > 0 && bytes.Contains(rawHeaders, []byte(FlashCookieName)) { c.Redirect().parseAndClearFlashMessages() } - // Find match in stack - var err error - if app.newCtxFunc != nil { - _, err = app.nextCustom(c) - } else { - _, err = app.next(c.(*DefaultCtx)) //nolint:errcheck // It is fine to ignore the error here - } - + // Attempt to match a route and execute the chain + _, err := app.nextCustom(c) if err != nil { if catch := c.App().ErrorHandler(c, err); catch != nil { - _ = c.SendStatus(StatusInternalServerError) //nolint:errcheck // It is fine to ignore the error here + _ = c.SendStatus(StatusInternalServerError) } - // TODO: Do we need to return here? } } From 8e213e0ac4c6a6a912f7bc4b9d53239976f3c271 Mon Sep 17 00:00:00 2001 From: Juan Calderon-Perez Date: Fri, 27 Dec 2024 21:53:59 -0500 Subject: [PATCH 07/11] Fix lint issues --- app.go | 4 ++-- binder/mapping.go | 4 +--- router.go | 18 +++++++++--------- 3 files changed, 12 insertions(+), 14 deletions(-) diff --git a/app.go b/app.go index 7026240082..3810a8ec0c 100644 --- a/app.go +++ b/app.go @@ -872,11 +872,11 @@ func (app *App) Config() Config { func (app *App) Handler() fasthttp.RequestHandler { //revive:disable-line:confusing-naming // Having both a Handler() (uppercase) and a handler() (lowercase) is fine. TODO: Use nolint:revive directive instead. See https://github.com/golangci/golangci-lint/issues/3476 // prepare the server for the start app.startupProcess() + if app.newCtxFunc != nil { return app.customRequestHandler - } else { - return app.defaultRequestHandler } + return app.defaultRequestHandler } // Stack returns the raw router stack. diff --git a/binder/mapping.go b/binder/mapping.go index d8b692f7e4..29b5550b10 100644 --- a/binder/mapping.go +++ b/binder/mapping.go @@ -107,8 +107,7 @@ func parseToStruct(aliasTag string, out any, data map[string][]string) error { func parseToMap(ptr any, data map[string][]string) error { elem := reflect.TypeOf(ptr).Elem() - //nolint:exhaustive // it's not necessary to check all types - switch elem.Kind() { + switch elem.Kind() { //nolint:exhaustive // it's not necessary to check all types case reflect.Slice: newMap, ok := ptr.(map[string][]string) if !ok { @@ -129,7 +128,6 @@ func parseToMap(ptr any, data map[string][]string) error { newMap[k] = "" continue } - newMap[k] = v[len(v)-1] } } diff --git a/router.go b/router.go index fa6d1d0930..ad8caab597 100644 --- a/router.go +++ b/router.go @@ -6,6 +6,7 @@ package fiber import ( "bytes" + "errors" "fmt" "html" "sort" @@ -214,16 +215,14 @@ func (app *App) defaultRequestHandler(rctx *fasthttp.RequestCtx) { // Acquire DefaultCtx from the pool ctx, ok := app.AcquireCtx(rctx).(*DefaultCtx) if !ok { - // Should not happen, but just in case: - panic("defaultRequestHandler: failed to type-assert *DefaultCtx") + panic(errors.New("requestHandler: failed to type-assert to *DefaultCtx")) } defer app.ReleaseCtx(ctx) // Check if the HTTP method is valid - // (e.g., methodInt returns -1 if it's not found in app.config.RequestMethods) if ctx.methodINT == -1 { - _ = ctx.SendStatus(StatusNotImplemented) + _ = ctx.SendStatus(StatusNotImplemented) //nolint:errcheck // Always return nil return } @@ -237,8 +236,9 @@ func (app *App) defaultRequestHandler(rctx *fasthttp.RequestCtx) { _, err := app.next(ctx) if err != nil { if catch := ctx.App().ErrorHandler(ctx, err); catch != nil { - _ = ctx.SendStatus(StatusInternalServerError) + _ = ctx.SendStatus(StatusInternalServerError) //nolint:errcheck // Always return nil } + // TODO: Do we need to return here? } } @@ -246,15 +246,14 @@ func (app *App) customRequestHandler(rctx *fasthttp.RequestCtx) { // Acquire CustomCtx from the pool c, ok := app.AcquireCtx(rctx).(CustomCtx) if !ok { - // Should not happen, but just in case: - panic("customRequestHandler: failed to type-assert CustomCtx") + panic(errors.New("requestHandler: failed to type-assert to CustomCtx")) } defer app.ReleaseCtx(c) // Check if the HTTP method is valid if app.methodInt(c.Method()) == -1 { - _ = c.SendStatus(StatusNotImplemented) + _ = c.SendStatus(StatusNotImplemented) //nolint:errcheck // Always return nil return } @@ -268,8 +267,9 @@ func (app *App) customRequestHandler(rctx *fasthttp.RequestCtx) { _, err := app.nextCustom(c) if err != nil { if catch := c.App().ErrorHandler(c, err); catch != nil { - _ = c.SendStatus(StatusInternalServerError) + _ = c.SendStatus(StatusInternalServerError) //nolint:errcheck // Always return nil } + // TODO: Do we need to return here? } } From 2e5ef2c9b561210739e562bdc7203d29c3c8d5d0 Mon Sep 17 00:00:00 2001 From: Juan Calderon-Perez Date: Sat, 28 Dec 2024 23:08:29 -0500 Subject: [PATCH 08/11] Add test case for app.All with custom method --- app_test.go | 33 +++++++++++++++++++++++++++++++++ router.go | 18 +++++++++--------- 2 files changed, 42 insertions(+), 9 deletions(-) diff --git a/app_test.go b/app_test.go index a99796a2c1..5fd1a4bd17 100644 --- a/app_test.go +++ b/app_test.go @@ -606,7 +606,40 @@ func Test_App_Add_Method_Test(t *testing.T) { require.NoError(t, err, "app.Test(req)") require.Equal(t, StatusNotImplemented, resp.StatusCode, "Status code") + // Add a new method app.Add([]string{"JANE"}, "/doe", testEmptyHandler) + + resp, err = app.Test(httptest.NewRequest("JANE", "/doe", nil)) + require.NoError(t, err, "app.Test(req)") + require.Equal(t, StatusOK, resp.StatusCode, "Status code") +} + +func Test_App_All_Method_Test(t *testing.T) { + t.Parallel() + defer func() { + if err := recover(); err != nil { + require.Equal(t, "add: invalid http method JANE\n", fmt.Sprintf("%v", err)) + } + }() + + methods := append(DefaultMethods, "JOHN") //nolint:gocritic // We want a new slice here + app := New(Config{ + RequestMethods: methods, + }) + + // Add a new method with All + app.All("/doe", testEmptyHandler) + + resp, err := app.Test(httptest.NewRequest("JOHN", "/doe", nil)) + require.NoError(t, err, "app.Test(req)") + require.Equal(t, StatusOK, resp.StatusCode, "Status code") + + // Add a new method + app.Add([]string{"JANE"}, "/doe", testEmptyHandler) + + resp, err = app.Test(httptest.NewRequest("JANE", "/doe", nil)) + require.NoError(t, err, "app.Test(req)") + require.Equal(t, StatusOK, resp.StatusCode, "Status code") } // go test -run Test_App_GETOnly diff --git a/router.go b/router.go index ad8caab597..9612da170b 100644 --- a/router.go +++ b/router.go @@ -244,30 +244,30 @@ func (app *App) defaultRequestHandler(rctx *fasthttp.RequestCtx) { func (app *App) customRequestHandler(rctx *fasthttp.RequestCtx) { // Acquire CustomCtx from the pool - c, ok := app.AcquireCtx(rctx).(CustomCtx) + ctx, ok := app.AcquireCtx(rctx).(CustomCtx) if !ok { panic(errors.New("requestHandler: failed to type-assert to CustomCtx")) } - defer app.ReleaseCtx(c) + defer app.ReleaseCtx(ctx) // Check if the HTTP method is valid - if app.methodInt(c.Method()) == -1 { - _ = c.SendStatus(StatusNotImplemented) //nolint:errcheck // Always return nil + if app.methodInt(ctx.Method()) == -1 { + _ = ctx.SendStatus(StatusNotImplemented) //nolint:errcheck // Always return nil return } // Optional: Check flash messages - rawHeaders := c.Request().Header.RawHeaders() + rawHeaders := ctx.Request().Header.RawHeaders() if len(rawHeaders) > 0 && bytes.Contains(rawHeaders, []byte(FlashCookieName)) { - c.Redirect().parseAndClearFlashMessages() + ctx.Redirect().parseAndClearFlashMessages() } // Attempt to match a route and execute the chain - _, err := app.nextCustom(c) + _, err := app.nextCustom(ctx) if err != nil { - if catch := c.App().ErrorHandler(c, err); catch != nil { - _ = c.SendStatus(StatusInternalServerError) //nolint:errcheck // Always return nil + if catch := ctx.App().ErrorHandler(ctx, err); catch != nil { + _ = ctx.SendStatus(StatusInternalServerError) //nolint:errcheck // Always return nil } // TODO: Do we need to return here? } From f0e7c895eb0d4f9b9126dd99f662eb53c915a38a Mon Sep 17 00:00:00 2001 From: Juan Calderon-Perez Date: Sun, 29 Dec 2024 01:27:13 -0500 Subject: [PATCH 09/11] Add test for custom Ctx and Request Methods --- app_test.go | 12 ++++++------ ctx_test.go | 36 ++++++++++++++++++++++++++++++++++++ 2 files changed, 42 insertions(+), 6 deletions(-) diff --git a/app_test.go b/app_test.go index 5fd1a4bd17..60f61a9b95 100644 --- a/app_test.go +++ b/app_test.go @@ -592,24 +592,24 @@ func Test_App_Add_Method_Test(t *testing.T) { RequestMethods: methods, }) - app.Add([]string{"JOHN"}, "/doe", testEmptyHandler) + app.Add([]string{"JOHN"}, "/john", testEmptyHandler) - resp, err := app.Test(httptest.NewRequest("JOHN", "/doe", nil)) + resp, err := app.Test(httptest.NewRequest("JOHN", "/john", nil)) require.NoError(t, err, "app.Test(req)") require.Equal(t, StatusOK, resp.StatusCode, "Status code") - resp, err = app.Test(httptest.NewRequest(MethodGet, "/doe", nil)) + resp, err = app.Test(httptest.NewRequest(MethodGet, "/john", nil)) require.NoError(t, err, "app.Test(req)") require.Equal(t, StatusMethodNotAllowed, resp.StatusCode, "Status code") - resp, err = app.Test(httptest.NewRequest("UNKNOWN", "/doe", nil)) + resp, err = app.Test(httptest.NewRequest("UNKNOWN", "/john", nil)) require.NoError(t, err, "app.Test(req)") require.Equal(t, StatusNotImplemented, resp.StatusCode, "Status code") // Add a new method - app.Add([]string{"JANE"}, "/doe", testEmptyHandler) + app.Add([]string{"JANE"}, "/jane", testEmptyHandler) - resp, err = app.Test(httptest.NewRequest("JANE", "/doe", nil)) + resp, err = app.Test(httptest.NewRequest("JANE", "/jane", nil)) require.NoError(t, err, "app.Test(req)") require.Equal(t, StatusOK, resp.StatusCode, "Status code") } diff --git a/ctx_test.go b/ctx_test.go index 88b617eb5b..eea22b1a8d 100644 --- a/ctx_test.go +++ b/ctx_test.go @@ -127,6 +127,42 @@ func Test_Ctx_CustomCtx(t *testing.T) { require.Equal(t, "prefix_v3", string(body)) } +// go test -run Test_Ctx_CustomCtx +func Test_Ctx_CustomCtx_and_Method(t *testing.T) { + t.Parallel() + + defer func() { + if err := recover(); err != nil { + require.Equal(t, "add: invalid http method JANE\n", fmt.Sprintf("%v", err)) + } + }() + + // Create app with custom request methods + methods := append(DefaultMethods, "JOHN") //nolint:gocritic // We want a new slice here + app := New(Config{ + RequestMethods: methods, + }) + + // Create custom context + app.NewCtxFunc(func(app *App) CustomCtx { + return &customCtx{ + DefaultCtx: *NewDefaultCtx(app), + } + }) + + // Add route with custom method + app.Add([]string{"JOHN"}, "/doe", testEmptyHandler) + resp, err := app.Test(httptest.NewRequest("JOHN", "/doe", nil)) + require.NoError(t, err, "app.Test(req)") + require.Equal(t, StatusOK, resp.StatusCode, "Status code") + + // Add a new method + app.Add([]string{"JANE"}, "/doe", testEmptyHandler) + resp, err = app.Test(httptest.NewRequest("JANE", "/doe", nil)) + require.NoError(t, err, "app.Test(req)") + require.Equal(t, StatusOK, resp.StatusCode, "Status code") +} + // go test -run Test_Ctx_Accepts_EmptyAccept func Test_Ctx_Accepts_EmptyAccept(t *testing.T) { t.Parallel() From cd0b631f7885d2fb63e7bbac1b547e3928d38379 Mon Sep 17 00:00:00 2001 From: Juan Calderon-Perez Date: Sun, 29 Dec 2024 01:32:08 -0500 Subject: [PATCH 10/11] Simplify test logic --- app_test.go | 13 +++---------- ctx_test.go | 13 +++---------- 2 files changed, 6 insertions(+), 20 deletions(-) diff --git a/app_test.go b/app_test.go index 60f61a9b95..9b005ced10 100644 --- a/app_test.go +++ b/app_test.go @@ -581,11 +581,6 @@ func Test_App_Use_StrictRouting(t *testing.T) { func Test_App_Add_Method_Test(t *testing.T) { t.Parallel() - defer func() { - if err := recover(); err != nil { - require.Equal(t, "add: invalid http method JANE\n", fmt.Sprintf("%v", err)) - } - }() methods := append(DefaultMethods, "JOHN") //nolint:gocritic // We want a new slice here app := New(Config{ @@ -607,11 +602,9 @@ func Test_App_Add_Method_Test(t *testing.T) { require.Equal(t, StatusNotImplemented, resp.StatusCode, "Status code") // Add a new method - app.Add([]string{"JANE"}, "/jane", testEmptyHandler) - - resp, err = app.Test(httptest.NewRequest("JANE", "/jane", nil)) - require.NoError(t, err, "app.Test(req)") - require.Equal(t, StatusOK, resp.StatusCode, "Status code") + require.Panics(t, func() { + app.Add([]string{"JANE"}, "/jane", testEmptyHandler) + }) } func Test_App_All_Method_Test(t *testing.T) { diff --git a/ctx_test.go b/ctx_test.go index eea22b1a8d..eb81876e37 100644 --- a/ctx_test.go +++ b/ctx_test.go @@ -131,12 +131,6 @@ func Test_Ctx_CustomCtx(t *testing.T) { func Test_Ctx_CustomCtx_and_Method(t *testing.T) { t.Parallel() - defer func() { - if err := recover(); err != nil { - require.Equal(t, "add: invalid http method JANE\n", fmt.Sprintf("%v", err)) - } - }() - // Create app with custom request methods methods := append(DefaultMethods, "JOHN") //nolint:gocritic // We want a new slice here app := New(Config{ @@ -157,10 +151,9 @@ func Test_Ctx_CustomCtx_and_Method(t *testing.T) { require.Equal(t, StatusOK, resp.StatusCode, "Status code") // Add a new method - app.Add([]string{"JANE"}, "/doe", testEmptyHandler) - resp, err = app.Test(httptest.NewRequest("JANE", "/doe", nil)) - require.NoError(t, err, "app.Test(req)") - require.Equal(t, StatusOK, resp.StatusCode, "Status code") + require.Panics(t, func() { + app.Add([]string{"JANE"}, "/jane", testEmptyHandler) + }) } // go test -run Test_Ctx_Accepts_EmptyAccept From 14a8635edc6901fa281ee9f72bcfb5519fc596bf Mon Sep 17 00:00:00 2001 From: Juan Calderon-Perez <835733+gaby@users.noreply.github.com> Date: Sun, 29 Dec 2024 02:04:18 -0500 Subject: [PATCH 11/11] Simplify test --- app_test.go | 13 +++---------- 1 file changed, 3 insertions(+), 10 deletions(-) diff --git a/app_test.go b/app_test.go index 9b005ced10..8455ded86e 100644 --- a/app_test.go +++ b/app_test.go @@ -609,11 +609,6 @@ func Test_App_Add_Method_Test(t *testing.T) { func Test_App_All_Method_Test(t *testing.T) { t.Parallel() - defer func() { - if err := recover(); err != nil { - require.Equal(t, "add: invalid http method JANE\n", fmt.Sprintf("%v", err)) - } - }() methods := append(DefaultMethods, "JOHN") //nolint:gocritic // We want a new slice here app := New(Config{ @@ -628,11 +623,9 @@ func Test_App_All_Method_Test(t *testing.T) { require.Equal(t, StatusOK, resp.StatusCode, "Status code") // Add a new method - app.Add([]string{"JANE"}, "/doe", testEmptyHandler) - - resp, err = app.Test(httptest.NewRequest("JANE", "/doe", nil)) - require.NoError(t, err, "app.Test(req)") - require.Equal(t, StatusOK, resp.StatusCode, "Status code") + require.Panics(t, func() { + app.Add([]string{"JANE"}, "/jane", testEmptyHandler) + }) } // go test -run Test_App_GETOnly