From c5c7f86d85d91718692433b7b031c9f78bc9a16e Mon Sep 17 00:00:00 2001 From: JIeJaitt <498938874@qq.com> Date: Tue, 1 Apr 2025 14:48:19 +0800 Subject: [PATCH] =?UTF-8?q?=F0=9F=94=A5=20Feature:=20Enhance=20CheckConstr?= =?UTF-8?q?aint=20method=20for=20improved=20error=20handling=20(#3356)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * 🔥 Feature: Enhance CheckConstraint method for improved error handling * Revert "🔥 Feature: Enhance CheckConstraint method for improved error handling" This reverts commit 68e8777b4cdb10702c3511a65ba76986a379a188. * Reapply "🔥 Feature: Enhance CheckConstraint method for improved error handling" This reverts commit 9e6c8e68df5ad43d869b6a7494e93b6fc41b4a40. * 🚨 Test: Add comprehensive tests for CheckConstraint method with various constraint scenarios * 🩹 Fix: lint error * 🩹 Fix: Update CheckConstraint method to return true for noConstraint and improve error handling * ♻️ Refactor: Remove unused CheckConstraint test cases and reorganize benchmark test cases for clarity * ♻️ Refactor: Remove outdated test cases from path_testcases_test.go and clean up CheckConstraint method in path.go * 📚 Doc: Update custom constraints section to clarify overriding behavior * 🔥 Feature: Enhance CheckConstraint method for improved error handling * Revert "🔥 Feature: Enhance CheckConstraint method for improved error handling" This reverts commit 68e8777b4cdb10702c3511a65ba76986a379a188. * Reapply "🔥 Feature: Enhance CheckConstraint method for improved error handling" This reverts commit 9e6c8e68df5ad43d869b6a7494e93b6fc41b4a40. * 🚨 Test: Add comprehensive tests for CheckConstraint method with various constraint scenarios * 🩹 Fix: lint error * 🩹 Fix: Update CheckConstraint method to return true for noConstraint and improve error handling * ♻️ Refactor: Remove unused CheckConstraint test cases and reorganize benchmark test cases for clarity * ♻️ Refactor: Remove outdated test cases from path_testcases_test.go and clean up CheckConstraint method in path.go * 📚 Doc: Update custom constraints section to clarify overriding behavior * 📚 Doc: Add caution note about custom constraints overriding built-in constraints in routing guide --------- Co-authored-by: Juan Calderon-Perez <835733+gaby@users.noreply.github.com> Co-authored-by: RW --- docs/guide/routing.md | 4 ++++ path.go | 39 +++++++++++++++++++++++++++------------ path_testcases_test.go | 8 ++++++++ 3 files changed, 39 insertions(+), 12 deletions(-) diff --git a/docs/guide/routing.md b/docs/guide/routing.md index 450932cc..8953dc08 100644 --- a/docs/guide/routing.md +++ b/docs/guide/routing.md @@ -250,6 +250,10 @@ app.Get("/:test?", func(c fiber.Ctx) error { Custom constraints can be added to Fiber using the `app.RegisterCustomConstraint` method. Your constraints have to be compatible with the `CustomConstraint` interface. +:::caution +Attention, custom constraints can now override built-in constraints. If a custom constraint has the same name as a built-in constraint, the custom constraint will be used instead. This allows for more flexibility in defining route parameter constraints. +::: + It is a good idea to add external constraints to your project once you want to add more specific rules to your routes. For example, you can add a constraint to check if a parameter is a valid ULID. diff --git a/path.go b/path.go index 8cfde73f..05589884 100644 --- a/path.go +++ b/path.go @@ -672,12 +672,25 @@ func getParamConstraintType(constraintPart string) TypeConstraint { } } -//nolint:errcheck // TODO: Properly check _all_ errors in here, log them & immediately return +// CheckConstraint validates if a param matches the given constraint +// Returns true if the param passes the constraint check, false otherwise +// +//nolint:errcheck // TODO: Properly check _all_ errors in here, log them or immediately return func (c *Constraint) CheckConstraint(param string) bool { - var err error - var num int + // First check if there's a custom constraint with the same name + // This allows custom constraints to override built-in constraints + for _, cc := range c.customConstraints { + if cc.Name() == c.Name { + return cc.Execute(param, c.Data...) + } + } - // check data exists + var ( + err error + num int + ) + + // Validate constraint has required data needOneData := []TypeConstraint{minLenConstraint, maxLenConstraint, lenConstraint, minConstraint, maxConstraint, datetimeConstraint, regexConstraint} needTwoData := []TypeConstraint{betweenLenConstraint, rangeConstraint} @@ -696,11 +709,7 @@ func (c *Constraint) CheckConstraint(param string) bool { // check constraints switch c.ID { case noConstraint: - for _, cc := range c.customConstraints { - if cc.Name() == c.Name { - return cc.Execute(param, c.Data...) - } - } + return true case intConstraint: _, err = strconv.Atoi(param) case boolConstraint: @@ -744,14 +753,14 @@ func (c *Constraint) CheckConstraint(param string) bool { data, _ := strconv.Atoi(c.Data[0]) num, err = strconv.Atoi(param) - if num < data { + if err != nil || num < data { return false } case maxConstraint: data, _ := strconv.Atoi(c.Data[0]) num, err = strconv.Atoi(param) - if num > data { + if err != nil || num > data { return false } case rangeConstraint: @@ -759,12 +768,18 @@ func (c *Constraint) CheckConstraint(param string) bool { data2, _ := strconv.Atoi(c.Data[1]) num, err = strconv.Atoi(param) - if num < data || num > data2 { + if err != nil || num < data || num > data2 { return false } case datetimeConstraint: _, err = time.Parse(c.Data[0], param) + if err != nil { + return false + } case regexConstraint: + if c.RegexCompiler == nil { + return false + } if match := c.RegexCompiler.MatchString(param); !match { return false } diff --git a/path_testcases_test.go b/path_testcases_test.go index d4c8dc8d..27746636 100644 --- a/path_testcases_test.go +++ b/path_testcases_test.go @@ -713,6 +713,14 @@ func init() { {url: "/api/v1/", params: []string{""}, match: true}, }, }, + // Add test case for RegexCompiler == nil + { + pattern: "/api/v1/:param", + testCases: []routeTestCase{ + {url: "/api/v1/123", params: []string{"123"}, match: true}, + {url: "/api/v1/abc", params: nil, match: false}, + }, + }, }..., ) }