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

🔥 feat: Add support for CBOR encoding #3173

Open
wants to merge 13 commits into
base: main
Choose a base branch
from
22 changes: 21 additions & 1 deletion app.go
Original file line number Diff line number Diff line change
Expand Up @@ -23,9 +23,9 @@ import (
"sync"
"time"

"github.com/fxamacker/cbor/v2"
"github.com/gofiber/fiber/v3/log"
"github.com/gofiber/utils/v2"

"github.com/valyala/fasthttp"
)

Expand Down Expand Up @@ -318,6 +318,20 @@ type Config struct { //nolint:govet // Aligning the struct fields is not necessa
// Default: json.Unmarshal
JSONDecoder utils.JSONUnmarshal `json:"-"`

// When set by an external client of Fiber it will use the provided implementation of a
// CBORMarshal
//
// Allowing for flexibility in using another cbor library for encoding
// Default: cbor.Marshal
CBOREncoder utils.CBORMarshal `json:"-"`

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

// XMLEncoder set by an external client of Fiber it will use the provided implementation of a
// XMLMarshal
//
Expand Down Expand Up @@ -535,6 +549,12 @@ func New(config ...Config) *App {
if app.config.JSONDecoder == nil {
app.config.JSONDecoder = json.Unmarshal
}
if app.config.CBOREncoder == nil {
app.config.CBOREncoder = cbor.Marshal
}
if app.config.CBORDecoder == nil {
app.config.CBORDecoder = cbor.Unmarshal
}
if app.config.XMLEncoder == nil {
app.config.XMLEncoder = xml.Marshal
}
Expand Down
9 changes: 9 additions & 0 deletions bind.go
Original file line number Diff line number Diff line change
Expand Up @@ -120,6 +120,13 @@ func (b *Bind) JSON(out any) error {
return b.validateStruct(out)
}

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 {
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 {
Expand Down Expand Up @@ -182,6 +189,8 @@ func (b *Bind) Body(out any) error {
return b.JSON(out)
case MIMETextXML, MIMEApplicationXML:
return b.XML(out)
case MIMEApplicationCBOR:
return b.CBOR(out)
case MIMEApplicationForm:
return b.Form(out)
case MIMEMultipartForm:
Expand Down
45 changes: 45 additions & 0 deletions bind_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ import (
"testing"
"time"

"github.com/fxamacker/cbor/v2"
"github.com/gofiber/fiber/v3/binder"
"github.com/stretchr/testify/require"
"github.com/valyala/fasthttp"
Expand Down Expand Up @@ -927,6 +928,14 @@ func Test_Bind_Body(t *testing.T) {
t.Run("JSON", func(t *testing.T) {
testDecodeParser(t, MIMEApplicationJSON, `{"name":"john"}`)
})
t.Run("CBOR", func(t *testing.T) {
enc, err := cbor.Marshal(&Demo{Name: "john"})
if err != nil {
t.Error(err)
}
str := string(enc)
testDecodeParser(t, MIMEApplicationCBOR, str)
})
Copy link
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue

Avoid converting CBOR binary data to a string

CBOR data is binary, and converting it to a string can lead to data corruption or unexpected behavior. Modify testDecodeParser to accept a byte slice instead of a string to handle binary data properly.

Refactor testDecodeParser to accept []byte for the body parameter:

-func testDecodeParser(t *testing.T, contentType, body string) {
+func testDecodeParser(t *testing.T, contentType string, body []byte) {
     t.Helper()
     c := app.AcquireCtx(&fasthttp.RequestCtx{})
     c.Request().Header.SetContentType(contentType)
-    c.Request().SetBody([]byte(body))
+    c.Request().SetBody(body)
     c.Request().Header.SetContentLength(len(body))
     d := new(Demo)
     require.NoError(t, c.Bind().Body(d))
     require.Equal(t, "john", d.Name)
 }

Update the test case to pass the byte slice directly:

-	str := string(enc)
-	testDecodeParser(t, MIMEApplicationCBOR, str)
+	testDecodeParser(t, MIMEApplicationCBOR, enc)
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
t.Run("CBOR", func(t *testing.T) {
enc, err := cbor.Marshal(&Demo{Name: "john"})
if err != nil {
t.Error(err)
}
str := string(enc)
testDecodeParser(t, MIMEApplicationCBOR, str)
})
t.Run("CBOR", func(t *testing.T) {
enc, err := cbor.Marshal(&Demo{Name: "john"})
if err != nil {
t.Error(err)
}
testDecodeParser(t, MIMEApplicationCBOR, enc)
})


t.Run("XML", func(t *testing.T) {
testDecodeParser(t, MIMEApplicationXML, `<Demo><name>john</name></Demo>`)
Expand Down Expand Up @@ -1091,6 +1100,35 @@ func Benchmark_Bind_Body_XML(b *testing.B) {
require.Equal(b, "john", d.Name)
}

// go test -v -run=^$ -bench=Benchmark_Bind_Body_CBOR -benchmem -count=4
func Benchmark_Bind_Body_CBOR(b *testing.B) {
var err error

app := New()
c := app.AcquireCtx(&fasthttp.RequestCtx{})

type Demo struct {
Name string `json:"name"`
}
body, err := cbor.Marshal(&Demo{Name: "john"})
if err != nil {
b.Error(err)
}
c.Request().SetBody(body)
c.Request().Header.SetContentType(MIMEApplicationCBOR)
c.Request().Header.SetContentLength(len(body))
d := new(Demo)

b.ReportAllocs()
b.ResetTimer()

for n := 0; n < b.N; n++ {
err = c.Bind().Body(d)
}
require.NoError(b, err)
require.Equal(b, "john", d.Name)
}

// go test -v -run=^$ -bench=Benchmark_Bind_Body_Form -benchmem -count=4
func Benchmark_Bind_Body_Form(b *testing.B) {
var err error
Expand Down Expand Up @@ -1710,9 +1748,16 @@ func Test_Bind_RepeatParserWithSameStruct(t *testing.T) {
require.NoError(t, c.Bind().Body(r))
require.Equal(t, "body_param", r.BodyParam)
}
cb, err := cbor.Marshal(&Request{
BodyParam: "body_param",
})
if err != nil {
t.Error(err)
}

testDecodeParser(MIMEApplicationJSON, `{"body_param":"body_param"}`)
testDecodeParser(MIMEApplicationXML, `<Demo><body_param>body_param</body_param></Demo>`)
testDecodeParser(MIMEApplicationCBOR, string(cb))
Comment on lines +1750 to +1759
Copy link
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue

Fix potential binary data corruption

Converting CBOR binary data to string and back could lead to data corruption. The testDecodeParser function should be used with the raw byte slice.

-    testDecodeParser(MIMEApplicationCBOR, string(cb))
+    testDecodeParser(MIMEApplicationCBOR, cb)
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
cb, err := cbor.Marshal(&Request{
BodyParam: "body_param",
})
if err != nil {
t.Error(err)
}
testDecodeParser(MIMEApplicationJSON, `{"body_param":"body_param"}`)
testDecodeParser(MIMEApplicationXML, `<Demo><body_param>body_param</body_param></Demo>`)
testDecodeParser(MIMEApplicationCBOR, string(cb))
cb, err := cbor.Marshal(&Request{
BodyParam: "body_param",
})
if err != nil {
t.Error(err)
}
testDecodeParser(MIMEApplicationJSON, `{"body_param":"body_param"}`)
testDecodeParser(MIMEApplicationXML, `<Demo><body_param>body_param</body_param></Demo>`)
testDecodeParser(MIMEApplicationCBOR, cb)

testDecodeParser(MIMEApplicationForm, "body_param=body_param")
testDecodeParser(MIMEMultipartForm+`;boundary="b"`, "--b\r\nContent-Disposition: form-data; name=\"body_param\"\r\n\r\nbody_param\r\n--b--")
}
1 change: 1 addition & 0 deletions binder/binder.go
Original file line number Diff line number Diff line change
Expand Up @@ -20,4 +20,5 @@ var (
URIBinder = &uriBinding{}
XMLBinder = &xmlBinding{}
JSONBinder = &jsonBinding{}
CBORBinder = &cborBinding{}
)
15 changes: 15 additions & 0 deletions binder/cbor.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
package binder

import (
"github.com/gofiber/utils/v2"
)
gaby marked this conversation as resolved.
Show resolved Hide resolved

type cborBinding struct{}

func (*cborBinding) Name() string {
Copy link
Member

Choose a reason for hiding this comment

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

This whole file is missing tests

return "json"
}
Copy link
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue

Fix incorrect binding name.

The Name() method returns "json" which is incorrect for a CBOR binding. This could cause confusion and potential issues with binding selection.

Apply this fix:

func (*cborBinding) Name() string {
-	return "json"
+	return "cbor"
}
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
func (*cborBinding) Name() string {
return "json"
}
func (*cborBinding) Name() string {
return "cbor"
}


func (*cborBinding) Bind(body []byte, cborDecoder utils.CBORUnmarshal, out any) error {
return cborDecoder(body, out)
}
24 changes: 24 additions & 0 deletions client/client.go
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,8 @@ type Client struct {
jsonUnmarshal utils.JSONUnmarshal
xmlMarshal utils.XMLMarshal
xmlUnmarshal utils.XMLUnmarshal
cborMarshal utils.CBORMarshal
cborUnmarshal utils.CBORUnmarshal
gaby marked this conversation as resolved.
Show resolved Hide resolved

cookieJar *CookieJar

Expand Down Expand Up @@ -150,6 +152,28 @@ func (c *Client) SetXMLUnmarshal(f utils.XMLUnmarshal) *Client {
return c
}

// CBORMarshal returns xml marshal function in Core.
func (c *Client) CBORMarshal() utils.CBORMarshal {
return c.cborMarshal
}

// SetCBORMarshal Set xml encoder.
func (c *Client) SetCBORMarshal(f utils.CBORMarshal) *Client {
c.cborMarshal = f
return c
}

// CBORUnmarshal returns xml unmarshal function in Core.
func (c *Client) CBORUnmarshal() utils.CBORUnmarshal {
return c.cborUnmarshal
}

// SetCBORUnmarshal Set xml decoder.
func (c *Client) SetCBORUnmarshal(f utils.CBORUnmarshal) *Client {
c.cborUnmarshal = f
return c
}

// TLSConfig returns tlsConfig in client.
// If client don't have tlsConfig, this function will init it.
func (c *Client) TLSConfig() *tls.Config {
Expand Down
28 changes: 28 additions & 0 deletions client/client_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ package client
import (
"context"
"crypto/tls"
"encoding/hex"
"errors"
"io"
"net"
Expand Down Expand Up @@ -202,6 +203,33 @@ func Test_Client_Marshal(t *testing.T) {
require.Equal(t, errors.New("empty xml"), err)
})

Copy link
Member

Choose a reason for hiding this comment

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

This is missing a test using the default CBOR Marshall and UnMarshall functions as noted by codecov below.

t.Run("set cbor marshal", func(t *testing.T) {
t.Parallel()
bs, err := hex.DecodeString("f6")
if err != nil {
t.Error(err)
}
client := New().
SetCBORMarshal(func(_ any) ([]byte, error) {
return bs, nil
})
val, err := client.CBORMarshal()(nil)

require.NoError(t, err)
require.Equal(t, bs, val)
})

t.Run("set cbor marshal error", func(t *testing.T) {
t.Parallel()
client := New().SetCBORMarshal(func(_ any) ([]byte, error) {
return nil, errors.New("invalid struct")
})

val, err := client.CBORMarshal()(nil)
require.Nil(t, val)
require.Equal(t, errors.New("invalid struct"), err)
})
gaby marked this conversation as resolved.
Show resolved Hide resolved

t.Run("set xml unmarshal", func(t *testing.T) {
t.Parallel()
client := New().
Expand Down
9 changes: 9 additions & 0 deletions client/hooks.go
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ var (
headerAccept = "Accept"

applicationJSON = "application/json"
applicationCBOR = "application/cbor"
applicationXML = "application/xml"
applicationForm = "application/x-www-form-urlencoded"
multipartFormData = "multipart/form-data"
Expand Down Expand Up @@ -129,6 +130,8 @@ func parserRequestHeader(c *Client, req *Request) error {
req.RawRequest.Header.Set(headerAccept, applicationJSON)
case xmlBody:
req.RawRequest.Header.SetContentType(applicationXML)
case cborBody:
req.RawRequest.Header.SetContentType(applicationCBOR)
case formBody:
req.RawRequest.Header.SetContentType(applicationForm)
case filesBody:
Expand Down Expand Up @@ -189,6 +192,12 @@ func parserRequestBody(c *Client, req *Request) error {
return err
}
req.RawRequest.SetBody(body)
case cborBody:
Copy link
Member

Choose a reason for hiding this comment

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

Missing hook test for cbor

body, err := c.cborMarshal(req.body)
if err != nil {
return err
}
req.RawRequest.SetBody(body)
case formBody:
req.RawRequest.SetBody(req.formData.QueryString())
case filesBody:
Expand Down
7 changes: 7 additions & 0 deletions client/request.go
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,7 @@ const (
formBody
filesBody
rawBody
cborBody
)

var ErrClientNil = errors.New("client can not be nil")
Expand Down Expand Up @@ -337,6 +338,12 @@ func (r *Request) SetXML(v any) *Request {
return r
}

func (r *Request) SetCBOR(v any) *Request {
Copy link
Member

Choose a reason for hiding this comment

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

Missing test using CBOR data.

r.body = v
r.bodyType = cborBody
return r
}

// SetRawBody method sets body with raw data in request.
func (r *Request) SetRawBody(v []byte) *Request {
r.body = v
Expand Down
5 changes: 5 additions & 0 deletions client/response.go
Original file line number Diff line number Diff line change
Expand Up @@ -75,6 +75,11 @@ func (r *Response) JSON(v any) error {
return r.client.jsonUnmarshal(r.Body(), v)
}

// CBOR method will unmarshal body to cbor.
func (r *Response) CBOR(v any) error {
Copy link
Member

Choose a reason for hiding this comment

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

Missing test receiving CBOR data

return r.client.cborUnmarshal(r.Body(), v)
}

// XML method will unmarshal body to xml.
func (r *Response) XML(v any) error {
return r.client.xmlUnmarshal(r.Body(), v)
Expand Down
1 change: 1 addition & 0 deletions constants.go
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ const (
MIMETextCSS = "text/css"
MIMEApplicationXML = "application/xml"
MIMEApplicationJSON = "application/json"
MIMEApplicationCBOR = "application/cbor"
// Deprecated: use MIMETextJavaScript instead
MIMEApplicationJavaScript = "application/javascript"
MIMEApplicationForm = "application/x-www-form-urlencoded"
Expand Down
18 changes: 18 additions & 0 deletions ctx.go
Original file line number Diff line number Diff line change
Expand Up @@ -883,6 +883,24 @@ func (c *DefaultCtx) JSON(data any, ctype ...string) error {
return nil
}

// CBOR converts any interface or string to cbor encoded bytes.
// If the ctype parameter is given, this method will set the
// Content-Type header equal to ctype. If ctype is not given,
// The Content-Type header will be set to application/cbor.
func (c *DefaultCtx) CBOR(data any, ctype ...string) error {
raw, err := c.app.config.CBOREncoder(data)
if err != nil {
return err
}
c.fasthttp.Response.SetBodyRaw(raw)
if len(ctype) > 0 {
c.fasthttp.Response.Header.SetContentType(ctype[0])
} else {
c.fasthttp.Response.Header.SetContentType(MIMEApplicationCBOR)
}
return nil
}

// JSONP sends a JSON response with JSONP support.
// This method is identical to JSON, except that it opts-in to JSONP callback support.
// By default, the callback name is simply callback.
Expand Down
5 changes: 5 additions & 0 deletions ctx_interface_gen.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Loading