Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

🩹 Fix (v3): handle un-matched open brackets in the query params #3126

Merged
merged 4 commits into from
Sep 9, 2024
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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 @@
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

Check warning on line 144 in binder/mapping.go

View check run for this annotation

Codecov / codecov/patch

binder/mapping.go#L144

Added line #L144 was not covered by tests
}
}
continue
}

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

Expand All @@ -149,6 +160,10 @@
}
}

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

return bb.String(), nil
}

Expand Down
48 changes: 48 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,50 @@
require.True(t, equalFieldType(&user, reflect.Int, "AGE"))
require.True(t, equalFieldType(&user, reflect.Int, "age"))
}

func Test_ParseParamSquareBrackets(t *testing.T) {
tests := []struct {

Check failure on line 35 in binder/mapping_test.go

View workflow job for this annotation

GitHub Actions / lint

fieldalignment: struct with 48 pointer bytes could be 40 (govet)
gaby marked this conversation as resolved.
Show resolved Hide resolved
input string
expected string
err error
}{
{
input: "foo[bar]",
expected: "foo.bar",
err: nil,
},
{
input: "foo[bar][baz]",
expected: "foo.bar.baz",
err: nil,
},
{
input: "foo[bar",
expected: "",
err: errors.New("unmatched brackets"),
},
{
input: "foo[bar][baz",
expected: "",
err: errors.New("unmatched brackets"),
},
{
input: "foo]bar[",
expected: "",
err: errors.New("unmatched brackets"),
},
}

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)
}
})
}
}
gaby marked this conversation as resolved.
Show resolved Hide resolved
Loading