Fix #180355662 Body not being logged correctly

[bakennedy]

https://www.pivotaltracker.com/n/projects/1646401/stories/180355662

http.Request.Body is an implementaition of the interface io.ReadCloser, this means you can read it and close it, and that's all it guarantees. Where our bug comes in is that both the customer's server will read request.Body and then if body logging is enabled, our middleware will read request.Body, but there is no Seek implementation on this or anything. Body is a stream which cannot be reversed and re-read. The only reason it worked in my testing earlier is that our example app doesn't read the body itself. When you read the body in the example handler it reproduces the customers bug

[bakennedy]

https://www.moesif.com/wrap/app/640:128-617:188/search/events/dates/-7d contains some example data from my testing both reading and not reading the request body using the original code and then using this buffered two ReadCloser approach

[keyur9]

@bakennedy please make the changes and bump the version.

[keyur9]

@bakennedy the correct package here is ioutil which has NopCloser function. It should be ioutil.NopCloser instead of io.NopCloser. This version won't get installed as it'll throw undefined error on io.NopCloser.

[bakennedy]

That function was moved to the I/o package last year. What is the error that you are expecting? golang/go#40025

…

On Fri, Nov 19, 2021, 13:18 Keyur ***@***.***> wrote: ***@***.**** commented on this pull request. @bakennedy <https://github.com/bakennedy> please make the changes and bump the version. ------------------------------ In moesifmiddleware.go <#23 (comment)> : > +// teeBody reads all of b to memory and then returns two equivalent +// ReadClosers yielding the same bytes. +// It returns an error if the initial slurp of all bytes fails. +func teeBody(b io.ReadCloser) (r1, r2 io.ReadCloser, err error) { + if b == nil || b == http.NoBody { + // No copying needed + return http.NoBody, http.NoBody, nil + } + var buf bytes.Buffer + if _, err = buf.ReadFrom(b); err != nil { + return nil, b, err + } + if err = b.Close(); err != nil { + return nil, b, err + } + return io.NopCloser(&buf), io.NopCloser(bytes.NewReader(buf.Bytes())), nil @bakennedy <https://github.com/bakennedy> the correct package here is ioutil which has NopCloser function. It should be ioutil.NopCloser instead of io.NopCloser. This version won't get installed as it'll throw undefined error on io.NopCloser. — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#23 (review)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AAF4FQVVR43DQXUU66NG7CDUM25KRANCNFSM5ILMUAVQ> .

[keyur9]

It's not backward compatible, so if anyone using go1.14.3 and similar, it'd throw an error while importing [email protected] package. In earlier go version, ReadAll, NopCloser, and all are still part of ioutil package. So this would break for existing users who are on not on the latest go version.

[bakennedy]

Okay, I understood. Derric asked me to test 1.17.3, but it makes sense that that would just be for the customer and not our minimum supported version. What is our minimum supported version?

…

On Fri, Nov 19, 2021, 13:29 Keyur ***@***.***> wrote: It's not backward compatible, so if anyone using go1.14.3 and similar, it'd throw an error while importing ***@***.*** package. In earlier go version, ReadAll, NopCloser, and all are still part of ioutil package ***@***.***>. So this would break for existing users who are on not on the latest go version. — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#23 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AAF4FQXEFS5SSKZBAD73BCDUM26U3ANCNFSM5ILMUAVQ> .