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
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,
Copy link
Member

Choose a reason for hiding this comment

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

Please use two spaces, thanks.

Copy link
Author

Choose a reason for hiding this comment

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

You mean “one space”, right? mean to replace ...˽Binding.˽˽BindBody˽is... into ...˽Binding.˽BindBody˽is... ?

Copy link
Member

Choose a reason for hiding this comment

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

Oh....so sorry, one space!

// 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
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 = "github.com/gin-gonic/gin/bodyBytes"
Copy link
Member

@thinkerou thinkerou May 1, 2018

Choose a reason for hiding this comment

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

Sorry, I don't understand it's mean, a little surprising.
Ohh, understand, but the value maybe BindBodyBytesKey better?

Copy link
Author

Choose a reason for hiding this comment

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

Sure. fixed.

Copy link
Member

Choose a reason for hiding this comment

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

I mean: BodyBytesKey = "BindBodyBytesKey", actually I see github.com/gin-gonic/gin/bodyBytes at first feel like import, do you think?

Copy link
Author

Choose a reason for hiding this comment

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

Ah, yes, but it is a bit dangerous. c.Get(key)'s key is only a string and this is easy to conflict with user-defined keys. I meant the full import-pathish strings as a private key.

I grepped sources and found BindKey = "_gin-gonic/gin/bindkey" (here). Following this, I want to use BodyBytesKey = "_gin-gonic/gin/bodybyteskey", how about this?

Copy link
Member

Choose a reason for hiding this comment

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

Good, agreed!

)

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
Copy link
Member

Choose a reason for hiding this comment

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

Please use two spaces, thanks.

// 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