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

Calling bind multiple times #216

Closed
thdxr opened this issue Feb 9, 2015 · 25 comments · Fixed by #1341
Closed

Calling bind multiple times #216

thdxr opened this issue Feb 9, 2015 · 25 comments · Fixed by #1341
Assignees

Comments

@thdxr
Copy link

thdxr commented Feb 9, 2015

I'm trying to call bind multiple times to bind to two different models during the same request. It only works the first time I call it, it ends up not binding anything the second time. I suspect this is because a buffer is being read and not being reset. Is there a fix or workaround for this?

@javierprovecho
Copy link
Member

I'll take a look into this, I can't guarantee any estimated time just than v0.6.

@javierprovecho javierprovecho added this to the 0.6 milestone Feb 12, 2015
@javierprovecho javierprovecho self-assigned this Feb 12, 2015
@javierprovecho javierprovecho removed this from the 0.6 milestone Mar 9, 2015
@msoedov
Copy link
Contributor

msoedov commented Mar 27, 2015

Hi guys, I found dirty hack solution how to fix this. But looks like extra memory allocation for bytes.NewBuffer(body) can leads to a performance decrease. Any thoughts?

func bodyReader(req *http.Request) (*bytes.Buffer, error) {
    if req.Body == nil {
        return nil, errors.New("Error")
    }
    body, err := ioutil.ReadAll(req.Body)
    if err != nil {
        return nil, err
    }
    buff := bytes.NewBuffer(body)
    req.Body = ioutil.NopCloser(buff)
    return bytes.NewBuffer(body), nil
}

func (_ jsonBinding) Bind(req *http.Request, obj interface{}) error {
    buff, _ := bodyReader(req)
    decoder := json.NewDecoder(buff)
    if err := decoder.Decode(obj); err == nil {
        return Validate(obj)
    } else {
        return err
    }
}

Test:

//  issue #216
func TestBindingMultipleTimes(t *testing.T) {

    body := bytes.NewBuffer([]byte("{\"foo\":\"bar\"}"))

    r := New()
    var modelA struct {
        A string `json:"foo"`
    }

    var modelB struct {
        B string `json:"foo"`
    }
    r.POST("/binding/multiple/bind", func(c *Context) {
        if !c.Bind(&modelA) {
            c.JSON(500, H{"error": "failed to bind first model"})
            return
        }
        if !c.Bind(&modelB) {
            c.JSON(500, H{"error": "failed to bind second model"})
            return
        }

        if modelA.A != modelB.B {
            debugPrint("%s!=%s", modelA, modelB)
            c.JSON(500, H{"error": "failed fields aren't equals"})
            return

        }
        c.JSON(200, H{"parsed": modelB.B})

    })

    req, _ := http.NewRequest("POST", "/binding/multiple/bind", body)
    req.Header.Set("Content-Type", "application/json")
    w := httptest.NewRecorder()

    r.ServeHTTP(w, req)

    if w.Code != 200 {
        t.Errorf("Response code should be Ok, was: %d", w.Code)
    }

    if w.Body.String() != "{\"parsed\":\"bar\"}\n" {
        t.Errorf("Response should be {\"parsed\":\"bar\"}, was: %s", w.Body.String())
    }

}

@manucorporat
Copy link
Contributor

how about this approach?

    r.Use(func(c *Context) {
        var model Model
        if c.Bind(&model) {
            c.Set("data_model", model)
            // use model
        }else{
            c.AbortWithStatus(400)
        }
    })

    r.POST("/binding/multiple/bind", func(c *Context) {
        model := c.MustGet("data_model").(Model)
    })

@msoedov
Copy link
Contributor

msoedov commented Mar 27, 2015

It won't work for case when model fields are significantly different i.e

    body := bytes.NewBuffer([]byte(`{"foo":"bar", "time": 2323793, "name": "GIN"}`))

    r := New()
    var modelA struct {
        A    string `json:"foo"`
        Name string `json:"name"`
    }

    var modelB struct {
        B         string `json:"foo"`
        Timestamp int    `json:"time"`
    }

Is it a real case? What do you think?

@nazwa
Copy link

nazwa commented Mar 27, 2015

I agree that binding multiple times should be possible one way or another, even with explicit and manual buffer refresh, since this will be a very infrequent use case. In the meantime, as a workaround, you could just create a 'parent' model that inherits both A and B

var modelAB struct {
    modelA
    modelB
}

Which, if i remember correctly, should bind nicely all variables. Only issue is that then you end up with all data in one but this shouldn't be too much of a problem in most cases.

@thinkong
Copy link

thinkong commented Feb 2, 2016

Has there been any progress on this?
Is there a work around?

@ghost
Copy link

ghost commented Dec 1, 2016

I am also in need of a way to bind a request and then also read it again. I could copy it to a bytes.Buffer, but then can no longer bind the request...

@adriendomoison
Copy link

adriendomoison commented Feb 26, 2017

+1 this would be awesome to patch that !

@manucorporat using the set capability is definitely not the approach we would like to implement since it makes routes highly dependent of middlewares, while middlewares should be only "plugins", @msoedov solution is today the only viable alternative.

Thank you ! ❤️

@joyride9999
Copy link

++ yes would be nice to have this .... there are lot of scenarios where this has uses....

@gawonmi
Copy link

gawonmi commented Apr 21, 2017

+1

11 similar comments
@vspiewak
Copy link

+1

@apieceofredcloth
Copy link

+1

@bradleygore
Copy link

+1

@anasanzari
Copy link

+1

@denmasyarikin
Copy link

+1

@sjnonweb
Copy link

+1

@chushik
Copy link

chushik commented Jan 31, 2018

+1

@kensay98
Copy link

+1

@richzw
Copy link

richzw commented Apr 17, 2018

+1

@flexphere
Copy link

+1

@rhzs
Copy link

rhzs commented Apr 27, 2018

+1

appleboy pushed a commit that referenced this issue May 11, 2018
* Add interface to read body bytes in binding

* Add BindingBody implementation for some binding

* Fix to use `BindBodyBytesKey` for key

* Revert "Fix to use `BindBodyBytesKey` for key"

This reverts commit 2c82901.

* Use private-like key for body bytes

* Add tests for BindingBody & ShouldBindBodyWith

* Add note for README

* Remove redundant space between sentences
@appleboy
Copy link
Member

appleboy commented May 11, 2018

See the implementation #1341 try it in the latest of master branch.

@skliarovartem
Copy link

I want to use it twice, the first one for middleware and the second time in my controller. Any suggestions?
Many thanks!

@sh0umik
Copy link

sh0umik commented Oct 26, 2020

I want to use it twice, the first one for middleware and the second time in my controller. Any suggestions?
Many thanks!

Follow the answer here https://stackoverflow.com/questions/62736851/go-gin-read-request-body-many-times

works well with ShouldBindBodyWith

@inievaMeLi
Copy link

i was trying to implement ShouldBindBodyWith, accidentally sent a literal empty body and got a panic.
Is this implementation supposed to panic with a "" body? (pretty new in go)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.