Skip to content

Commit

Permalink
🩹 Fix (v3): handle un-matched open brackets in the query params (#3126)
Browse files Browse the repository at this point in the history
* Add logic for handling unmatched square brackets in query params

* Add UTs

* Fix lint: fieldalignment error

* Add UTs
  • Loading branch information
dojutsu-user committed Sep 9, 2024
1 parent f668537 commit 62f66e5
Show file tree
Hide file tree
Showing 2 changed files with 88 additions and 5 deletions.
25 changes: 20 additions & 5 deletions binder/mapping.go
Original file line number Diff line number Diff line change
@@ -1,13 +1,15 @@
package binder

import (
"errors"
"reflect"
"strings"
"sync"

"github.com/gofiber/fiber/v3/internal/schema"
"github.com/gofiber/utils/v2"
"github.com/valyala/bytebufferpool"

"github.com/gofiber/fiber/v3/internal/schema"
)

// ParserConfig form decoder config for SetParserDecoder
Expand Down Expand Up @@ -132,15 +134,24 @@ func parseParamSquareBrackets(k string) (string, error) {
defer bytebufferpool.Put(bb)

kbytes := []byte(k)
openBracketsCount := 0

for i, b := range kbytes {
if b == '[' && kbytes[i+1] != ']' {
if err := bb.WriteByte('.'); err != nil {
return "", err //nolint:wrapcheck // unnecessary to wrap it
if b == '[' {
openBracketsCount++
if i+1 < len(kbytes) && kbytes[i+1] != ']' {
if err := bb.WriteByte('.'); err != nil {
return "", err //nolint:wrapcheck // unnecessary to wrap it
}
}
continue
}

if b == '[' || b == ']' {
if b == ']' {
openBracketsCount--
if openBracketsCount < 0 {
return "", errors.New("unmatched brackets")
}
continue
}

Expand All @@ -149,6 +160,10 @@ func parseParamSquareBrackets(k string) (string, error) {
}
}

if openBracketsCount > 0 {
return "", errors.New("unmatched brackets")
}

return bb.String(), nil
}

Expand Down
68 changes: 68 additions & 0 deletions binder/mapping_test.go
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
package binder

import (
"errors"
"reflect"
"testing"

Expand Down Expand Up @@ -29,3 +30,70 @@ func Test_EqualFieldType(t *testing.T) {
require.True(t, equalFieldType(&user, reflect.Int, "AGE"))
require.True(t, equalFieldType(&user, reflect.Int, "age"))
}

func Test_ParseParamSquareBrackets(t *testing.T) {
tests := []struct {
err error
input string
expected string
}{
{
err: nil,
input: "foo[bar]",
expected: "foo.bar",
},
{
err: nil,
input: "foo[bar][baz]",
expected: "foo.bar.baz",
},
{
err: errors.New("unmatched brackets"),
input: "foo[bar",
expected: "",
},
{
err: errors.New("unmatched brackets"),
input: "foo[bar][baz",
expected: "",
},
{
err: errors.New("unmatched brackets"),
input: "foo]bar[",
expected: "",
},
{
err: nil,
input: "foo[bar[baz]]",
expected: "foo.bar.baz",
},
{
err: nil,
input: "",
expected: "",
},
{
err: nil,
input: "[]",
expected: "",
},
{
err: nil,
input: "foo[]",
expected: "foo",
},
}

for _, tt := range tests {
t.Run(tt.input, func(t *testing.T) {
result, err := parseParamSquareBrackets(tt.input)
if tt.err != nil {
require.Error(t, err)
require.EqualError(t, err, tt.err.Error())
} else {
require.NoError(t, err)
require.Equal(t, tt.expected, result)
}
})
}
}

1 comment on commit 62f66e5

@github-actions
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Performance Alert ⚠️

Possible performance regression was detected for benchmark.
Benchmark result of this commit is worse than the previous benchmark result exceeding threshold 1.50.

Benchmark suite Current: 62f66e5 Previous: 6e74114 Ratio
Benchmark_Ctx_Send 7.434 ns/op 0 B/op 0 allocs/op 4.342 ns/op 0 B/op 0 allocs/op 1.71
Benchmark_Ctx_Send - ns/op 7.434 ns/op 4.342 ns/op 1.71
Benchmark_Utils_GetOffer/1_parameter 208.3 ns/op 0 B/op 0 allocs/op 131.1 ns/op 0 B/op 0 allocs/op 1.59
Benchmark_Utils_GetOffer/1_parameter - ns/op 208.3 ns/op 131.1 ns/op 1.59
Benchmark_Middleware_BasicAuth - B/op 80 B/op 48 B/op 1.67
Benchmark_Middleware_BasicAuth - allocs/op 5 allocs/op 3 allocs/op 1.67
Benchmark_Middleware_BasicAuth_Upper - B/op 80 B/op 48 B/op 1.67
Benchmark_Middleware_BasicAuth_Upper - allocs/op 5 allocs/op 3 allocs/op 1.67
Benchmark_CORS_NewHandler - B/op 16 B/op 0 B/op +∞
Benchmark_CORS_NewHandler - allocs/op 1 allocs/op 0 allocs/op +∞
Benchmark_CORS_NewHandlerSingleOrigin - B/op 16 B/op 0 B/op +∞
Benchmark_CORS_NewHandlerSingleOrigin - allocs/op 1 allocs/op 0 allocs/op +∞
Benchmark_CORS_NewHandlerPreflight - B/op 104 B/op 0 B/op +∞
Benchmark_CORS_NewHandlerPreflight - allocs/op 5 allocs/op 0 allocs/op +∞
Benchmark_CORS_NewHandlerPreflightSingleOrigin - B/op 104 B/op 0 B/op +∞
Benchmark_CORS_NewHandlerPreflightSingleOrigin - allocs/op 5 allocs/op 0 allocs/op +∞
Benchmark_CORS_NewHandlerPreflightWildcard - B/op 104 B/op 0 B/op +∞
Benchmark_CORS_NewHandlerPreflightWildcard - allocs/op 5 allocs/op 0 allocs/op +∞
Benchmark_Middleware_CSRF_GenerateToken - B/op 517 B/op 334 B/op 1.55
Benchmark_Middleware_CSRF_GenerateToken - allocs/op 10 allocs/op 6 allocs/op 1.67

This comment was automatically generated by workflow using github-action-benchmark.

Please sign in to comment.