Skip to content

Commit

Permalink
🐛 bug: fix EnableSplittingOnParsers is not functional (#3231)
Browse files Browse the repository at this point in the history
* 🐛 bug: fix EnableSplittingOnParsers is not functional

* remove wrong testcase

* add support for external xml decoders

* improve test coverage

* fix linter

* update

* add reset methods

* improve test coverage

* merge Form and MultipartForm methods

* fix linter

* split reset and putting steps

* fix linter
  • Loading branch information
efectn authored Dec 25, 2024
1 parent 58677d5 commit 57744eb
Show file tree
Hide file tree
Showing 30 changed files with 1,339 additions and 152 deletions.
10 changes: 10 additions & 0 deletions app.go
Original file line number Diff line number Diff line change
Expand Up @@ -341,6 +341,13 @@ type Config struct { //nolint:govet // Aligning the struct fields is not necessa
// Default: xml.Marshal
XMLEncoder utils.XMLMarshal `json:"-"`

// XMLDecoder set by an external client of Fiber it will use the provided implementation of a
// XMLUnmarshal
//
// Allowing for flexibility in using another XML library for decoding
// Default: xml.Unmarshal
XMLDecoder utils.XMLUnmarshal `json:"-"`

// If you find yourself behind some sort of proxy, like a load balancer,
// then certain header information may be sent to you using special X-Forwarded-* headers or the Forwarded header.
// For example, the Host HTTP header is usually used to return the requested host.
Expand Down Expand Up @@ -560,6 +567,9 @@ func New(config ...Config) *App {
if app.config.XMLEncoder == nil {
app.config.XMLEncoder = xml.Marshal
}
if app.config.XMLDecoder == nil {
app.config.XMLDecoder = xml.Unmarshal
}
if len(app.config.RequestMethods) == 0 {
app.config.RequestMethods = DefaultMethods
}
Expand Down
109 changes: 90 additions & 19 deletions bind.go
Original file line number Diff line number Diff line change
Expand Up @@ -77,7 +77,16 @@ func (b *Bind) Custom(name string, dest any) error {

// Header binds the request header strings into the struct, map[string]string and map[string][]string.
func (b *Bind) Header(out any) error {
if err := b.returnErr(binder.HeaderBinder.Bind(b.ctx.Request(), out)); err != nil {
bind := binder.GetFromThePool[*binder.HeaderBinding](&binder.HeaderBinderPool)
bind.EnableSplitting = b.ctx.App().config.EnableSplittingOnParsers

// Reset & put binder
defer func() {
bind.Reset()
binder.PutToThePool(&binder.HeaderBinderPool, bind)
}()

if err := b.returnErr(bind.Bind(b.ctx.Request(), out)); err != nil {
return err
}

Expand All @@ -86,7 +95,16 @@ func (b *Bind) Header(out any) error {

// RespHeader binds the response header strings into the struct, map[string]string and map[string][]string.
func (b *Bind) RespHeader(out any) error {
if err := b.returnErr(binder.RespHeaderBinder.Bind(b.ctx.Response(), out)); err != nil {
bind := binder.GetFromThePool[*binder.RespHeaderBinding](&binder.RespHeaderBinderPool)
bind.EnableSplitting = b.ctx.App().config.EnableSplittingOnParsers

// Reset & put binder
defer func() {
bind.Reset()
binder.PutToThePool(&binder.RespHeaderBinderPool, bind)
}()

if err := b.returnErr(bind.Bind(b.ctx.Response(), out)); err != nil {
return err
}

Expand All @@ -96,7 +114,16 @@ func (b *Bind) RespHeader(out any) error {
// Cookie binds the request cookie strings into the struct, map[string]string and map[string][]string.
// NOTE: If your cookie is like key=val1,val2; they'll be binded as an slice if your map is map[string][]string. Else, it'll use last element of cookie.
func (b *Bind) Cookie(out any) error {
if err := b.returnErr(binder.CookieBinder.Bind(b.ctx.RequestCtx(), out)); err != nil {
bind := binder.GetFromThePool[*binder.CookieBinding](&binder.CookieBinderPool)
bind.EnableSplitting = b.ctx.App().config.EnableSplittingOnParsers

// Reset & put binder
defer func() {
bind.Reset()
binder.PutToThePool(&binder.CookieBinderPool, bind)
}()

if err := b.returnErr(bind.Bind(&b.ctx.RequestCtx().Request, out)); err != nil {
return err
}

Expand All @@ -105,7 +132,16 @@ func (b *Bind) Cookie(out any) error {

// Query binds the query string into the struct, map[string]string and map[string][]string.
func (b *Bind) Query(out any) error {
if err := b.returnErr(binder.QueryBinder.Bind(b.ctx.RequestCtx(), out)); err != nil {
bind := binder.GetFromThePool[*binder.QueryBinding](&binder.QueryBinderPool)
bind.EnableSplitting = b.ctx.App().config.EnableSplittingOnParsers

// Reset & put binder
defer func() {
bind.Reset()
binder.PutToThePool(&binder.QueryBinderPool, bind)
}()

if err := b.returnErr(bind.Bind(&b.ctx.RequestCtx().Request, out)); err != nil {
return err
}

Expand All @@ -114,7 +150,16 @@ func (b *Bind) Query(out any) error {

// JSON binds the body string into the struct.
func (b *Bind) JSON(out any) error {
if err := b.returnErr(binder.JSONBinder.Bind(b.ctx.Body(), b.ctx.App().Config().JSONDecoder, out)); err != nil {
bind := binder.GetFromThePool[*binder.JSONBinding](&binder.JSONBinderPool)
bind.JSONDecoder = b.ctx.App().Config().JSONDecoder

// Reset & put binder
defer func() {
bind.Reset()
binder.PutToThePool(&binder.JSONBinderPool, bind)
}()

if err := b.returnErr(bind.Bind(b.ctx.Body(), out)); err != nil {
return err
}

Expand All @@ -123,24 +168,54 @@ func (b *Bind) JSON(out any) error {

// CBOR binds the body string into the struct.
func (b *Bind) CBOR(out any) error {
if err := b.returnErr(binder.CBORBinder.Bind(b.ctx.Body(), b.ctx.App().Config().CBORDecoder, out)); err != nil {
bind := binder.GetFromThePool[*binder.CBORBinding](&binder.CBORBinderPool)
bind.CBORDecoder = b.ctx.App().Config().CBORDecoder

// Reset & put binder
defer func() {
bind.Reset()
binder.PutToThePool(&binder.CBORBinderPool, bind)
}()

if err := b.returnErr(bind.Bind(b.ctx.Body(), out)); err != nil {
return err
}
return b.validateStruct(out)
}

// XML binds the body string into the struct.
func (b *Bind) XML(out any) error {
if err := b.returnErr(binder.XMLBinder.Bind(b.ctx.Body(), out)); err != nil {
bind := binder.GetFromThePool[*binder.XMLBinding](&binder.XMLBinderPool)
bind.XMLDecoder = b.ctx.App().config.XMLDecoder

// Reset & put binder
defer func() {
bind.Reset()
binder.PutToThePool(&binder.XMLBinderPool, bind)
}()

if err := b.returnErr(bind.Bind(b.ctx.Body(), out)); err != nil {
return err
}

return b.validateStruct(out)
}

// Form binds the form into the struct, map[string]string and map[string][]string.
// If Content-Type is "application/x-www-form-urlencoded" or "multipart/form-data", it will bind the form values.
//
// Binding multipart files is not supported yet.
func (b *Bind) Form(out any) error {
if err := b.returnErr(binder.FormBinder.Bind(b.ctx.RequestCtx(), out)); err != nil {
bind := binder.GetFromThePool[*binder.FormBinding](&binder.FormBinderPool)
bind.EnableSplitting = b.ctx.App().config.EnableSplittingOnParsers

// Reset & put binder
defer func() {
bind.Reset()
binder.PutToThePool(&binder.FormBinderPool, bind)
}()

if err := b.returnErr(bind.Bind(&b.ctx.RequestCtx().Request, out)); err != nil {
return err
}

Expand All @@ -149,16 +224,14 @@ func (b *Bind) Form(out any) error {

// URI binds the route parameters into the struct, map[string]string and map[string][]string.
func (b *Bind) URI(out any) error {
if err := b.returnErr(binder.URIBinder.Bind(b.ctx.Route().Params, b.ctx.Params, out)); err != nil {
return err
}
bind := binder.GetFromThePool[*binder.URIBinding](&binder.URIBinderPool)

return b.validateStruct(out)
}
// Reset & put binder
defer func() {
binder.PutToThePool(&binder.URIBinderPool, bind)
}()

// MultipartForm binds the multipart form into the struct, map[string]string and map[string][]string.
func (b *Bind) MultipartForm(out any) error {
if err := b.returnErr(binder.FormBinder.BindMultipart(b.ctx.RequestCtx(), out)); err != nil {
if err := b.returnErr(bind.Bind(b.ctx.Route().Params, b.ctx.Params, out)); err != nil {
return err
}

Expand Down Expand Up @@ -193,10 +266,8 @@ func (b *Bind) Body(out any) error {
return b.XML(out)
case MIMEApplicationCBOR:
return b.CBOR(out)
case MIMEApplicationForm:
case MIMEApplicationForm, MIMEMultipartForm:
return b.Form(out)
case MIMEMultipartForm:
return b.MultipartForm(out)
}

// No suitable content type found
Expand Down
38 changes: 25 additions & 13 deletions bind_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,9 @@ func Test_returnErr(t *testing.T) {
// go test -run Test_Bind_Query -v
func Test_Bind_Query(t *testing.T) {
t.Parallel()
app := New()
app := New(Config{
EnableSplittingOnParsers: true,
})
c := app.AcquireCtx(&fasthttp.RequestCtx{})

type Query struct {
Expand Down Expand Up @@ -111,7 +113,9 @@ func Test_Bind_Query(t *testing.T) {
func Test_Bind_Query_Map(t *testing.T) {
t.Parallel()

app := New()
app := New(Config{
EnableSplittingOnParsers: true,
})
c := app.AcquireCtx(&fasthttp.RequestCtx{})

c.Request().SetBody([]byte(``))
Expand Down Expand Up @@ -318,13 +322,13 @@ func Test_Bind_Header(t *testing.T) {
c.Request().Header.Add("Hobby", "golang,fiber")
q := new(Header)
require.NoError(t, c.Bind().Header(q))
require.Len(t, q.Hobby, 2)
require.Len(t, q.Hobby, 1)

c.Request().Header.Del("hobby")
c.Request().Header.Add("Hobby", "golang,fiber,go")
q = new(Header)
require.NoError(t, c.Bind().Header(q))
require.Len(t, q.Hobby, 3)
require.Len(t, q.Hobby, 1)

empty := new(Header)
c.Request().Header.Del("hobby")
Expand Down Expand Up @@ -357,7 +361,7 @@ func Test_Bind_Header(t *testing.T) {
require.Equal(t, "go,fiber", h2.Hobby)
require.True(t, h2.Bool)
require.Equal(t, "Jane Doe", h2.Name) // check value get overwritten
require.Equal(t, []string{"milo", "coke", "pepsi"}, h2.FavouriteDrinks)
require.Equal(t, []string{"milo,coke,pepsi"}, h2.FavouriteDrinks)
var nilSlice []string
require.Equal(t, nilSlice, h2.Empty)
require.Equal(t, []string{""}, h2.Alloc)
Expand Down Expand Up @@ -386,13 +390,13 @@ func Test_Bind_Header_Map(t *testing.T) {
c.Request().Header.Add("Hobby", "golang,fiber")
q := make(map[string][]string, 0)
require.NoError(t, c.Bind().Header(&q))
require.Len(t, q["Hobby"], 2)
require.Len(t, q["Hobby"], 1)

c.Request().Header.Del("hobby")
c.Request().Header.Add("Hobby", "golang,fiber,go")
q = make(map[string][]string, 0)
require.NoError(t, c.Bind().Header(&q))
require.Len(t, q["Hobby"], 3)
require.Len(t, q["Hobby"], 1)

empty := make(map[string][]string, 0)
c.Request().Header.Del("hobby")
Expand Down Expand Up @@ -543,7 +547,9 @@ func Test_Bind_Header_Schema(t *testing.T) {
// go test -run Test_Bind_Resp_Header -v
func Test_Bind_RespHeader(t *testing.T) {
t.Parallel()
app := New()
app := New(Config{
EnableSplittingOnParsers: true,
})
c := app.AcquireCtx(&fasthttp.RequestCtx{})

type Header struct {
Expand Down Expand Up @@ -627,13 +633,13 @@ func Test_Bind_RespHeader_Map(t *testing.T) {
c.Response().Header.Add("Hobby", "golang,fiber")
q := make(map[string][]string, 0)
require.NoError(t, c.Bind().RespHeader(&q))
require.Len(t, q["Hobby"], 2)
require.Len(t, q["Hobby"], 1)

c.Response().Header.Del("hobby")
c.Response().Header.Add("Hobby", "golang,fiber,go")
q = make(map[string][]string, 0)
require.NoError(t, c.Bind().RespHeader(&q))
require.Len(t, q["Hobby"], 3)
require.Len(t, q["Hobby"], 1)

empty := make(map[string][]string, 0)
c.Response().Header.Del("hobby")
Expand Down Expand Up @@ -751,7 +757,9 @@ func Benchmark_Bind_Query_WithParseParam(b *testing.B) {
func Benchmark_Bind_Query_Comma(b *testing.B) {
var err error

app := New()
app := New(Config{
EnableSplittingOnParsers: true,
})
c := app.AcquireCtx(&fasthttp.RequestCtx{})

type Query struct {
Expand Down Expand Up @@ -1341,7 +1349,9 @@ func Benchmark_Bind_URI_Map(b *testing.B) {
func Test_Bind_Cookie(t *testing.T) {
t.Parallel()

app := New()
app := New(Config{
EnableSplittingOnParsers: true,
})
c := app.AcquireCtx(&fasthttp.RequestCtx{})

type Cookie struct {
Expand Down Expand Up @@ -1414,7 +1424,9 @@ func Test_Bind_Cookie(t *testing.T) {
func Test_Bind_Cookie_Map(t *testing.T) {
t.Parallel()

app := New()
app := New(Config{
EnableSplittingOnParsers: true,
})
c := app.AcquireCtx(&fasthttp.RequestCtx{})

c.Request().SetBody([]byte(``))
Expand Down
Loading

1 comment on commit 57744eb

@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: 57744eb Previous: 47be681 Ratio
Benchmark_Ctx_AcceptsEncodings 266.5 ns/op 0 B/op 0 allocs/op 170.5 ns/op 0 B/op 0 allocs/op 1.56
Benchmark_Ctx_AcceptsEncodings - ns/op 266.5 ns/op 170.5 ns/op 1.56
Benchmark_Ctx_AcceptsLanguages 384.2 ns/op 0 B/op 0 allocs/op 249.2 ns/op 0 B/op 0 allocs/op 1.54
Benchmark_Ctx_AcceptsLanguages - ns/op 384.2 ns/op 249.2 ns/op 1.54
Benchmark_Utils_GetOffer/1_parameter 207.2 ns/op 0 B/op 0 allocs/op 131.7 ns/op 0 B/op 0 allocs/op 1.57
Benchmark_Utils_GetOffer/1_parameter - ns/op 207.2 ns/op 131.7 ns/op 1.57
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 518 B/op 332 B/op 1.56
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.