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 #216: Enable to call binding multiple times in some formats #1341

Merged
merged 10 commits into from
May 11, 2018
59 changes: 59 additions & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,7 @@ Gin is a web framework written in Go (Golang). It features a martini-like API wi
- [Graceful restart or stop](#graceful-restart-or-stop)
- [Build a single binary with templates](#build-a-single-binary-with-templates)
- [Bind form-data request with custom struct](#bind-form-data-request-with-custom-struct)
- [Try to bind body into different structs](#try-to-bind-body-into-different-structs)
- [Testing](#testing)
- [Users](#users--)

Expand Down Expand Up @@ -1554,6 +1555,64 @@ type StructZ struct {

In a word, only support nested custom struct which have no `form` now.

### Try to bind body into different structs

The normal methods for binding request body consumes `c.Request.Body` and they
cannot be called multiple times.

```go
type formA struct {
Foo string `json:"foo" xml:"foo" binding:"required"`
}

type formB struct {
Bar string `json:"bar" xml:"bar" binding:"required"`
}

func SomeHandler(c *gin.Context) {
objA := formA{}
objB := formB{}
// This c.ShouldBind consumes c.Request.Body and it cannot be reused.
if errA := c.ShouldBind(&objA); errA == nil {
c.String(http.StatusOK, `the body should be formA`)
// Always an error is occurred by this because c.Request.Body is EOF now.
} else if errB := c.ShouldBind(&objB); errB == nil {
c.String(http.StatusOK, `the body should be formB`)
} else {
...
}
}
```

For this, you can use `c.ShouldBindBodyWith`.

```go
func SomeHandler(c *gin.Context) {
objA := formA{}
objB := formB{}
// This reads c.Request.Body and stores the result into the context.
if errA := c.ShouldBindBodyWith(&objA, binding.JSON); errA == nil {
c.String(http.StatusOK, `the body should be formA`)
// At this time, it reuses body stored in the context.
} else if errB := c.ShouldBindBodyWith(&objB, binding.JSON); errB == nil {
c.String(http.StatusOK, `the body should be formB JSON`)
// And it can accepts other formats
} else if errB2 := c.ShouldBindBodyWith(&objB, binding.XML); errB2 == nil {
c.String(http.StatusOK, `the body should be formB XML`)
} else {
...
}
}
```

* `c.ShouldBindBodyWith` stores body into the context before binding. This has
a slight impact to performance, so you should not use this method if you are
enough to call binding at once.
* This feature is only needed for some formats -- `JSON`, `XML`, `MsgPack`,
`ProtoBuf`. For other formats, `Query`, `Form`, `FormPost`, `FormMultipart`,
can be called by `c.ShouldBind()` multiple times without any damage to
performance (See [#1341](https://github.com/gin-gonic/gin/pull/1341)).

## Testing

The `net/http/httptest` package is preferable way for HTTP testing.
Expand Down
7 changes: 7 additions & 0 deletions binding/binding.go
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,13 @@ type Binding interface {
Bind(*http.Request, interface{}) error
}

// BindingBody adds BindBody method to Binding. BindBody is similar with Bind,
// but it reads the body from supplied bytes instead of req.Body.
type BindingBody interface {
Binding
BindBody([]byte, interface{}) error
}

// StructValidator is the minimal interface which needs to be implemented in
// order for it to be used as the validator engine for ensuring the correctness
// of the reqest. Gin provides a default implementation for this using
Expand Down
67 changes: 67 additions & 0 deletions binding/binding_body_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,67 @@
package binding

import (
"bytes"
"io/ioutil"
"testing"

"github.com/gin-gonic/gin/binding/example"
"github.com/golang/protobuf/proto"
"github.com/stretchr/testify/assert"
"github.com/ugorji/go/codec"
)

func TestBindingBody(t *testing.T) {
for _, tt := range []struct {
name string
binding BindingBody
body string
want string
}{
{
name: "JSON bidning",
binding: JSON,
body: `{"foo":"FOO"}`,
},
{
name: "XML bidning",
binding: XML,
body: `<?xml version="1.0" encoding="UTF-8"?>
<root>
<foo>FOO</foo>
</root>`,
},
{
name: "MsgPack binding",
binding: MsgPack,
body: msgPackBody(t),
},
} {
t.Logf("testing: %s", tt.name)
req := requestWithBody("POST", "/", tt.body)
form := FooStruct{}
body, _ := ioutil.ReadAll(req.Body)
assert.NoError(t, tt.binding.BindBody(body, &form))
assert.Equal(t, FooStruct{"FOO"}, form)
}
}

func msgPackBody(t *testing.T) string {
test := FooStruct{"FOO"}
h := new(codec.MsgpackHandle)
buf := bytes.NewBuffer(nil)
assert.NoError(t, codec.NewEncoder(buf, h).Encode(test))
return buf.String()
}

func TestBindingBodyProto(t *testing.T) {
test := example.Test{
Label: proto.String("FOO"),
}
data, _ := proto.Marshal(&test)
req := requestWithBody("POST", "/", string(data))
form := example.Test{}
body, _ := ioutil.ReadAll(req.Body)
assert.NoError(t, ProtoBuf.BindBody(body, &form))
assert.Equal(t, test, form)
}
12 changes: 11 additions & 1 deletion binding/json.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,8 @@
package binding

import (
"bytes"
"io"
"net/http"

"github.com/gin-gonic/gin/json"
Expand All @@ -22,7 +24,15 @@ func (jsonBinding) Name() string {
}

func (jsonBinding) Bind(req *http.Request, obj interface{}) error {
decoder := json.NewDecoder(req.Body)
return decodeJSON(req.Body, obj)
}

func (jsonBinding) BindBody(body []byte, obj interface{}) error {
return decodeJSON(bytes.NewReader(body), obj)
}

func decodeJSON(r io.Reader, obj interface{}) error {
decoder := json.NewDecoder(r)
if EnableDecoderUseNumber {
decoder.UseNumber()
}
Expand Down
13 changes: 12 additions & 1 deletion binding/msgpack.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,8 @@
package binding

import (
"bytes"
"io"
"net/http"

"github.com/ugorji/go/codec"
Expand All @@ -17,7 +19,16 @@ func (msgpackBinding) Name() string {
}

func (msgpackBinding) Bind(req *http.Request, obj interface{}) error {
if err := codec.NewDecoder(req.Body, new(codec.MsgpackHandle)).Decode(&obj); err != nil {
return decodeMsgPack(req.Body, obj)
}

func (msgpackBinding) BindBody(body []byte, obj interface{}) error {
return decodeMsgPack(bytes.NewReader(body), obj)
}

func decodeMsgPack(r io.Reader, obj interface{}) error {
cdc := new(codec.MsgpackHandle)
if err := codec.NewDecoder(r, cdc).Decode(&obj); err != nil {
return err
}
return validate(obj)
Expand Down
15 changes: 8 additions & 7 deletions binding/protobuf.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,19 +17,20 @@ func (protobufBinding) Name() string {
return "protobuf"
}

func (protobufBinding) Bind(req *http.Request, obj interface{}) error {

func (b protobufBinding) Bind(req *http.Request, obj interface{}) error {
buf, err := ioutil.ReadAll(req.Body)
if err != nil {
return err
}
return b.BindBody(buf, obj)
}

if err = proto.Unmarshal(buf, obj.(proto.Message)); err != nil {
func (protobufBinding) BindBody(body []byte, obj interface{}) error {
if err := proto.Unmarshal(body, obj.(proto.Message)); err != nil {
return err
}

//Here it's same to return validate(obj), but util now we cann't add `binding:""` to the struct
//which automatically generate by gen-proto
// Here it's same to return validate(obj), but util now we cann't add
// `binding:""` to the struct which automatically generate by gen-proto
return nil
//return validate(obj)
// return validate(obj)
}
11 changes: 10 additions & 1 deletion binding/xml.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,9 @@
package binding

import (
"bytes"
"encoding/xml"
"io"
"net/http"
)

Expand All @@ -16,7 +18,14 @@ func (xmlBinding) Name() string {
}

func (xmlBinding) Bind(req *http.Request, obj interface{}) error {
decoder := xml.NewDecoder(req.Body)
return decodeXML(req.Body, obj)
}

func (xmlBinding) BindBody(body []byte, obj interface{}) error {
return decodeXML(bytes.NewReader(body), obj)
}
func decodeXML(r io.Reader, obj interface{}) error {
decoder := xml.NewDecoder(r)
if err := decoder.Decode(obj); err != nil {
return err
}
Expand Down
25 changes: 25 additions & 0 deletions context.go
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@ const (
MIMEPlain = binding.MIMEPlain
MIMEPOSTForm = binding.MIMEPOSTForm
MIMEMultipartPOSTForm = binding.MIMEMultipartPOSTForm
BodyBytesKey = "_gin-gonic/gin/bodybyteskey"
)

const abortIndex int8 = math.MaxInt8 / 2
Expand Down Expand Up @@ -508,6 +509,30 @@ func (c *Context) ShouldBindWith(obj interface{}, b binding.Binding) error {
return b.Bind(c.Request, obj)
}

// ShouldBindBodyWith is similar with ShouldBindWith, but it stores the request
// body into the context, and reuse when it is called again.
//
// NOTE: This method reads the body before binding. So you should use
// ShouldBindWith for better performance if you need to call only once.
func (c *Context) ShouldBindBodyWith(
obj interface{}, bb binding.BindingBody,
) (err error) {
Copy link
Member

Choose a reason for hiding this comment

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

Maybe not need named return.

Copy link
Author

Choose a reason for hiding this comment

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

Without this, I should add an explicit declaration var err error because this is needed for reusing body in L527.

// if you use unnamed return value...
func (...) ShouldBindBodyWith(...) error {

  ...

  if body == nil {
    // you need explicit declaration here.
    var err error
    body, err = ioutil.ReadAll(...)
    if err != nil {
      return err
    }
    ...
  }
  return ...
}

I want to simplify the code and use this named return variable.

Copy link
Member

Choose a reason for hiding this comment

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

OK, got it! Thanks~

var body []byte
if cb, ok := c.Get(BodyBytesKey); ok {
if cbb, ok := cb.([]byte); ok {
body = cbb
}
}
if body == nil {
body, err = ioutil.ReadAll(c.Request.Body)
if err != nil {
return err
}
c.Set(BodyBytesKey, body)
}
return bb.BindBody(body, obj)
}

// ClientIP implements a best effort algorithm to return the real client IP, it parses
// X-Real-IP and X-Forwarded-For in order to work properly with reverse-proxies such us: nginx or haproxy.
// Use X-Forwarded-For before X-Real-Ip as nginx uses X-Real-Ip with the proxy's IP.
Expand Down
80 changes: 80 additions & 0 deletions context_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ import (
"time"

"github.com/gin-contrib/sse"
"github.com/gin-gonic/gin/binding"
"github.com/stretchr/testify/assert"
"golang.org/x/net/context"
)
Expand Down Expand Up @@ -1334,6 +1335,85 @@ func TestContextBadAutoShouldBind(t *testing.T) {
assert.False(t, c.IsAborted())
}

func TestContextShouldBindBodyWith(t *testing.T) {
type typeA struct {
Foo string `json:"foo" xml:"foo" binding:"required"`
}
type typeB struct {
Bar string `json:"bar" xml:"bar" binding:"required"`
}
for _, tt := range []struct {
name string
bindingA, bindingB binding.BindingBody
bodyA, bodyB string
}{
{
name: "JSON & JSON",
bindingA: binding.JSON,
bindingB: binding.JSON,
bodyA: `{"foo":"FOO"}`,
bodyB: `{"bar":"BAR"}`,
},
{
name: "JSON & XML",
bindingA: binding.JSON,
bindingB: binding.XML,
bodyA: `{"foo":"FOO"}`,
bodyB: `<?xml version="1.0" encoding="UTF-8"?>
<root>
<bar>BAR</bar>
</root>`,
},
{
name: "XML & XML",
bindingA: binding.XML,
bindingB: binding.XML,
bodyA: `<?xml version="1.0" encoding="UTF-8"?>
<root>
<foo>FOO</foo>
</root>`,
bodyB: `<?xml version="1.0" encoding="UTF-8"?>
<root>
<bar>BAR</bar>
</root>`,
},
} {
t.Logf("testing: %s", tt.name)
// bodyA to typeA and typeB
{
w := httptest.NewRecorder()
c, _ := CreateTestContext(w)
c.Request, _ = http.NewRequest(
"POST", "http://example.com", bytes.NewBufferString(tt.bodyA),
)
// When it binds to typeA and typeB, it finds the body is
// not typeB but typeA.
objA := typeA{}
assert.NoError(t, c.ShouldBindBodyWith(&objA, tt.bindingA))
assert.Equal(t, typeA{"FOO"}, objA)
objB := typeB{}
assert.Error(t, c.ShouldBindBodyWith(&objB, tt.bindingB))
assert.NotEqual(t, typeB{"BAR"}, objB)
}
// bodyB to typeA and typeB
{
// When it binds to typeA and typeB, it finds the body is
// not typeA but typeB.
w := httptest.NewRecorder()
c, _ := CreateTestContext(w)
c.Request, _ = http.NewRequest(
"POST", "http://example.com", bytes.NewBufferString(tt.bodyB),
)
objA := typeA{}
assert.Error(t, c.ShouldBindBodyWith(&objA, tt.bindingA))
assert.NotEqual(t, typeA{"FOO"}, objA)
objB := typeB{}
assert.NoError(t, c.ShouldBindBodyWith(&objB, tt.bindingB))
assert.Equal(t, typeB{"BAR"}, objB)
}
}
}

func TestContextGolangContext(t *testing.T) {
c, _ := CreateTestContext(httptest.NewRecorder())
c.Request, _ = http.NewRequest("POST", "/", bytes.NewBufferString("{\"foo\":\"bar\", \"bar\":\"foo\"}"))
Expand Down